From 48e5cf8023f08cb1775da5b5e09045184537d07a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ignacy=20=C5=81=C4=85tka?= Date: Tue, 30 Jun 2026 18:18:36 +0200 Subject: [PATCH 01/10] fix(installer): stop clobbering foreign and JSONC MCP server config The MCP config installer's uninstall/write paths violated the "only touch argent" contract in two ways: Defect A (uninstall over-prunes unrelated config): writeJsonOrRemove ran the config through pruneEmptyConfig, which recursively dropped every empty object/array anywhere in the tree. After remove() deleted the argent entry and wrote back, other MCP servers lost their args:[] / env:{} keys and any sibling user key holding an empty object/array was deleted. Defect B (VS Code clobbers JSONC config): .vscode/mcp.json is JSONC (VS Code allows comments and trailing commas), but the VS Code adapter read it with readJson, whose catch returns {}; write then persisted only the argent entry, destroying every pre-existing user server and comment. Fix A: writeJsonOrRemove now writes data unchanged and only deletes the file when the top-level object has no own keys; each remover collapses just its own emptied argent container (via deleteIfEmpty) so a config that held only argent still results in file deletion. No more deep prune. Fix B: the VS Code adapter now reads with readJsonc and applies write/ remove via editJsoncFile, mirroring the Zed and opencode adapters, so comments and foreign servers survive. --- packages/argent-installer/src/mcp-configs.ts | 62 ++++++++++++++----- .../argent-installer/test/mcp-configs.test.ts | 55 ++++++++++++++++ 2 files changed, 102 insertions(+), 15 deletions(-) diff --git a/packages/argent-installer/src/mcp-configs.ts b/packages/argent-installer/src/mcp-configs.ts index ee1532db9..839132936 100644 --- a/packages/argent-installer/src/mcp-configs.ts +++ b/packages/argent-installer/src/mcp-configs.ts @@ -120,15 +120,36 @@ function isRecord(value: unknown): value is Record { return Boolean(value) && typeof value === "object" && !Array.isArray(value); } +// After a remover deletes the argent entry, collapse the now-empty container it +// lived in (e.g. an emptied `mcpServers`) so a config that held only argent +// reduces to {} and writeJsonOrRemove can delete the file. Only this one key is +// touched — foreign sibling keys and empty values elsewhere in the tree are +// preserved byte-for-byte (the contract is "only touch argent"). +function deleteIfEmpty(parent: Record, key: string): void { + const value = parent[key]; + if ( + (Array.isArray(value) && value.length === 0) || + (isRecord(value) && Object.keys(value).length === 0) + ) { + delete parent[key]; + } +} + +// Writes `data` unchanged, except: when it has no own keys the file (and an +// empty parent directory) is removed instead. This deliberately does NOT +// recursively prune empty objects/arrays — the previous deep-prune silently +// stripped foreign servers' `args: []` / `env: {}` and deleted any user key +// holding an empty object/array, violating the "only touch argent" contract. +// Removers collapse their own emptied argent container via deleteIfEmpty before +// calling here, so "config held only argent" still results in file deletion. function writeJsonOrRemove(filePath: string, data: Record): void { - const cleaned = pruneEmptyConfig(data); - if (!isRecord(cleaned)) { + if (Object.keys(data).length === 0) { fs.rmSync(filePath, { force: true }); removeDirIfEmpty(path.dirname(filePath)); return; } - writeJson(filePath, cleaned); + writeJson(filePath, data); } function writeTomlOrRemove(filePath: string, data: Record): void { @@ -181,6 +202,7 @@ const cursorAdapter: McpConfigAdapter = { const servers = config.mcpServers as Record | undefined; if (!servers?.[MCP_SERVER_KEY]) return false; delete servers[MCP_SERVER_KEY]; + deleteIfEmpty(config, "mcpServers"); writeJsonOrRemove(configPath, config); return true; }, @@ -213,6 +235,7 @@ const cursorAdapter: McpConfigAdapter = { if (idx === -1) return; list.splice(idx, 1); config.mcpAllowlist = list; + deleteIfEmpty(config, "mcpAllowlist"); writeJsonOrRemove(permPath, config); }, }; @@ -262,6 +285,7 @@ const claudeAdapter: McpConfigAdapter = { const servers = config.mcpServers as Record | undefined; if (!servers?.[MCP_SERVER_KEY]) return false; delete servers[MCP_SERVER_KEY]; + deleteIfEmpty(config, "mcpServers"); writeJsonOrRemove(configPath, config); return true; }, @@ -304,32 +328,34 @@ const vscodeAdapter: McpConfigAdapter = { return null; }, + // .vscode/mcp.json is JSONC — VS Code allows line/block comments and trailing + // commas. The previous JSON.parse → mutate → JSON.stringify path ran through + // readJson, whose `catch { return {} }` turned any commented file into {} and + // then persisted only { servers: { argent } }, destroying every pre-existing + // user server (and their comments). All four entry points now go through + // readJsonc / editJsoncFile — path-targeted text edits that preserve comments + // and foreign servers — matching the Zed and opencode adapters. write(configPath: string, entry: McpServerEntry): void { - const config = readJson(configPath); - const servers = (config.servers ?? {}) as Record; - servers[MCP_SERVER_KEY] = { + editJsoncFile(configPath, ["servers", MCP_SERVER_KEY], { type: "stdio", command: entry.command, args: entry.args, ...(hasEnv(entry) ? { env: entry.env } : {}), - }; - config.servers = servers; - writeJson(configPath, config); + }); }, remove(configPath: string): boolean { if (!fs.existsSync(configPath)) return false; - const config = readJson(configPath); + const config = readJsonc(configPath); const servers = config.servers as Record | undefined; if (!servers?.[MCP_SERVER_KEY]) return false; - delete servers[MCP_SERVER_KEY]; - writeJsonOrRemove(configPath, config); + editJsoncFile(configPath, ["servers", MCP_SERVER_KEY], undefined); return true; }, hasArgentEntry(configPath: string): boolean { if (!fs.existsSync(configPath)) return false; - const config = readJson(configPath); + const config = readJsonc(configPath); const servers = config.servers as Record | undefined; return Boolean(servers?.[MCP_SERVER_KEY]); }, @@ -373,6 +399,7 @@ const windsurfAdapter: McpConfigAdapter = { const servers = config.mcpServers as Record | undefined; if (!servers?.[MCP_SERVER_KEY]) return false; delete servers[MCP_SERVER_KEY]; + deleteIfEmpty(config, "mcpServers"); writeJsonOrRemove(configPath, config); return true; }, @@ -526,6 +553,7 @@ const geminiAdapter: McpConfigAdapter = { const servers = config.mcpServers as Record | undefined; if (!servers?.[MCP_SERVER_KEY]) return false; delete servers[MCP_SERVER_KEY]; + deleteIfEmpty(config, "mcpServers"); writeJsonOrRemove(configPath, config); return true; }, @@ -878,6 +906,7 @@ const kiroAdapter: McpConfigAdapter = { const servers = config.mcpServers as Record | undefined; if (!servers?.[MCP_SERVER_KEY]) return false; delete servers[MCP_SERVER_KEY]; + deleteIfEmpty(config, "mcpServers"); writeJsonOrRemove(configPath, config); return true; }, @@ -1005,11 +1034,14 @@ export function removeClaudePermission(root: string, scope: "local" | "global"): if (!fs.existsSync(settingsPath)) return; const config = readJson(settingsPath); - const allow = (config?.permissions as Record)?.allow as string[]; - if (!Array.isArray(allow)) return; + const permissions = config?.permissions as Record | undefined; + const allow = permissions?.allow as string[] | undefined; + if (!permissions || !Array.isArray(allow)) return; const idx = allow.indexOf(PERMISSION_RULE); if (idx === -1) return; allow.splice(idx, 1); + deleteIfEmpty(permissions, "allow"); + deleteIfEmpty(config, "permissions"); writeJsonOrRemove(settingsPath, config); } diff --git a/packages/argent-installer/test/mcp-configs.test.ts b/packages/argent-installer/test/mcp-configs.test.ts index 2450416b6..1822505f1 100644 --- a/packages/argent-installer/test/mcp-configs.test.ts +++ b/packages/argent-installer/test/mcp-configs.test.ts @@ -1602,3 +1602,58 @@ describe("generated configs are portable across machines (issue #238)", () => { } }); }); + +// ── Foreign-config preservation ───────────────────────────────────────────── +// The installer's contract is "only touch argent". Two regressions broke it: +// - uninstall recursively pruned every empty object/array in the tree, so a +// foreign server's `args: []` / `env: {}` and any sibling user key holding an +// empty value were silently deleted (Defect A). +// - the VS Code adapter read .vscode/mcp.json with strict JSON, so a JSONC file +// (comments / trailing commas) parsed to {} and was rewritten with only the +// argent entry, destroying every pre-existing user server (Defect B). + +describe("installer preserves foreign MCP config", () => { + it("remove keeps an unrelated server's empty env/args and a sibling user key (Cursor)", () => { + const adapter = ALL_ADAPTERS.find((a) => a.name === "Cursor")!; + const dir = fs.mkdtempSync(path.join(os.tmpdir(), "argent-fc-")); + const configPath = path.join(dir, ".cursor", "mcp.json"); + fs.mkdirSync(path.dirname(configPath), { recursive: true }); + fs.writeFileSync( + configPath, + JSON.stringify( + { + mcpServers: { + argent: { command: "argent", args: ["mcp"] }, + other: { command: "other-bin", args: [], env: {} }, + }, + userSettings: { theme: "dark", overrides: {} }, + }, + null, + 2 + ) + ); + expect(adapter.remove(configPath)).toBe(true); + const after = JSON.parse(fs.readFileSync(configPath, "utf8")); + expect(after.mcpServers.other).toEqual({ command: "other-bin", args: [], env: {} }); + expect(after.userSettings).toEqual({ theme: "dark", overrides: {} }); + fs.rmSync(dir, { recursive: true, force: true }); + }); + + it("VS Code write preserves a pre-existing server in a JSONC (commented) file", () => { + const adapter = ALL_ADAPTERS.find((a) => a.name === "VS Code")!; + const dir = fs.mkdtempSync(path.join(os.tmpdir(), "argent-fc-")); + const configPath = path.join(dir, ".vscode", "mcp.json"); + fs.mkdirSync(path.dirname(configPath), { recursive: true }); + fs.writeFileSync( + configPath, + `{\n // my own MCP server, do not touch\n "servers": { "myserver": { "type": "stdio", "command": "my-bin", "args": ["x"] } }\n}` + ); + adapter.write(configPath, getMcpEntry()); + const after = parseJsonc(fs.readFileSync(configPath, "utf8"), [], { + allowTrailingComma: true, + }) as Record; + expect(after.servers).toHaveProperty("argent"); + expect(after.servers).toHaveProperty("myserver"); + fs.rmSync(dir, { recursive: true, force: true }); + }); +}); From 42d1c94d4832a87fd36e7f122101b4755ff8a420 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ignacy=20=C5=81=C4=85tka?= Date: Wed, 1 Jul 2026 17:18:26 +0200 Subject: [PATCH 02/10] fix(installer): stop deep-pruning the Codex TOML config on remove MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit writeTomlOrRemove still ran the old recursive pruneEmptyConfig, the exact "only touch argent" violation the JSON adapters were just fixed for — silently stripping a foreign Codex server's args = [] and any sibling empty table. Mirror writeJsonOrRemove: only delete the file when it has no keys at all, and have codexAdapter.remove() collapse its own emptied mcp_servers table via deleteIfEmpty first. --- packages/argent-installer/src/mcp-configs.ts | 29 +++++--------- .../argent-installer/test/mcp-configs.test.ts | 39 +++++++++++++++++++ 2 files changed, 48 insertions(+), 20 deletions(-) diff --git a/packages/argent-installer/src/mcp-configs.ts b/packages/argent-installer/src/mcp-configs.ts index 839132936..fd7a45ba8 100644 --- a/packages/argent-installer/src/mcp-configs.ts +++ b/packages/argent-installer/src/mcp-configs.ts @@ -99,23 +99,6 @@ function removeDirIfEmpty(dirPath: string): void { } } -function pruneEmptyConfig(value: unknown): unknown | undefined { - if (Array.isArray(value)) { - return value.length > 0 ? value : undefined; - } - - if (value && typeof value === "object") { - const cleaned: Record = {}; - for (const [key, entry] of Object.entries(value as Record)) { - const next = pruneEmptyConfig(entry); - if (next !== undefined) cleaned[key] = next; - } - return Object.keys(cleaned).length > 0 ? cleaned : undefined; - } - - return value; -} - function isRecord(value: unknown): value is Record { return Boolean(value) && typeof value === "object" && !Array.isArray(value); } @@ -152,15 +135,20 @@ function writeJsonOrRemove(filePath: string, data: Record): voi writeJson(filePath, data); } +// Writes `data` unchanged, except: when it has no own keys the file (and an +// empty parent directory) is removed instead. Mirrors writeJsonOrRemove (see +// its comment above) — this deliberately does NOT recursively prune empty +// tables/arrays, so a foreign TOML server's `args = []` and any sibling empty +// value survive. Callers collapse their own emptied argent container via +// deleteIfEmpty before calling here. function writeTomlOrRemove(filePath: string, data: Record): void { - const cleaned = pruneEmptyConfig(data); - if (!isRecord(cleaned)) { + if (Object.keys(data).length === 0) { fs.rmSync(filePath, { force: true }); removeDirIfEmpty(path.dirname(filePath)); return; } - writeToml(filePath, cleaned); + writeToml(filePath, data); } // ── Cursor adapter ──────────────────────────────────────────────────────────── @@ -639,6 +627,7 @@ const codexAdapter: McpConfigAdapter = { const servers = config.mcp_servers as Record | undefined; if (!servers?.[MCP_SERVER_KEY]) return false; delete servers[MCP_SERVER_KEY]; + deleteIfEmpty(config, "mcp_servers"); writeTomlOrRemove(configPath, config); return true; }, diff --git a/packages/argent-installer/test/mcp-configs.test.ts b/packages/argent-installer/test/mcp-configs.test.ts index 1822505f1..ebb2d5e9a 100644 --- a/packages/argent-installer/test/mcp-configs.test.ts +++ b/packages/argent-installer/test/mcp-configs.test.ts @@ -1656,4 +1656,43 @@ describe("installer preserves foreign MCP config", () => { expect(after.servers).toHaveProperty("myserver"); fs.rmSync(dir, { recursive: true, force: true }); }); + + it("Codex remove keeps an unrelated server's empty args and a sibling empty table (TOML)", () => { + // Same "only touch argent" contract as the JSON adapters above, but for the + // TOML-backed Codex config: writeTomlOrRemove used to deep-prune the whole + // tree (pruneEmptyConfig), silently stripping a foreign server's `args = []` + // and any sibling empty table alongside deleting the argent entry. + const adapter = ALL_ADAPTERS.find((a) => a.name === "Codex")!; + const dir = fs.mkdtempSync(path.join(os.tmpdir(), "argent-fc-")); + const configPath = path.join(dir, "config.toml"); + fs.writeFileSync( + configPath, + 'model = "o3"\n\n' + + "[other_section]\n\n" + + "[mcp_servers.other]\n" + + 'command = "other-bin"\n' + + "args = []\n\n" + + "[mcp_servers.argent]\n" + + 'command = "argent"\n' + + 'args = ["mcp"]\n' + ); + expect(adapter.remove(configPath)).toBe(true); + const after = fs.readFileSync(configPath, "utf8"); + expect(after).not.toContain("mcp_servers.argent"); + expect(after).toContain('model = "o3"'); + expect(after).toContain("[other_section]"); + expect(after).toContain("[mcp_servers.other]"); + expect(after).toContain("args = []"); + fs.rmSync(dir, { recursive: true, force: true }); + }); + + it("Codex remove deletes the file when argent was the only content (TOML)", () => { + const adapter = ALL_ADAPTERS.find((a) => a.name === "Codex")!; + const dir = fs.mkdtempSync(path.join(os.tmpdir(), "argent-fc-")); + const configPath = path.join(dir, "config.toml"); + fs.writeFileSync(configPath, '[mcp_servers.argent]\ncommand = "argent"\nargs = ["mcp"]\n'); + expect(adapter.remove(configPath)).toBe(true); + expect(fs.existsSync(configPath)).toBe(false); + fs.rmSync(dir, { recursive: true, force: true }); + }); }); From 933284039bc11d0f774272a17f79547a04dd9b99 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ignacy=20=C5=81=C4=85tka?= Date: Thu, 2 Jul 2026 00:14:58 +0200 Subject: [PATCH 03/10] fix(installer): preserve JSONC comments and foreign servers on Cursor/Kiro write --- packages/argent-installer/src/mcp-configs.ts | 97 ++++++++----------- .../argent-installer/test/mcp-configs.test.ts | 25 +++++ 2 files changed, 67 insertions(+), 55 deletions(-) diff --git a/packages/argent-installer/src/mcp-configs.ts b/packages/argent-installer/src/mcp-configs.ts index fd7a45ba8..0e1483f22 100644 --- a/packages/argent-installer/src/mcp-configs.ts +++ b/packages/argent-installer/src/mcp-configs.ts @@ -172,32 +172,32 @@ const cursorAdapter: McpConfigAdapter = { return path.join(homedir(), ".cursor", "mcp.json"); }, + // Cursor is a VS Code fork: .cursor/mcp.json is JSONC (line/block comments, + // trailing commas). Routing write/remove/hasArgentEntry through readJsonc / + // editJsoncFile applies path-targeted text edits that preserve comments and + // foreign servers. The old readJson → writeJson path ran commented files + // through readJson's `catch { return {} }`, then persisted only the argent + // entry — destroying every pre-existing server and comment. Matches VS Code. write(configPath: string, entry: McpServerEntry): void { - const config = readJson(configPath); - const servers = (config.mcpServers ?? {}) as Record; - servers[MCP_SERVER_KEY] = { + editJsoncFile(configPath, ["mcpServers", MCP_SERVER_KEY], { command: entry.command, args: entry.args, ...(hasEnv(entry) ? { env: entry.env } : {}), - }; - config.mcpServers = servers; - writeJson(configPath, config); + }); }, remove(configPath: string): boolean { if (!fs.existsSync(configPath)) return false; - const config = readJson(configPath); + const config = readJsonc(configPath); const servers = config.mcpServers as Record | undefined; if (!servers?.[MCP_SERVER_KEY]) return false; - delete servers[MCP_SERVER_KEY]; - deleteIfEmpty(config, "mcpServers"); - writeJsonOrRemove(configPath, config); + editJsoncFile(configPath, ["mcpServers", MCP_SERVER_KEY], undefined); return true; }, hasArgentEntry(configPath: string): boolean { if (!fs.existsSync(configPath)) return false; - const config = readJson(configPath); + const config = readJsonc(configPath); const servers = config.mcpServers as Record | undefined; return Boolean(servers?.[MCP_SERVER_KEY]); }, @@ -254,33 +254,30 @@ const claudeAdapter: McpConfigAdapter = { return path.join(homedir(), ".claude.json"); }, + // JSONC is a superset of JSON, so routing through readJsonc / editJsoncFile is + // safe for this strict-JSON config and keeps every MCP-entry write on the one + // comment- and foreign-server-preserving path (see the Cursor adapter). write(configPath: string, entry: McpServerEntry): void { - const config = readJson(configPath); - const servers = (config.mcpServers ?? {}) as Record; - servers[MCP_SERVER_KEY] = { + editJsoncFile(configPath, ["mcpServers", MCP_SERVER_KEY], { type: "stdio", command: entry.command, args: entry.args, ...(hasEnv(entry) ? { env: entry.env } : {}), - }; - config.mcpServers = servers; - writeJson(configPath, config); + }); }, remove(configPath: string): boolean { if (!fs.existsSync(configPath)) return false; - const config = readJson(configPath); + const config = readJsonc(configPath); const servers = config.mcpServers as Record | undefined; if (!servers?.[MCP_SERVER_KEY]) return false; - delete servers[MCP_SERVER_KEY]; - deleteIfEmpty(config, "mcpServers"); - writeJsonOrRemove(configPath, config); + editJsoncFile(configPath, ["mcpServers", MCP_SERVER_KEY], undefined); return true; }, hasArgentEntry(configPath: string): boolean { if (!fs.existsSync(configPath)) return false; - const config = readJson(configPath); + const config = readJsonc(configPath); const servers = config.mcpServers as Record | undefined; return Boolean(servers?.[MCP_SERVER_KEY]); }, @@ -369,32 +366,28 @@ const windsurfAdapter: McpConfigAdapter = { return path.join(homedir(), ".codeium", "windsurf", "mcp_config.json"); }, + // JSONC-safe MCP-entry writes (see the Cursor adapter): editJsoncFile + // preserves comments and pre-existing foreign servers on this JSON config. write(configPath: string, entry: McpServerEntry): void { - const config = readJson(configPath); - const servers = (config.mcpServers ?? {}) as Record; - servers[MCP_SERVER_KEY] = { + editJsoncFile(configPath, ["mcpServers", MCP_SERVER_KEY], { command: entry.command, args: entry.args, ...(hasEnv(entry) ? { env: entry.env } : {}), - }; - config.mcpServers = servers; - writeJson(configPath, config); + }); }, remove(configPath: string): boolean { if (!fs.existsSync(configPath)) return false; - const config = readJson(configPath); + const config = readJsonc(configPath); const servers = config.mcpServers as Record | undefined; if (!servers?.[MCP_SERVER_KEY]) return false; - delete servers[MCP_SERVER_KEY]; - deleteIfEmpty(config, "mcpServers"); - writeJsonOrRemove(configPath, config); + editJsoncFile(configPath, ["mcpServers", MCP_SERVER_KEY], undefined); return true; }, hasArgentEntry(configPath: string): boolean { if (!fs.existsSync(configPath)) return false; - const config = readJson(configPath); + const config = readJsonc(configPath); const servers = config.mcpServers as Record | undefined; return Boolean(servers?.[MCP_SERVER_KEY]); }, @@ -523,32 +516,28 @@ const geminiAdapter: McpConfigAdapter = { return path.join(homedir(), ".gemini", "settings.json"); }, + // JSONC-safe MCP-entry writes (see the Cursor adapter): editJsoncFile + // preserves comments and pre-existing foreign servers on this JSON config. write(configPath: string, entry: McpServerEntry): void { - const config = readJson(configPath); - const servers = (config.mcpServers ?? {}) as Record; - servers[MCP_SERVER_KEY] = { + editJsoncFile(configPath, ["mcpServers", MCP_SERVER_KEY], { command: entry.command, args: entry.args, ...(hasEnv(entry) ? { env: entry.env } : {}), - }; - config.mcpServers = servers; - writeJson(configPath, config); + }); }, remove(configPath: string): boolean { if (!fs.existsSync(configPath)) return false; - const config = readJson(configPath); + const config = readJsonc(configPath); const servers = config.mcpServers as Record | undefined; if (!servers?.[MCP_SERVER_KEY]) return false; - delete servers[MCP_SERVER_KEY]; - deleteIfEmpty(config, "mcpServers"); - writeJsonOrRemove(configPath, config); + editJsoncFile(configPath, ["mcpServers", MCP_SERVER_KEY], undefined); return true; }, hasArgentEntry(configPath: string): boolean { if (!fs.existsSync(configPath)) return false; - const config = readJson(configPath); + const config = readJsonc(configPath); const servers = config.mcpServers as Record | undefined; return Boolean(servers?.[MCP_SERVER_KEY]); }, @@ -877,32 +866,30 @@ const kiroAdapter: McpConfigAdapter = { return path.join(homedir(), ".kiro", "settings", "mcp.json"); }, + // Kiro is a VS Code fork: .kiro/settings/mcp.json is JSONC. As with Cursor, + // route write/remove/hasArgentEntry through readJsonc / editJsoncFile so + // comments and foreign servers survive instead of being flattened away by the + // old readJson → writeJson path. write(configPath: string, entry: McpServerEntry): void { - const config = readJson(configPath); - const servers = (config.mcpServers ?? {}) as Record; - servers[MCP_SERVER_KEY] = { + editJsoncFile(configPath, ["mcpServers", MCP_SERVER_KEY], { command: entry.command, args: entry.args, ...(hasEnv(entry) ? { env: entry.env } : {}), - }; - config.mcpServers = servers; - writeJson(configPath, config); + }); }, remove(configPath: string): boolean { if (!fs.existsSync(configPath)) return false; - const config = readJson(configPath); + const config = readJsonc(configPath); const servers = config.mcpServers as Record | undefined; if (!servers?.[MCP_SERVER_KEY]) return false; - delete servers[MCP_SERVER_KEY]; - deleteIfEmpty(config, "mcpServers"); - writeJsonOrRemove(configPath, config); + editJsoncFile(configPath, ["mcpServers", MCP_SERVER_KEY], undefined); return true; }, hasArgentEntry(configPath: string): boolean { if (!fs.existsSync(configPath)) return false; - const config = readJson(configPath); + const config = readJsonc(configPath); const servers = config.mcpServers as Record | undefined; return Boolean(servers?.[MCP_SERVER_KEY]); }, diff --git a/packages/argent-installer/test/mcp-configs.test.ts b/packages/argent-installer/test/mcp-configs.test.ts index ebb2d5e9a..93302b56e 100644 --- a/packages/argent-installer/test/mcp-configs.test.ts +++ b/packages/argent-installer/test/mcp-configs.test.ts @@ -1657,6 +1657,31 @@ describe("installer preserves foreign MCP config", () => { fs.rmSync(dir, { recursive: true, force: true }); }); + // Same Defect B, but for the { mcpServers } adapters: Cursor and Kiro are + // comment-tolerant VS Code forks, and Claude / Windsurf / Gemini configs are + // strict JSON that JSONC is a superset of. All five now write through + // editJsoncFile, so a hand-authored comment and a pre-existing foreign server + // survive an argent `write` — the old readJson → writeJson path reduced the + // commented file to {} and rewrote it with only the argent entry. + for (const name of ["Cursor", "Claude Code", "Windsurf", "Gemini", "Kiro"]) { + it(`${name} write preserves a comment and a foreign server in a JSONC file`, () => { + const adapter = ALL_ADAPTERS.find((a) => a.name === name)!; + const dir = fs.mkdtempSync(path.join(os.tmpdir(), "argent-fc-")); + const configPath = path.join(dir, "mcp.json"); + fs.writeFileSync( + configPath, + `{\n // my own MCP server, do not touch\n "mcpServers": { "myserver": { "command": "my-bin", "args": ["x"] } }\n}` + ); + adapter.write(configPath, getMcpEntry()); + const raw = fs.readFileSync(configPath, "utf8"); + expect(raw).toContain("// my own MCP server, do not touch"); + const after = parseJsonc(raw, [], { allowTrailingComma: true }) as Record; + expect(after.mcpServers).toHaveProperty("argent"); + expect(after.mcpServers).toHaveProperty("myserver"); + fs.rmSync(dir, { recursive: true, force: true }); + }); + } + it("Codex remove keeps an unrelated server's empty args and a sibling empty table (TOML)", () => { // Same "only touch argent" contract as the JSON adapters above, but for the // TOML-backed Codex config: writeTomlOrRemove used to deep-prune the whole From 24e55b319d8f0542dc680b2d7e050e7b3ee1c9a3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ignacy=20=C5=81=C4=85tka?= Date: Thu, 2 Jul 2026 00:41:12 +0200 Subject: [PATCH 04/10] fix(installer): make Windsurf/Gemini/Kiro allowlist edits JSONC-safe --- packages/argent-installer/src/mcp-configs.ts | 68 ++++++++------ .../argent-installer/test/mcp-configs.test.ts | 92 +++++++++++++++++++ 2 files changed, 131 insertions(+), 29 deletions(-) diff --git a/packages/argent-installer/src/mcp-configs.ts b/packages/argent-installer/src/mcp-configs.ts index 0e1483f22..9eac6e1d2 100644 --- a/packages/argent-installer/src/mcp-configs.ts +++ b/packages/argent-installer/src/mcp-configs.ts @@ -392,25 +392,27 @@ const windsurfAdapter: McpConfigAdapter = { return Boolean(servers?.[MCP_SERVER_KEY]); }, + // JSONC-safe allowlist edits (see the Cursor adapter): the argent entry lives + // in this same mcp_config.json, and `init` runs write() (comment-preserving) + // before addAllowlist(). The old readJson path choked on any user comment and + // silently skipped the toggle; editJsoncFile targets just the argent entry's + // alwaysAllow key so comments and foreign servers survive. addAllowlist(): void { const configPath = path.join(homedir(), ".codeium", "windsurf", "mcp_config.json"); - const config = readJson(configPath); - const servers = (config.mcpServers ?? {}) as Record>; - const entry = servers[MCP_SERVER_KEY]; - if (!entry) return; - entry.alwaysAllow = ["*"]; - writeJson(configPath, config); + const config = readJsonc(configPath); + const servers = config.mcpServers as Record | undefined; + if (!servers?.[MCP_SERVER_KEY]) return; + editJsoncFile(configPath, ["mcpServers", MCP_SERVER_KEY, "alwaysAllow"], ["*"]); }, removeAllowlist(): void { const configPath = path.join(homedir(), ".codeium", "windsurf", "mcp_config.json"); if (!fs.existsSync(configPath)) return; - const config = readJson(configPath); - const servers = (config.mcpServers ?? {}) as Record>; - const entry = servers[MCP_SERVER_KEY]; + const config = readJsonc(configPath); + const servers = config.mcpServers as Record> | undefined; + const entry = servers?.[MCP_SERVER_KEY]; if (!entry?.alwaysAllow) return; - delete entry.alwaysAllow; - writeJsonOrRemove(configPath, config); + editJsoncFile(configPath, ["mcpServers", MCP_SERVER_KEY, "alwaysAllow"], undefined); }, }; @@ -542,6 +544,11 @@ const geminiAdapter: McpConfigAdapter = { return Boolean(servers?.[MCP_SERVER_KEY]); }, + // JSONC-safe allowlist edits (see the Cursor adapter): the argent entry lives + // in this same settings.json, and `init` runs write() (comment-preserving) + // before addAllowlist(). The old readJson path choked on any user comment and + // silently skipped the toggle; editJsoncFile targets just the argent entry's + // trust key so comments and foreign servers survive. addAllowlist(root: string, scope: "local" | "global"): void { const configPath = scope === "global" ? this.globalPath() : this.projectPath(root); @@ -549,12 +556,10 @@ const geminiAdapter: McpConfigAdapter = { return; } - const config = readJson(configPath); - const servers = (config.mcpServers ?? {}) as Record>; - const entry = servers[MCP_SERVER_KEY]; - if (!entry) return; - entry.trust = true; - writeJson(configPath, config); + const config = readJsonc(configPath); + const servers = config.mcpServers as Record | undefined; + if (!servers?.[MCP_SERVER_KEY]) return; + editJsoncFile(configPath, ["mcpServers", MCP_SERVER_KEY, "trust"], true); }, removeAllowlist(root: string, scope: "local" | "global"): void { @@ -564,12 +569,11 @@ const geminiAdapter: McpConfigAdapter = { return; } - const config = readJson(configPath); + const config = readJsonc(configPath); const servers = config.mcpServers as Record> | undefined; const entry = servers?.[MCP_SERVER_KEY]; if (!entry?.trust) return; - delete entry.trust; - writeJsonOrRemove(configPath, config); + editJsoncFile(configPath, ["mcpServers", MCP_SERVER_KEY, "trust"], undefined); }, }; @@ -894,26 +898,32 @@ const kiroAdapter: McpConfigAdapter = { return Boolean(servers?.[MCP_SERVER_KEY]); }, + // JSONC-safe allowlist edits (see the Cursor adapter): .kiro/settings/mcp.json + // is JSONC, the argent entry lives in it, and `init` runs write() (comment- + // preserving) before addAllowlist(). The old readJson path choked on any user + // comment and silently skipped the toggle; editJsoncFile targets just the + // argent entry's autoApprove key so comments and foreign servers survive. addAllowlist(root: string, scope: "local" | "global"): void { const configPath = scope === "global" ? this.globalPath() : this.projectPath(root); if (!configPath) return; - const config = readJson(configPath); - const servers = (config.mcpServers ?? {}) as Record>; - const entry = servers[MCP_SERVER_KEY]; - if (!entry) return; - entry.autoApprove = [...KIRO_AUTO_APPROVE_ALL]; - writeJson(configPath, config); + const config = readJsonc(configPath); + const servers = config.mcpServers as Record | undefined; + if (!servers?.[MCP_SERVER_KEY]) return; + editJsoncFile( + configPath, + ["mcpServers", MCP_SERVER_KEY, "autoApprove"], + [...KIRO_AUTO_APPROVE_ALL] + ); }, removeAllowlist(root: string, scope: "local" | "global"): void { const configPath = scope === "global" ? this.globalPath() : this.projectPath(root); if (!configPath || !fs.existsSync(configPath)) return; - const config = readJson(configPath); + const config = readJsonc(configPath); const servers = config.mcpServers as Record> | undefined; const entry = servers?.[MCP_SERVER_KEY]; if (!entry?.autoApprove) return; - delete entry.autoApprove; - writeJsonOrRemove(configPath, config); + editJsoncFile(configPath, ["mcpServers", MCP_SERVER_KEY, "autoApprove"], undefined); }, }; diff --git a/packages/argent-installer/test/mcp-configs.test.ts b/packages/argent-installer/test/mcp-configs.test.ts index 93302b56e..7411185d8 100644 --- a/packages/argent-installer/test/mcp-configs.test.ts +++ b/packages/argent-installer/test/mcp-configs.test.ts @@ -241,6 +241,10 @@ describe("VS Code adapter", () => { describe("Windsurf adapter", () => { const adapter = ALL_ADAPTERS.find((a) => a.name === "Windsurf")!; + afterEach(() => { + homedirOverride = undefined; + }); + it("writes { mcpServers: { argent: ... } } without type", () => { const configPath = path.join(tmpDir, "mcp_config.json"); adapter.write(configPath, getMcpEntry()); @@ -260,6 +264,36 @@ describe("Windsurf adapter", () => { path.join(os.homedir(), ".codeium", "windsurf", "mcp_config.json") ); }); + + // The argent entry and its alwaysAllow toggle live in the same JSONC config, + // and `init` runs write() (comment-preserving) before addAllowlist(). The old + // readJson path parsed the commented file to {}, found no argent entry, and + // silently skipped the toggle. editJsoncFile applies it while keeping comments + // and foreign servers. + it("addAllowlist/removeAllowlist round-trip on a commented config (JSONC-safe)", () => { + homedirOverride = path.join(tmpDir, "home"); + const configPath = path.join(homedirOverride, ".codeium", "windsurf", "mcp_config.json"); + fs.mkdirSync(path.dirname(configPath), { recursive: true }); + fs.writeFileSync( + configPath, + `{\n // my own MCP server, do not touch\n "mcpServers": { "myserver": { "command": "my-bin", "args": ["x"] } }\n}\n` + ); + + adapter.write(configPath, getMcpEntry()); + adapter.addAllowlist!(tmpDir, "global"); + + expect(fs.readFileSync(configPath, "utf8")).toContain("// my own MCP server, do not touch"); + const servers = readJsoncFile(configPath).mcpServers as Record; + expect(servers).toHaveProperty("myserver"); + expect((servers.argent as Record).alwaysAllow).toEqual(["*"]); + + adapter.removeAllowlist!(tmpDir, "global"); + + expect(fs.readFileSync(configPath, "utf8")).toContain("// my own MCP server, do not touch"); + const servers2 = readJsoncFile(configPath).mcpServers as Record; + expect(servers2).toHaveProperty("myserver"); + expect(servers2.argent as Record).not.toHaveProperty("alwaysAllow"); + }); }); // ── Zed adapter ────────────────────────────────────────────────────────────── @@ -486,6 +520,35 @@ describe("Gemini adapter", () => { it("removeAllowlist is a no-op when file does not exist", () => { expect(() => adapter.removeAllowlist!(tmpDir, "local")).not.toThrow(); }); + + // The argent entry and its trust flag live in the same JSONC settings.json, + // and `init` runs write() (comment-preserving) before addAllowlist(). The old + // readJson path parsed the commented file to {}, found no argent entry, and + // silently skipped the toggle. editJsoncFile applies it while keeping comments + // and foreign servers. + it("addAllowlist/removeAllowlist round-trip on a commented config (JSONC-safe)", () => { + const configPath = path.join(tmpDir, ".gemini", "settings.json"); + fs.mkdirSync(path.dirname(configPath), { recursive: true }); + fs.writeFileSync( + configPath, + `{\n // my own MCP server, do not touch\n "mcpServers": { "myserver": { "command": "my-bin", "args": ["x"] } }\n}\n` + ); + + adapter.write(configPath, getMcpEntry()); + adapter.addAllowlist!(tmpDir, "local"); + + expect(fs.readFileSync(configPath, "utf8")).toContain("// my own MCP server, do not touch"); + const servers = readJsoncFile(configPath).mcpServers as Record; + expect(servers).toHaveProperty("myserver"); + expect((servers.argent as Record).trust).toBe(true); + + adapter.removeAllowlist!(tmpDir, "local"); + + expect(fs.readFileSync(configPath, "utf8")).toContain("// my own MCP server, do not touch"); + const servers2 = readJsoncFile(configPath).mcpServers as Record; + expect(servers2).toHaveProperty("myserver"); + expect(servers2.argent as Record).not.toHaveProperty("trust"); + }); }); // ── Codex adapter ──────────────────────────────────────────────────────────── @@ -1147,6 +1210,35 @@ describe("Kiro adapter", () => { it("removeAllowlist is a no-op when file does not exist", () => { expect(() => adapter.removeAllowlist!(tmpDir, "local")).not.toThrow(); }); + + // The argent entry and its autoApprove list live in the same JSONC mcp.json, + // and `init` runs write() (comment-preserving) before addAllowlist(). The old + // readJson path parsed the commented file to {}, found no argent entry, and + // silently skipped the toggle. editJsoncFile applies it while keeping comments + // and foreign servers. + it("addAllowlist/removeAllowlist round-trip on a commented config (JSONC-safe)", () => { + const configPath = path.join(tmpDir, ".kiro", "settings", "mcp.json"); + fs.mkdirSync(path.dirname(configPath), { recursive: true }); + fs.writeFileSync( + configPath, + `{\n // my own MCP server, do not touch\n "mcpServers": { "myserver": { "command": "my-bin", "args": ["x"] } }\n}\n` + ); + + adapter.write(configPath, getMcpEntry()); + adapter.addAllowlist!(tmpDir, "local"); + + expect(fs.readFileSync(configPath, "utf8")).toContain("// my own MCP server, do not touch"); + const servers = readJsoncFile(configPath).mcpServers as Record; + expect(servers).toHaveProperty("myserver"); + expect((servers.argent as Record).autoApprove).toEqual(["*"]); + + adapter.removeAllowlist!(tmpDir, "local"); + + expect(fs.readFileSync(configPath, "utf8")).toContain("// my own MCP server, do not touch"); + const servers2 = readJsoncFile(configPath).mcpServers as Record; + expect(servers2).toHaveProperty("myserver"); + expect(servers2.argent as Record).not.toHaveProperty("autoApprove"); + }); }); // ── Claude permissions ──────────────────────────────────────────────────────── From f7d82054735ad3199757ea01460537cf86e6ccbf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ignacy=20=C5=81=C4=85tka?= Date: Thu, 2 Jul 2026 12:45:24 +0200 Subject: [PATCH 05/10] fix(installer): write a trailing newline for freshly-created JSONC configs editJsoncFile (which six adapters' write() now route through) wrote the file with no trailing newline, unlike the JSON/TOML writers. A fresh `argent init` therefore created .cursor/mcp.json etc. without a final newline (git "No newline at end of file", newline-requiring linters). Append one only when the file did not exist before, so an existing file's own formatting is preserved untouched. --- packages/argent-installer/src/utils.ts | 8 +++++++- packages/argent-installer/test/mcp-configs.test.ts | 11 +++++++++++ 2 files changed, 18 insertions(+), 1 deletion(-) diff --git a/packages/argent-installer/src/utils.ts b/packages/argent-installer/src/utils.ts index 6edca0b78..d3be9b391 100644 --- a/packages/argent-installer/src/utils.ts +++ b/packages/argent-installer/src/utils.ts @@ -297,6 +297,7 @@ export function readJsonc(filePath: string): Record { * when there are no comments to preserve. */ export function editJsoncFile(filePath: string, jsonPath: JSONPath, value: unknown): void { + const existedBefore = fs.existsSync(filePath); const { text: initial, hadBom } = readJsoncFileRaw(filePath); let text = setJsoncIn(initial, jsonPath, value); @@ -317,7 +318,12 @@ export function editJsoncFile(filePath: string, jsonPath: JSONPath, value: unkno } fs.mkdirSync(path.dirname(filePath), { recursive: true }); - fs.writeFileSync(filePath, (hadBom ? "" : "") + text); + // A freshly-created config gets a trailing newline, matching writeJson/TOML + // (POSIX text-file convention, avoids a git "\ No newline at end of file"). + // An existing file's own EOL/formatting is preserved untouched — setJsoncIn + // edits in place — so only append when the file did not exist before. + const out = !existedBefore && !text.endsWith("\n") ? text + "\n" : text; + fs.writeFileSync(filePath, (hadBom ? "" : "") + out); } // ── Directory helpers ───────────────────────────────────────────────────────── diff --git a/packages/argent-installer/test/mcp-configs.test.ts b/packages/argent-installer/test/mcp-configs.test.ts index 7411185d8..8e38d8639 100644 --- a/packages/argent-installer/test/mcp-configs.test.ts +++ b/packages/argent-installer/test/mcp-configs.test.ts @@ -372,6 +372,17 @@ describe("Zed adapter", () => { expect(readJsoncFile(configPath)).not.toHaveProperty("context_servers"); }); + it("writes a freshly-created JSONC config with a trailing newline", () => { + // No pre-existing file: editJsoncFile creates it. Like writeJson/TOML, a + // freshly-created config must end with a newline (POSIX convention; avoids a + // git "\\ No newline at end of file" and satisfies newline-requiring linters). + const configPath = path.join(tmpDir, ".zed", "settings.json"); + adapter.write(configPath, getMcpEntry()); + const text = fs.readFileSync(configPath, "utf8"); + expect(text.endsWith("\n")).toBe(true); + expect(readJsoncFile(configPath).context_servers).toHaveProperty("argent"); + }); + it("removes the file when only the argent key was present", () => { const configPath = path.join(tmpDir, ".zed", "settings.json"); adapter.write(configPath, getMcpEntry()); From a4937d0afe42148a41ca73669127b11651603755 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ignacy=20=C5=81=C4=85tka?= Date: Thu, 2 Jul 2026 19:34:25 +0200 Subject: [PATCH 06/10] test(installer): pin that uninstall preserves comments + foreign servers (JSONC) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The foreign-config preservation suite covered the `write` path for comment survival but only tested `remove` against strict-JSON Cursor. Add a full install→uninstall round-trip on a hand-commented JSONC file for VS Code and the five { mcpServers } JSONC adapters (Cursor / Claude Code / Windsurf / Gemini / Kiro), asserting the user's comment and foreign server survive uninstall, argent is gone, and the file is kept when a foreign server remains — the remove side of Defect B. --- .../argent-installer/test/mcp-configs.test.ts | 45 +++++++++++++++++++ 1 file changed, 45 insertions(+) diff --git a/packages/argent-installer/test/mcp-configs.test.ts b/packages/argent-installer/test/mcp-configs.test.ts index 8e38d8639..11903d840 100644 --- a/packages/argent-installer/test/mcp-configs.test.ts +++ b/packages/argent-installer/test/mcp-configs.test.ts @@ -1785,6 +1785,51 @@ describe("installer preserves foreign MCP config", () => { }); } + // Defect B's REMOVE side: the write-preservation tests above only prove + // `write` keeps comments/foreign servers. Uninstall (`remove`) must ALSO keep + // them — a full argent install→uninstall round-trip on a hand-commented JSONC + // file must leave the user's comment and foreign server byte-intact and the + // file on disk (it still has a foreign server), with only argent gone. + it("VS Code remove preserves a comment and a foreign server (uninstall round-trip)", () => { + const adapter = ALL_ADAPTERS.find((a) => a.name === "VS Code")!; + const dir = fs.mkdtempSync(path.join(os.tmpdir(), "argent-fc-")); + const configPath = path.join(dir, ".vscode", "mcp.json"); + fs.mkdirSync(path.dirname(configPath), { recursive: true }); + fs.writeFileSync( + configPath, + `{\n // my own MCP server, do not touch\n "servers": { "myserver": { "type": "stdio", "command": "my-bin", "args": ["x"] } }\n}` + ); + adapter.write(configPath, getMcpEntry()); + expect(adapter.remove(configPath)).toBe(true); + const raw = fs.readFileSync(configPath, "utf8"); + expect(raw).toContain("// my own MCP server, do not touch"); + const after = parseJsonc(raw, [], { allowTrailingComma: true }) as Record; + expect(after.servers).not.toHaveProperty("argent"); + expect(after.servers).toHaveProperty("myserver"); + expect(fs.existsSync(configPath)).toBe(true); + fs.rmSync(dir, { recursive: true, force: true }); + }); + + for (const name of ["Cursor", "Claude Code", "Windsurf", "Gemini", "Kiro"]) { + it(`${name} remove preserves a comment and a foreign server (uninstall round-trip)`, () => { + const adapter = ALL_ADAPTERS.find((a) => a.name === name)!; + const dir = fs.mkdtempSync(path.join(os.tmpdir(), "argent-fc-")); + const configPath = path.join(dir, "mcp.json"); + fs.writeFileSync( + configPath, + `{\n // my own MCP server, do not touch\n "mcpServers": { "myserver": { "command": "my-bin", "args": ["x"] } }\n}` + ); + adapter.write(configPath, getMcpEntry()); + expect(adapter.remove(configPath)).toBe(true); + const raw = fs.readFileSync(configPath, "utf8"); + expect(raw).toContain("// my own MCP server, do not touch"); + const after = parseJsonc(raw, [], { allowTrailingComma: true }) as Record; + expect(after.mcpServers).not.toHaveProperty("argent"); + expect(after.mcpServers).toHaveProperty("myserver"); + fs.rmSync(dir, { recursive: true, force: true }); + }); + } + it("Codex remove keeps an unrelated server's empty args and a sibling empty table (TOML)", () => { // Same "only touch argent" contract as the JSON adapters above, but for the // TOML-backed Codex config: writeTomlOrRemove used to deep-prune the whole From 837c44e775bfb0b09b7e69a28aafac694d366be2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ignacy=20=C5=81=C4=85tka?= Date: Tue, 7 Jul 2026 16:12:03 +0200 Subject: [PATCH 07/10] fix(installer): route the allowlist writers through the JSONC-safe path MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The allowlist helpers were the one surface still on the readJson -> writeJson path this PR migrated everywhere else, and it clobbers foreign config: - cursorAdapter.addAllowlist read ~/.cursor/permissions.json with readJson, whose `catch { return {} }` discards a hand-commented or multi-rule file, and rewrote it with only { mcpAllowlist }, dropping every foreign rule and comment. Cursor is a comment-tolerant VS Code fork, and this became newly reachable through `update`: now that hasArgentEntry reads mcp.json with readJsonc, a commented .cursor/mcp.json is detected as configured, so update calls addAllowlist and touches permissions.json. - addClaudePermission had the same unconditional read-write shape on .claude/settings.json — a comment there would drop the user's other permissions on the next write. Route all four allowlist/permission helpers (Cursor add/removeAllowlist, add/removeClaudePermission) through readJsonc / editJsoncFile, matching the adapters this PR already migrated. editJsoncFile preserves comments and foreign keys, prunes only the edited path's empty ancestors, and deletes the file when the document collapses to {} — so removeAllowlist / removeClaudePermission keep the exact file-deletion semantics of the old deleteIfEmpty + writeJsonOrRemove chain while also working on commented files. This unifies every JSON write on editJsoncFile and lets writeJsonOrRemove (and the readJson/writeJson imports) go; deleteIfEmpty stays for the TOML-backed Codex config. Adds regression tests that fail on the pre-fix code (Cursor and Claude add + install/uninstall round-trip preserve a comment and a foreign rule/permission), plus pins three behaviors the review flagged as correct-but-untested: write() leaves an existing no-trailing-newline file untouched, re-write() replaces a stale env key rather than merging it, and remove() keeps the file and an empty top-level sibling when argent was the only server. --- packages/argent-installer/src/mcp-configs.ts | 114 +++++++------- .../argent-installer/test/mcp-configs.test.ts | 142 ++++++++++++++++++ 2 files changed, 196 insertions(+), 60 deletions(-) diff --git a/packages/argent-installer/src/mcp-configs.ts b/packages/argent-installer/src/mcp-configs.ts index 9eac6e1d2..b16132bb1 100644 --- a/packages/argent-installer/src/mcp-configs.ts +++ b/packages/argent-installer/src/mcp-configs.ts @@ -9,9 +9,7 @@ import { CURSOR_ALLOWLIST_PATTERN, } from "./constants.js"; import { - readJson, readJsonc, - writeJson, dirExists, readToml, writeToml, @@ -103,11 +101,12 @@ function isRecord(value: unknown): value is Record { return Boolean(value) && typeof value === "object" && !Array.isArray(value); } -// After a remover deletes the argent entry, collapse the now-empty container it -// lived in (e.g. an emptied `mcpServers`) so a config that held only argent -// reduces to {} and writeJsonOrRemove can delete the file. Only this one key is -// touched — foreign sibling keys and empty values elsewhere in the tree are -// preserved byte-for-byte (the contract is "only touch argent"). +// After the Codex remover deletes the argent entry, collapse the now-empty +// `mcp_servers` table it lived in so a config that held only argent reduces to {} +// and writeTomlOrRemove can delete the file. Only this one key is touched — +// foreign sibling keys and empty values elsewhere in the tree are preserved +// byte-for-byte (the contract is "only touch argent"). The JSON adapters get the +// same ancestor collapse for free from editJsoncFile. function deleteIfEmpty(parent: Record, key: string): void { const value = parent[key]; if ( @@ -120,27 +119,16 @@ function deleteIfEmpty(parent: Record, key: string): void { // Writes `data` unchanged, except: when it has no own keys the file (and an // empty parent directory) is removed instead. This deliberately does NOT -// recursively prune empty objects/arrays — the previous deep-prune silently -// stripped foreign servers' `args: []` / `env: {}` and deleted any user key -// holding an empty object/array, violating the "only touch argent" contract. -// Removers collapse their own emptied argent container via deleteIfEmpty before -// calling here, so "config held only argent" still results in file deletion. -function writeJsonOrRemove(filePath: string, data: Record): void { - if (Object.keys(data).length === 0) { - fs.rmSync(filePath, { force: true }); - removeDirIfEmpty(path.dirname(filePath)); - return; - } - - writeJson(filePath, data); -} - -// Writes `data` unchanged, except: when it has no own keys the file (and an -// empty parent directory) is removed instead. Mirrors writeJsonOrRemove (see -// its comment above) — this deliberately does NOT recursively prune empty -// tables/arrays, so a foreign TOML server's `args = []` and any sibling empty -// value survive. Callers collapse their own emptied argent container via -// deleteIfEmpty before calling here. +// recursively prune empty tables/arrays, so a foreign TOML server's `args = []` +// and any sibling empty value survive — the previous deep-prune silently +// stripped them, violating the "only touch argent" contract. Callers collapse +// their own emptied argent container via deleteIfEmpty before calling here, so +// "config held only argent" still results in file deletion. +// +// The JSON adapters achieve the same via editJsoncFile (which prunes only the +// edited path's empty ancestors and deletes the file when the document +// collapses to {}); TOML has no comment-preserving editor, so it keeps this +// parse → mutate → stringify writer. function writeTomlOrRemove(filePath: string, data: Record): void { if (Object.keys(data).length === 0) { fs.rmSync(filePath, { force: true }); @@ -202,29 +190,32 @@ const cursorAdapter: McpConfigAdapter = { return Boolean(servers?.[MCP_SERVER_KEY]); }, + // Cursor's allowlist lives in a separate ~/.cursor/permissions.json, which — + // like every Cursor config — is JSONC (comments, trailing commas) and may + // carry the user's own unrelated rules. Route through readJsonc / editJsoncFile + // so a hand-commented or multi-rule permissions.json survives: the old readJson + // path ran it through `catch { return {} }` and rewrote the whole file with + // only { mcpAllowlist }, dropping every foreign rule and comment. Newly matters + // because `hasArgentEntry` now reads mcp.json with readJsonc, so `update` + // detects a commented config as configured and calls this. addAllowlist(): void { const permPath = path.join(homedir(), ".cursor", "permissions.json"); - const config = readJson(permPath); - const list = (config.mcpAllowlist ?? []) as string[]; - if (!list.includes(CURSOR_ALLOWLIST_PATTERN)) { - list.push(CURSOR_ALLOWLIST_PATTERN); - config.mcpAllowlist = list; - writeJson(permPath, config); - } + const config = readJsonc(permPath); + const list = Array.isArray(config.mcpAllowlist) ? (config.mcpAllowlist as string[]) : []; + if (list.includes(CURSOR_ALLOWLIST_PATTERN)) return; + editJsoncFile(permPath, ["mcpAllowlist"], [...list, CURSOR_ALLOWLIST_PATTERN]); }, removeAllowlist(): void { const permPath = path.join(homedir(), ".cursor", "permissions.json"); if (!fs.existsSync(permPath)) return; - const config = readJson(permPath); - const list = config.mcpAllowlist as string[] | undefined; - if (!Array.isArray(list)) return; - const idx = list.indexOf(CURSOR_ALLOWLIST_PATTERN); - if (idx === -1) return; - list.splice(idx, 1); - config.mcpAllowlist = list; - deleteIfEmpty(config, "mcpAllowlist"); - writeJsonOrRemove(permPath, config); + const config = readJsonc(permPath); + const list = Array.isArray(config.mcpAllowlist) ? (config.mcpAllowlist as string[]) : undefined; + if (!list || !list.includes(CURSOR_ALLOWLIST_PATTERN)) return; + const next = list.filter((rule) => rule !== CURSOR_ALLOWLIST_PATTERN); + // undefined deletes the emptied key; editJsoncFile prunes it and removes the + // file if the document collapses to {} (matching the old writeJsonOrRemove). + editJsoncFile(permPath, ["mcpAllowlist"], next.length > 0 ? next : undefined); }, }; @@ -1001,15 +992,17 @@ export function addClaudePermission(root: string, scope: "local" | "global"): vo ? path.join(homedir(), ".claude", "settings.json") : path.join(root, ".claude", "settings.json"); - const config = readJson(settingsPath); + // .claude/settings.json is normally strict JSON but is comment-tolerant in + // practice; route through readJsonc / editJsoncFile so a hand-added comment + // doesn't make readJson's `catch { return {} }` drop the user's other + // permissions on write — the same unconditional read-write clobber the mcp.json + // adapters were migrated off. editJsoncFile creates the permissions.allow path + // if absent and preserves comments and foreign keys. + const config = readJsonc(settingsPath); const permissions = (config.permissions ?? {}) as Record; - const allow = (permissions.allow ?? []) as string[]; - if (!allow.includes(PERMISSION_RULE)) { - allow.push(PERMISSION_RULE); - permissions.allow = allow; - config.permissions = permissions; - writeJson(settingsPath, config); - } + const allow = Array.isArray(permissions.allow) ? (permissions.allow as string[]) : []; + if (allow.includes(PERMISSION_RULE)) return; + editJsoncFile(settingsPath, ["permissions", "allow"], [...allow, PERMISSION_RULE]); } export function removeClaudePermission(root: string, scope: "local" | "global"): void { @@ -1019,16 +1012,17 @@ export function removeClaudePermission(root: string, scope: "local" | "global"): : path.join(root, ".claude", "settings.json"); if (!fs.existsSync(settingsPath)) return; - const config = readJson(settingsPath); + const config = readJsonc(settingsPath); const permissions = config?.permissions as Record | undefined; - const allow = permissions?.allow as string[] | undefined; + const allow = permissions?.allow; if (!permissions || !Array.isArray(allow)) return; - const idx = allow.indexOf(PERMISSION_RULE); - if (idx === -1) return; - allow.splice(idx, 1); - deleteIfEmpty(permissions, "allow"); - deleteIfEmpty(config, "permissions"); - writeJsonOrRemove(settingsPath, config); + if (!allow.includes(PERMISSION_RULE)) return; + const next = (allow as string[]).filter((rule) => rule !== PERMISSION_RULE); + // undefined deletes the emptied `allow`; editJsoncFile then prunes an emptied + // `permissions` and removes the file if the document collapses to {} (matching + // the old deleteIfEmpty + writeJsonOrRemove chain), while a comment or foreign + // permission keeps the file and survives byte-intact. + editJsoncFile(settingsPath, ["permissions", "allow"], next.length > 0 ? next : undefined); } // ── Rules / Agents copy helpers ─────────────────────────────────────────────── diff --git a/packages/argent-installer/test/mcp-configs.test.ts b/packages/argent-installer/test/mcp-configs.test.ts index 11903d840..64cd67a4c 100644 --- a/packages/argent-installer/test/mcp-configs.test.ts +++ b/packages/argent-installer/test/mcp-configs.test.ts @@ -1868,4 +1868,146 @@ describe("installer preserves foreign MCP config", () => { expect(fs.existsSync(configPath)).toBe(false); fs.rmSync(dir, { recursive: true, force: true }); }); + + // ── Allowlist writers (the last readJson → writeJson clobber path) ────────── + // The allowlist helpers were the one surface left on the readJson → writeJson + // path this PR migrated everywhere else. Cursor's allowlist lives in a + // *separate* ~/.cursor/permissions.json (a JSONC file that can hold the user's + // own unrelated rules), and Claude's in .claude/settings.json — both are now + // routed through readJsonc / editJsoncFile so an install (add) no longer runs + // a commented/multi-rule file through `catch { return {} }` and rewrites it + // with only the argent entry, and an uninstall (remove) round-trips cleanly. + afterEach(() => { + homedirOverride = undefined; + }); + + it("Cursor addAllowlist preserves a comment and foreign rules in permissions.json", () => { + const cursor = ALL_ADAPTERS.find((a) => a.name === "Cursor")!; + homedirOverride = fs.mkdtempSync(path.join(os.tmpdir(), "argent-fc-home-")); + const permPath = path.join(homedirOverride, ".cursor", "permissions.json"); + fs.mkdirSync(path.dirname(permPath), { recursive: true }); + fs.writeFileSync( + permPath, + `{\n // user's own Cursor rules, do not touch\n "fileAllowlist": ["src/**"],\n "mcpAllowlist": ["other:*"]\n}\n` + ); + cursor.addAllowlist!(tmpDir, "global"); + const raw = fs.readFileSync(permPath, "utf8"); + const after = readJsoncFile(permPath); + expect(after.mcpAllowlist).toContain("argent:*"); // argent added + expect(after.mcpAllowlist).toContain("other:*"); // foreign rule survives + expect(after.fileAllowlist).toEqual(["src/**"]); // foreign key survives + expect(raw).toContain("do not touch"); // comment survives + fs.rmSync(homedirOverride, { recursive: true, force: true }); + }); + + it("Cursor addAllowlist/removeAllowlist round-trip preserves foreign rules and keeps the file", () => { + const cursor = ALL_ADAPTERS.find((a) => a.name === "Cursor")!; + homedirOverride = fs.mkdtempSync(path.join(os.tmpdir(), "argent-fc-home-")); + const permPath = path.join(homedirOverride, ".cursor", "permissions.json"); + fs.mkdirSync(path.dirname(permPath), { recursive: true }); + fs.writeFileSync(permPath, `{\n // keep me\n "mcpAllowlist": ["other:*"]\n}\n`); + cursor.addAllowlist!(tmpDir, "global"); + cursor.removeAllowlist!(tmpDir, "global"); + expect(fs.existsSync(permPath)).toBe(true); // foreign rule ⇒ file kept + const raw = fs.readFileSync(permPath, "utf8"); + const after = readJsoncFile(permPath); + expect(after.mcpAllowlist).toEqual(["other:*"]); // argent gone, foreign kept + expect(raw).toContain("keep me"); // comment survives + fs.rmSync(homedirOverride, { recursive: true, force: true }); + }); + + it("Cursor removeAllowlist deletes the file when argent was the only rule", () => { + const cursor = ALL_ADAPTERS.find((a) => a.name === "Cursor")!; + homedirOverride = fs.mkdtempSync(path.join(os.tmpdir(), "argent-fc-home-")); + const permPath = path.join(homedirOverride, ".cursor", "permissions.json"); + cursor.addAllowlist!(tmpDir, "global"); // fresh create: { mcpAllowlist: ["argent:*"] } + cursor.removeAllowlist!(tmpDir, "global"); + expect(fs.existsSync(permPath)).toBe(false); + fs.rmSync(homedirOverride, { recursive: true, force: true }); + }); + + it("addClaudePermission preserves a comment and foreign permissions in settings.json", () => { + const settingsPath = path.join(tmpDir, ".claude", "settings.json"); + fs.mkdirSync(path.dirname(settingsPath), { recursive: true }); + fs.writeFileSync( + settingsPath, + `{\n // user's own settings, do not touch\n "model": "opus",\n "permissions": { "allow": ["Bash(ls:*)"] }\n}\n` + ); + addClaudePermission(tmpDir, "local"); + const raw = fs.readFileSync(settingsPath, "utf8"); + const after = readJsoncFile(settingsPath); + const allow = (after.permissions as Record).allow as string[]; + expect(allow).toContain("mcp__argent"); // argent added + expect(allow).toContain("Bash(ls:*)"); // foreign permission survives + expect(after.model).toBe("opus"); // foreign key survives + expect(raw).toContain("do not touch"); // comment survives + }); + + it("addClaudePermission/removeClaudePermission round-trip preserves foreign permissions", () => { + const settingsPath = path.join(tmpDir, ".claude", "settings.json"); + fs.mkdirSync(path.dirname(settingsPath), { recursive: true }); + fs.writeFileSync( + settingsPath, + `{\n // keep me\n "permissions": { "allow": ["Bash(ls:*)"] }\n}\n` + ); + addClaudePermission(tmpDir, "local"); + removeClaudePermission(tmpDir, "local"); + expect(fs.existsSync(settingsPath)).toBe(true); // foreign permission ⇒ file kept + const raw = fs.readFileSync(settingsPath, "utf8"); + const after = readJsoncFile(settingsPath); + const allow = (after.permissions as Record).allow as string[]; + expect(allow).toEqual(["Bash(ls:*)"]); // argent gone, foreign kept + expect(raw).toContain("keep me"); // comment survives + }); + + // ── Behaviors correct but previously unpinned (review coverage notes) ──────── + + it("write leaves an existing file without a trailing newline untouched", () => { + const adapter = ALL_ADAPTERS.find((a) => a.name === "VS Code")!; + const configPath = path.join(tmpDir, ".vscode", "mcp.json"); + fs.mkdirSync(path.dirname(configPath), { recursive: true }); + // Deliberately no trailing newline after the closing brace. + fs.writeFileSync( + configPath, + `{\n "servers": { "myserver": { "type": "stdio", "command": "my-bin" } }\n}` + ); + adapter.write(configPath, getMcpEntry()); + const raw = fs.readFileSync(configPath, "utf8"); + // editJsoncFile only appends a newline on fresh create — an existing file's + // own EOL style is preserved, so this one stays newline-free. + expect(raw.endsWith("\n")).toBe(false); + const after = parseJsonc(raw, [], { allowTrailingComma: true }) as Record; + expect(after.servers).toHaveProperty("argent"); + expect(after.servers).toHaveProperty("myserver"); + }); + + it("re-write replaces a stale env key rather than merging it", () => { + const adapter = ALL_ADAPTERS.find((a) => a.name === "Cursor")!; + const configPath = path.join(tmpDir, ".cursor", "mcp.json"); + // A pre-#238 entry that still carried env (the update upgrade path). + adapter.write(configPath, { + command: "argent", + args: ["mcp"], + env: { ARGENT_MCP_LOG: "/tmp/x.log" }, + }); + adapter.write(configPath, getMcpEntry()); // env-less canonical entry + const after = readJsoncFile(configPath); + const argent = (after.mcpServers as Record).argent as Record; + expect(argent).not.toHaveProperty("env"); // replaced, not merged + }); + + it("remove keeps the file and an empty top-level sibling when argent was the only server", () => { + const adapter = ALL_ADAPTERS.find((a) => a.name === "Cursor")!; + const configPath = path.join(tmpDir, ".cursor", "mcp.json"); + fs.mkdirSync(path.dirname(configPath), { recursive: true }); + fs.writeFileSync( + configPath, + `{\n "mcpServers": { "argent": { "command": "argent", "args": ["mcp"] } },\n "userThing": {}\n}\n` + ); + expect(adapter.remove(configPath)).toBe(true); + expect(fs.existsSync(configPath)).toBe(true); // kept: userThing is a foreign key + const after = readJsoncFile(configPath); + expect(after).not.toHaveProperty("mcpServers"); // emptied container pruned + expect(after.userThing).toEqual({}); // empty top-level sibling survives + }); }); From d5cbb7e1714cc93c1f0137dd2e590a101d9a1467 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ignacy=20=C5=81=C4=85tka?= Date: Tue, 7 Jul 2026 16:27:51 +0200 Subject: [PATCH 08/10] fix(installer): normalize a trailing newline for empty existing JSONC configs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Routing addClaudePermission / cursor.addAllowlist through editJsoncFile lost the trailing-newline normalization the old writeJson gave for free on an empty or whitespace-only *existing* config: editJsoncFile only appended a newline when the file did not exist before, but a whitespace-only file (readJsoncFileRaw substitutes it with "{}") is synthesized fresh just like an absent file, so it was written back without a final newline — the exact git "No newline at end of file" nit the fresh-create newline handling was added to prevent. readJsoncFileRaw now reports whether the prior content was empty/whitespace, and editJsoncFile appends the newline for any synthesized document (fresh file or empty-existing) while still leaving a file with real content on its own EOL. This also drops a redundant fs.existsSync call. Adds pinning tests for this and three behaviors the review confirmed correct but unpinned: removeClaudePermission keeps a permissions.deny sibling when argent was the sole allow rule (the nested-ancestor partial-prune boundary), addClaudePermission preserves a UTF-8 BOM, and Cursor removeAllowlist prunes an emptied mcpAllowlist while keeping the file for a foreign top-level key. --- packages/argent-installer/src/utils.ts | 26 ++++---- .../argent-installer/test/mcp-configs.test.ts | 64 +++++++++++++++++++ 2 files changed, 79 insertions(+), 11 deletions(-) diff --git a/packages/argent-installer/src/utils.ts b/packages/argent-installer/src/utils.ts index d3be9b391..d6d3ea3b8 100644 --- a/packages/argent-installer/src/utils.ts +++ b/packages/argent-installer/src/utils.ts @@ -226,13 +226,16 @@ function setJsoncIn(text: string, jsonPath: JSONPath, value: unknown): string { return applyJsoncEdits(text, edits); } -function readJsoncFileRaw(filePath: string): { text: string; hadBom: boolean } { - if (!fs.existsSync(filePath)) return { text: "{}", hadBom: false }; +function readJsoncFileRaw(filePath: string): { text: string; hadBom: boolean; wasEmpty: boolean } { + if (!fs.existsSync(filePath)) return { text: "{}", hadBom: false, wasEmpty: true }; let text = fs.readFileSync(filePath, "utf8"); const hadBom = text.charCodeAt(0) === 0xfeff; if (hadBom) text = text.slice(1); - if (text.trim() === "") text = "{}"; - return { text, hadBom }; + // A whitespace-only file has no real content whose formatting we'd preserve — + // it is substituted with "{}" and synthesized fresh, like a non-existent file. + const wasEmpty = text.trim() === ""; + if (wasEmpty) text = "{}"; + return { text, hadBom, wasEmpty }; } function getAtJsoncPath(value: unknown, jsonPath: JSONPath): unknown { @@ -297,8 +300,7 @@ export function readJsonc(filePath: string): Record { * when there are no comments to preserve. */ export function editJsoncFile(filePath: string, jsonPath: JSONPath, value: unknown): void { - const existedBefore = fs.existsSync(filePath); - const { text: initial, hadBom } = readJsoncFileRaw(filePath); + const { text: initial, hadBom, wasEmpty } = readJsoncFileRaw(filePath); let text = setJsoncIn(initial, jsonPath, value); if (value === undefined) { @@ -318,11 +320,13 @@ export function editJsoncFile(filePath: string, jsonPath: JSONPath, value: unkno } fs.mkdirSync(path.dirname(filePath), { recursive: true }); - // A freshly-created config gets a trailing newline, matching writeJson/TOML - // (POSIX text-file convention, avoids a git "\ No newline at end of file"). - // An existing file's own EOL/formatting is preserved untouched — setJsoncIn - // edits in place — so only append when the file did not exist before. - const out = !existedBefore && !text.endsWith("\n") ? text + "\n" : text; + // A synthesized document — a fresh file, or an existing file that was empty / + // whitespace-only and so had no real content to preserve — gets a trailing + // newline, matching writeJson/TOML (POSIX text-file convention, avoids a git + // "\ No newline at end of file"). An existing file with real content keeps its + // own EOL/formatting untouched — setJsoncIn edits in place — so we never + // rewrite its trailing byte. + const out = wasEmpty && !text.endsWith("\n") ? text + "\n" : text; fs.writeFileSync(filePath, (hadBom ? "" : "") + out); } diff --git a/packages/argent-installer/test/mcp-configs.test.ts b/packages/argent-installer/test/mcp-configs.test.ts index 64cd67a4c..977bbfc6c 100644 --- a/packages/argent-installer/test/mcp-configs.test.ts +++ b/packages/argent-installer/test/mcp-configs.test.ts @@ -2010,4 +2010,68 @@ describe("installer preserves foreign MCP config", () => { expect(after).not.toHaveProperty("mcpServers"); // emptied container pruned expect(after.userThing).toEqual({}); // empty top-level sibling survives }); + + it("addClaudePermission writes a trailing newline when the existing settings file was empty", () => { + // Routing addClaudePermission through editJsoncFile must not regress the + // trailing-newline normalization writeJson gave for free: an empty / + // whitespace-only existing file has no real content to preserve, so it is + // synthesized fresh and gets a final newline (no git "No newline at end of + // file"). A file with real content still keeps its own EOL (test above). + const settingsPath = path.join(tmpDir, ".claude", "settings.json"); + fs.mkdirSync(path.dirname(settingsPath), { recursive: true }); + fs.writeFileSync(settingsPath, " \n"); // whitespace-only existing file + addClaudePermission(tmpDir, "local"); + const raw = fs.readFileSync(settingsPath, "utf8"); + expect(raw.endsWith("\n")).toBe(true); + const after = readJsoncFile(settingsPath); + expect((after.permissions as Record).allow).toContain("mcp__argent"); + }); + + it("removeClaudePermission keeps a permissions.deny sibling when argent was the sole allow rule", () => { + // The nested-ancestor partial-prune boundary: allow empties and is pruned, + // but permissions must be KEPT because a foreign `deny` remains (allow+deny + // is a common real Claude config). An over-prune here would silently delete + // the user's deny rules. + const settingsPath = path.join(tmpDir, ".claude", "settings.json"); + fs.mkdirSync(path.dirname(settingsPath), { recursive: true }); + fs.writeFileSync( + settingsPath, + `{\n "permissions": { "allow": ["mcp__argent"], "deny": ["Bash(rm:*)"] }\n}\n` + ); + removeClaudePermission(tmpDir, "local"); + expect(fs.existsSync(settingsPath)).toBe(true); // deny keeps the file + const after = readJsoncFile(settingsPath); + const permissions = after.permissions as Record; + expect(permissions).not.toHaveProperty("allow"); // emptied allow pruned + expect(permissions.deny).toEqual(["Bash(rm:*)"]); // sibling survives, permissions kept + }); + + it("addClaudePermission preserves a UTF-8 BOM and foreign keys", () => { + const settingsPath = path.join(tmpDir, ".claude", "settings.json"); + fs.mkdirSync(path.dirname(settingsPath), { recursive: true }); + fs.writeFileSync(settingsPath, "" + `{\n "model": "opus"\n}\n`); + addClaudePermission(tmpDir, "local"); + const raw = fs.readFileSync(settingsPath, "utf8"); + expect(raw.charCodeAt(0)).toBe(0xfeff); // BOM preserved byte-correct + const after = readJsoncFile(settingsPath); + expect(after.model).toBe("opus"); + expect((after.permissions as Record).allow).toContain("mcp__argent"); + }); + + it("Cursor removeAllowlist prunes an emptied mcpAllowlist but keeps the file for a foreign key", () => { + const cursor = ALL_ADAPTERS.find((a) => a.name === "Cursor")!; + homedirOverride = fs.mkdtempSync(path.join(os.tmpdir(), "argent-fc-home-")); + const permPath = path.join(homedirOverride, ".cursor", "permissions.json"); + fs.mkdirSync(path.dirname(permPath), { recursive: true }); + fs.writeFileSync( + permPath, + `{\n "mcpAllowlist": ["argent:*"],\n "fileAllowlist": ["src/**"]\n}\n` + ); + cursor.removeAllowlist!(tmpDir, "global"); + expect(fs.existsSync(permPath)).toBe(true); // fileAllowlist keeps the file + const after = readJsoncFile(permPath); + expect(after).not.toHaveProperty("mcpAllowlist"); // emptied array pruned + expect(after.fileAllowlist).toEqual(["src/**"]); // foreign key survives + fs.rmSync(homedirOverride, { recursive: true, force: true }); + }); }); From 4671b86956ffcdf4977f66f9bfdd39b66f3bbc3f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ignacy=20=C5=81=C4=85tka?= Date: Fri, 10 Jul 2026 13:09:18 +0200 Subject: [PATCH 09/10] fix(installer): refresh JSONC helper docs and pin commented-config detection MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - editJsoncFile / readJsonc JSDoc no longer steer callers to the removed writeJsonOrRemove or claim it is Zed-only; document it as the shared MCP-config write path (strict-JSON adapters included) and keep writeJson for whole-document rewrites that must never delete the file (~/.claude.json). - Add a findConfiguredAdapterScopes test pinning that a commented (JSONC) config is detected as configured — the behavior transition from strict readJson (which parsed it to {} and skipped it). Fails on a readJson regression. --- packages/argent-installer/src/utils.ts | 19 +++++++----- .../argent-installer/test/mcp-configs.test.ts | 29 +++++++++++++++++++ 2 files changed, 40 insertions(+), 8 deletions(-) diff --git a/packages/argent-installer/src/utils.ts b/packages/argent-installer/src/utils.ts index 9bf10159c..7a852ed90 100644 --- a/packages/argent-installer/src/utils.ts +++ b/packages/argent-installer/src/utils.ts @@ -296,9 +296,10 @@ function rmEmptyDir(dirPath: string): void { /** * Read a JSON-with-Comments file (line + block comments + trailing commas). - * Used by callers that need to inspect Zed's settings.json without the - * `JSON.parse` failure on user-authored comments. For mutations go through - * {@link editJsoncFile} instead — it preserves comments on write. + * Used across the MCP-config adapters to inspect editor config files that may + * be JSONC (Zed, Cursor, VS Code, Kiro, ...) without `JSON.parse` failing on a + * user-authored comment. For mutations go through {@link editJsoncFile} + * instead — it preserves comments on write. */ export function readJsonc(filePath: string): Record { if (!fs.existsSync(filePath)) return {}; @@ -319,12 +320,14 @@ export function readJsonc(filePath: string): Record { * * Pass `undefined` as `value` to delete the key. Empty ancestor objects are * pruned, and if the document collapses to `{}` the file (and an empty - * parent directory) is removed — mirroring the JSON `writeJsonOrRemove` - * semantics used elsewhere. + * parent directory) is removed. * - * Use this for editor settings files that are JSONC (Zed). For pure JSON - * configs go through {@link writeJson} instead — JSONC.modify is overhead - * when there are no comments to preserve. + * This is the shared write path for every MCP-config adapter — including the + * strict-JSON ones (Claude's `.mcp.json`, Windsurf, Gemini). JSONC is a + * superset of JSON, so routing them here is safe and keeps every argent entry + * on one comment- and foreign-server-preserving path. {@link writeJson} + * remains for whole-document rewrites that must never delete the file (e.g. + * `~/.claude.json`, which holds the user's OAuth state). */ export function editJsoncFile(filePath: string, jsonPath: JSONPath, value: unknown): void { const { text: initial, hadBom, wasEmpty } = readJsoncFileRaw(filePath); diff --git a/packages/argent-installer/test/mcp-configs.test.ts b/packages/argent-installer/test/mcp-configs.test.ts index 75ec9552c..bf84e9cbb 100644 --- a/packages/argent-installer/test/mcp-configs.test.ts +++ b/packages/argent-installer/test/mcp-configs.test.ts @@ -1907,6 +1907,35 @@ describe("findConfiguredAdapterScopes", () => { expect(result.map((r) => `${r.adapter.name}:${r.scope}`)).toContain("Claude Code:global"); expect(result.map((r) => r.adapter.name)).not.toContain("Hermes"); }); + + it("detects a JSONC (commented) config that strict JSON.parse would have skipped", () => { + // The behavior transition this fix is built around: hasArgentEntry reads + // through readJsonc, so a comment-carrying config — which the old strict + // readJson parsed to {} and `update` silently skipped — is now seen as + // configured. A regression back to readJson would make this file parse to + // {}, drop the argent entry, and quietly stop refreshing these users (and + // stop reaching their allowlist). No other fixture here carries a comment, + // so this is the one that pins it. + const projectRoot = path.join(tmpDir, "project"); + fs.mkdirSync(projectRoot, { recursive: true }); + + const cursor = ALL_ADAPTERS.find((a) => a.name === "Cursor")!; + const configPath = cursor.globalPath()!; + fs.mkdirSync(path.dirname(configPath), { recursive: true }); + // A line comment AND a trailing comma — both make strict JSON.parse throw. + fs.writeFileSync( + configPath, + `{\n // my Cursor MCP config, hand-edited\n "mcpServers": {\n "argent": { "command": "argent", "args": ["mcp"] },\n }\n}` + ); + + // Guard: the fixture really is invalid strict JSON, so the strict path + // would have returned {} and reported "not configured". + expect(() => JSON.parse(fs.readFileSync(configPath, "utf8"))).toThrow(); + + expect(cursor.hasArgentEntry(configPath)).toBe(true); + const result = findConfiguredAdapterScopes(ALL_ADAPTERS, projectRoot); + expect(result.map((r) => `${r.adapter.name}:${r.scope}`)).toContain("Cursor:global"); + }); }); // ── Portability: generated configs must not embed absolute home paths ─────── From 8bde13e931e9b9a3c47e1c3c943e08ee472ef4f1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ignacy=20=C5=81=C4=85tka?= Date: Fri, 10 Jul 2026 13:16:19 +0200 Subject: [PATCH 10/10] docs(installer): generalize the JSONC-helpers section banner beyond Zed --- packages/argent-installer/src/utils.ts | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/packages/argent-installer/src/utils.ts b/packages/argent-installer/src/utils.ts index 7a852ed90..f02b415dc 100644 --- a/packages/argent-installer/src/utils.ts +++ b/packages/argent-installer/src/utils.ts @@ -238,10 +238,12 @@ export function writeJson(filePath: string, data: unknown): void { } // ── JSONC helpers ──────────────────────────────────────────────────────────── -// Comment-preserving edits for editor settings files that are JSONC (Zed). -// Unlike the JSON.parse → mutate → JSON.stringify path used elsewhere, these -// helpers operate on the source string via jsonc-parser's modify(), so user -// comments, trailing commas, blank lines, and key ordering all survive. +// Comment-preserving edits for the editor config files that are JSONC (Zed, +// Cursor, VS Code, Kiro, ...) — and, since JSONC is a superset of JSON, the +// shared write path for the strict-JSON adapters too. Unlike the +// JSON.parse → mutate → JSON.stringify path, these helpers operate on the +// source string via jsonc-parser's modify(), so user comments, trailing +// commas, blank lines, and key ordering all survive. // jsonc-parser's modify() needs a formatting hint for newly-inserted keys. // Zed's bundled defaults use 2-space indentation; matching that keeps writes