From 4aa5aae691c75cbeea86f210ad34178c7d497cae Mon Sep 17 00:00:00 2001 From: Bryan Helmkamp Date: Fri, 6 Mar 2026 19:28:00 -0500 Subject: [PATCH] Fix duplicate .env vars on re-setup, capture GitHub App slug Replace append-based .env writing with mergeEnv() that upserts keys, preventing duplicates when setup runs multiple times. Capture the GitHub App slug from the manifest API response and persist it in TOML config so we can construct installation URLs. Co-Authored-By: Claude Opus 4.6 (1M context) --- apps/arc-web/app/lib/config.server.ts | 2 + apps/arc-web/app/lib/merge-env.test.ts | 76 ++++++++++++++++++++++ apps/arc-web/app/lib/merge-env.ts | 46 +++++++++++++ apps/arc-web/app/routes/setup-callback.tsx | 22 ++++--- crates/arc-api/src/demo/mod.rs | 1 + crates/arc-api/src/server_config.rs | 1 + 6 files changed, 139 insertions(+), 9 deletions(-) create mode 100644 apps/arc-web/app/lib/merge-env.test.ts create mode 100644 apps/arc-web/app/lib/merge-env.ts diff --git a/apps/arc-web/app/lib/config.server.ts b/apps/arc-web/app/lib/config.server.ts index 35bd18c3e..ed6845831 100644 --- a/apps/arc-web/app/lib/config.server.ts +++ b/apps/arc-web/app/lib/config.server.ts @@ -17,6 +17,7 @@ interface GitConfig { provider: "github"; app_id: string | null; client_id: string | null; + slug: string | null; } interface FeatureFlags { @@ -54,6 +55,7 @@ const GIT_DEFAULTS: GitConfig = { provider: "github", app_id: null, client_id: null, + slug: null, }; const FEATURE_FLAGS_DEFAULTS: FeatureFlags = { diff --git a/apps/arc-web/app/lib/merge-env.test.ts b/apps/arc-web/app/lib/merge-env.test.ts new file mode 100644 index 000000000..c77331a83 --- /dev/null +++ b/apps/arc-web/app/lib/merge-env.test.ts @@ -0,0 +1,76 @@ +import { describe, expect, test } from "bun:test"; +import { mergeEnv } from "./merge-env"; + +describe("mergeEnv", () => { + test("replaces existing key with new value", () => { + const result = mergeEnv( + "FOO=old\nBAR=keep\n", + new Map([["FOO", "new"]]), + ); + expect(result).toContain("FOO=new"); + expect(result).toContain("BAR=keep"); + }); + + test("preserves comments, blank lines, and unrelated vars", () => { + const existing = "# A comment\n\nFOO=old\n# Another\nBAR=keep\n"; + const result = mergeEnv(existing, new Map([["FOO", "new"]])); + expect(result).toContain("# A comment"); + expect(result).toContain("# Another"); + expect(result).toContain("FOO=new"); + expect(result).toContain("BAR=keep"); + }); + + test("appends keys not already present", () => { + const result = mergeEnv( + "FOO=old\n", + new Map([ + ["FOO", "new"], + ["BAZ", "added"], + ]), + ); + expect(result).toContain("FOO=new"); + expect(result).toContain("BAZ=added"); + }); + + test("handles export prefix", () => { + const result = mergeEnv( + "export FOO=old\nexport BAR=keep\n", + new Map([["FOO", "new"]]), + ); + expect(result).toContain("export FOO=new"); + expect(result).toContain("export BAR=keep"); + }); + + test("idempotent: merging twice produces same result", () => { + const vars = new Map([ + ["FOO", "new"], + ["BAZ", "added"], + ]); + const first = mergeEnv("FOO=old\nBAR=keep\n", vars); + const second = mergeEnv(first, vars); + expect(second).toBe(first); + }); + + test("full scenario matches expected output", () => { + const result = mergeEnv( + "FOO=old\nBAR=keep", + new Map([ + ["FOO", "new"], + ["BAZ", "added"], + ]), + ); + expect(result).toBe("FOO=new\nBAR=keep\nBAZ=added\n"); + }); + + test("empty existing string", () => { + const result = mergeEnv( + "", + new Map([ + ["FOO", "bar"], + ["BAZ", "qux"], + ]), + ); + expect(result).toContain("FOO=bar"); + expect(result).toContain("BAZ=qux"); + }); +}); diff --git a/apps/arc-web/app/lib/merge-env.ts b/apps/arc-web/app/lib/merge-env.ts new file mode 100644 index 000000000..14930bab6 --- /dev/null +++ b/apps/arc-web/app/lib/merge-env.ts @@ -0,0 +1,46 @@ +/** + * Merge new key=value pairs into an existing .env file string. + * - Replaces lines whose key matches (handles optional `export ` prefix) + * - Preserves comments, blank lines, and unrelated variables + * - Appends keys not already present + */ +export function mergeEnv( + existing: string, + newVars: Map, +): string { + const handledKeys = new Set(); + const resultLines: string[] = []; + + for (const line of existing.split("\n")) { + const eqPos = line.indexOf("="); + if (eqPos !== -1) { + let rawKey = line.slice(0, eqPos).trim(); + const hasExport = rawKey.startsWith("export "); + if (hasExport) { + rawKey = rawKey.slice("export ".length).trim(); + } + if (rawKey.length > 0 && !rawKey.startsWith("#")) { + const newVal = newVars.get(rawKey); + if (newVal !== undefined) { + const prefix = hasExport ? "export " : ""; + resultLines.push(`${prefix}${rawKey}=${newVal}`); + handledKeys.add(rawKey); + continue; + } + } + } + resultLines.push(line); + } + + for (const [key, val] of newVars) { + if (!handledKeys.has(key)) { + resultLines.push(`${key}=${val}`); + } + } + + let result = resultLines.join("\n"); + if (!result.endsWith("\n")) { + result += "\n"; + } + return result; +} diff --git a/apps/arc-web/app/routes/setup-callback.tsx b/apps/arc-web/app/routes/setup-callback.tsx index f9e9d187d..810440547 100644 --- a/apps/arc-web/app/routes/setup-callback.tsx +++ b/apps/arc-web/app/routes/setup-callback.tsx @@ -4,6 +4,7 @@ import { randomBytes } from "node:crypto"; import { redirect } from "react-router"; import { parse, stringify } from "smol-toml"; import { ARC_CONFIG_PATH, reloadAppConfig } from "../lib/config.server"; +import { mergeEnv } from "../lib/merge-env"; import type { Route } from "./+types/setup-callback"; const ENV_PATH = resolve(import.meta.dirname, "../../../../.env"); @@ -27,6 +28,7 @@ export async function loader({ request }: Route.LoaderArgs) { const data = (await response.json()) as { id: number; + slug: string; client_id: string; client_secret: string; webhook_secret: string; @@ -47,12 +49,13 @@ export async function loader({ request }: Route.LoaderArgs) { provider: "github", app_id: String(data.id), client_id: data.client_id, + slug: data.slug, }; await mkdir(dirname(ARC_CONFIG_PATH), { recursive: true }); await writeFile(ARC_CONFIG_PATH, stringify(tomlConfig), "utf-8"); reloadAppConfig(); - // Write secrets to .env + // Write secrets to .env (merge to avoid duplicates on re-run) let existing = ""; try { existing = await readFile(ENV_PATH, "utf-8"); @@ -60,14 +63,15 @@ export async function loader({ request }: Route.LoaderArgs) { // file doesn't exist yet } - const newVars = [ - `export SESSION_SECRET=${sessionSecret}`, - `export GITHUB_APP_CLIENT_SECRET=${data.client_secret}`, - `export GITHUB_APP_WEBHOOK_SECRET=${data.webhook_secret}`, - `export GITHUB_APP_PRIVATE_KEY=${Buffer.from(data.pem).toString("base64")}`, - ].join("\n"); - - const envContent = existing ? `${existing.trimEnd()}\n\n${newVars}\n` : `${newVars}\n`; + const envContent = mergeEnv( + existing, + new Map([ + ["SESSION_SECRET", sessionSecret], + ["GITHUB_APP_CLIENT_SECRET", data.client_secret], + ["GITHUB_APP_WEBHOOK_SECRET", data.webhook_secret], + ["GITHUB_APP_PRIVATE_KEY", Buffer.from(data.pem).toString("base64")], + ]), + ); await writeFile(ENV_PATH, envContent, "utf-8"); process.env.SESSION_SECRET = sessionSecret; diff --git a/crates/arc-api/src/demo/mod.rs b/crates/arc-api/src/demo/mod.rs index 451fadfe0..907460dd6 100644 --- a/crates/arc-api/src/demo/mod.rs +++ b/crates/arc-api/src/demo/mod.rs @@ -2568,6 +2568,7 @@ mod settings { provider: GitProvider::Github, app_id: Some("12345".into()), client_id: Some("Iv1.abc123".into()), + slug: Some("arc-dev".into()), }, feature_flags: FeatureFlags { session_sandboxes: false, diff --git a/crates/arc-api/src/server_config.rs b/crates/arc-api/src/server_config.rs index 50b312163..cc67a4368 100644 --- a/crates/arc-api/src/server_config.rs +++ b/crates/arc-api/src/server_config.rs @@ -71,6 +71,7 @@ pub struct GitConfig { pub provider: GitProvider, pub app_id: Option, pub client_id: Option, + pub slug: Option, } #[derive(Debug, Clone, Deserialize, PartialEq, Serialize)]