diff --git a/src/adapters/codex/index.ts b/src/adapters/codex/index.ts index d475b6b..3822df1 100644 --- a/src/adapters/codex/index.ts +++ b/src/adapters/codex/index.ts @@ -82,16 +82,52 @@ interface BlockSpan { end: number; } -function findMcpServerBlock(text: string, name: string): BlockSpan | null { - const headerRe = new RegExp(`^\\[mcp_servers\\.${escapeRegex(name)}\\]\\s*$`, "m"); - const m = headerRe.exec(text); - if (!m) return null; - const start = m.index; - // Block ends at the next top-level `[...]` header or end-of-file. - const after = text.slice(start + m[0].length); - const nextHeader = /^\[[^\]]+\]\s*$/m.exec(after); - const blockLen = nextHeader ? m[0].length + nextHeader.index : text.length - start; - return { start, end: start + blockLen }; +// Recognizes a TOML table (`[key]`) or array-of-tables (`[[key]]`) header +// line, tolerating leading whitespace, an inline trailing comment, and CRLF. +// Returns the dotted key with whitespace around dots trimmed, or null when the +// line is not a header. Quoted key segments containing `]` or `#` are outside +// this grammar (we never write them; unrecognized lines stay untouched). +function tableHeaderKey(line: string): string | null { + const m = /^\s*(\[\[|\[)([^\]]+)(\]\]|\])\s*(?:#.*)?$/.exec(line.replace(/\r$/, "")); + const key = m?.[2]; + if (!m || key === undefined) return null; + if ((m[1] === "[[") !== (m[3] === "]]")) return null; + return key + .split(".") + .map((part) => part.trim()) + .join("."); +} + +// Finds the span of the `[mcp_servers.]` table via a line scan (a bare +// next-header regex misses commented headers and `[[array]]` tables, which +// made the block swallow whatever followed it). `withSubtables` extends the +// span across `[mcp_servers..*]` subtables: uninstall removes them so +// no orphaned subtable re-creates the server table; upsert leaves them alone +// so a user's env subtable survives updates. The span ends before trailing +// blank/comment-only lines so a comment attached to the next table survives. +function findMcpServerBlock(text: string, name: string, withSubtables = false): BlockSpan | null { + const owned = `mcp_servers.${name}`; + let start = -1; + let end = -1; + let offset = 0; + for (const line of text.split("\n")) { + const lineEnd = Math.min(offset + line.length + 1, text.length); + const key = tableHeaderKey(line); + if (start < 0) { + if (key === owned) { + start = offset; + end = lineEnd; + } + } else if (key !== null && key !== owned && !(withSubtables && key.startsWith(`${owned}.`))) { + break; + } else { + const bare = line.replace(/\r$/, "").trim(); + if (bare.length > 0 && !bare.startsWith("#")) end = lineEnd; + } + offset += line.length + 1; + } + if (start < 0) return null; + return { start, end }; } function upsertCodexMcpServer(text: string, name: string, command: string, args: string[]): string { @@ -102,14 +138,16 @@ function upsertCodexMcpServer(text: string, name: string, command: string, args: const sep = base.length > 0 && !base.endsWith("\n\n") ? "\n" : ""; return `${base}${sep}${block}`; } - return `${text.slice(0, existing.start)}${block}${text.slice(existing.end).replace(/^\n+/, "\n")}`; + return `${text.slice(0, existing.start)}${block}${text.slice(existing.end).replace(/^(?:\r?\n)+/, "\n")}`; } function removeCodexMcpServer(text: string, name: string): string { - const existing = findMcpServerBlock(text, name); + const existing = findMcpServerBlock(text, name, true); if (!existing) return text; - const before = text.slice(0, existing.start).replace(/\n+$/, "\n"); - const after = text.slice(existing.end).replace(/^\n+/, ""); + const before = text + .slice(0, existing.start) + .replace(/(?:\r?\n)+$/, (m) => (m.startsWith("\r") ? "\r\n" : "\n")); + const after = text.slice(existing.end).replace(/^(?:\r?\n)+/, ""); if (before.length === 0) return after; if (after.length === 0) return before; return `${before}\n${after}`; @@ -126,10 +164,6 @@ function tomlString(s: string): string { return `"${escaped}"`; } -function escapeRegex(s: string): string { - return s.replace(/[.*+?^${}()|[\]\\]/g, "\\$&"); -} - // Exposed for tests. export const __internals = { findMcpServerBlock, diff --git a/test/adapters/codex.test.ts b/test/adapters/codex.test.ts index 2d786f4..c95cc41 100644 --- a/test/adapters/codex.test.ts +++ b/test/adapters/codex.test.ts @@ -100,3 +100,182 @@ describe("codex adapter", () => { expect(cap.highest).toBe(1); }); }); + +describe("codex adapter TOML splicing against real-world configs", () => { + const cfgPath = () => join(homeDir, ".codex", "config.toml"); + const writeCfg = (text: string) => { + mkdirSync(join(homeDir, ".codex"), { recursive: true }); + writeFileSync(cfgPath(), text, "utf8"); + }; + + it("uninstall keeps a following server whose header carries an inline comment", async () => { + writeCfg( + [ + 'model = "gpt-5"', + "", + "[mcp_servers.graphctx]", + 'command = "graphctx"', + 'args = ["serve", "--mcp"]', + "", + "# web search server", + "[mcp_servers.exa] # keep me", + 'url = "https://mcp.exa.ai/mcp"', + 'api_key = "exa-key-123"', + "", + ].join("\n"), + ); + + await new CodexAdapter(workDir, homeDir).uninstall(); + + const text = readFileSync(cfgPath(), "utf8"); + expect(text).toContain('model = "gpt-5"'); + expect(text).toContain("# web search server"); + expect(text).toContain("[mcp_servers.exa] # keep me"); + expect(text).toContain('api_key = "exa-key-123"'); + expect(text).not.toContain("mcp_servers.graphctx"); + }); + + it("uninstall keeps a following array-of-tables section", async () => { + writeCfg( + [ + "[mcp_servers.graphctx]", + 'command = "graphctx"', + 'args = ["serve", "--mcp"]', + "", + "[[profiles]]", + 'name = "work"', + "", + "[[profiles]]", + 'name = "home"', + "", + ].join("\n"), + ); + + await new CodexAdapter(workDir, homeDir).uninstall(); + + const text = readFileSync(cfgPath(), "utf8"); + expect(text.match(/^\[\[profiles\]\]/gm)).toHaveLength(2); + expect(text).toContain('name = "work"'); + expect(text).toContain('name = "home"'); + expect(text).not.toContain("mcp_servers.graphctx"); + }); + + it("reinstall replaces the block without swallowing servers below it", async () => { + writeCfg( + [ + "[mcp_servers.graphctx]", + 'command = "graphctx"', + 'args = ["serve", "--mcp"]', + "", + "[mcp_servers.exa] # added after graphctx", + 'url = "https://mcp.exa.ai/mcp"', + "", + ].join("\n"), + ); + + await new CodexAdapter(workDir, homeDir).install({ + workspaceDir: workDir, + binPath: "/abs/path/to/graphctx", + }); + + const text = readFileSync(cfgPath(), "utf8"); + expect(text).toContain("[mcp_servers.exa] # added after graphctx"); + expect(text).toContain('url = "https://mcp.exa.ai/mcp"'); + expect(text).toContain('command = "/abs/path/to/graphctx"'); + expect(text.match(/\[mcp_servers\.graphctx\]/g)).toHaveLength(1); + }); + + it("uninstall removes the graphctx env subtable along with the block", async () => { + writeCfg( + [ + "[mcp_servers.graphctx]", + 'command = "graphctx"', + 'args = ["serve", "--mcp"]', + "", + "[mcp_servers.graphctx.env]", + 'GRAPHCTX_INJECT_TOTAL_BUDGET_TOKENS = "900"', + "", + "[mcp_servers.other]", + 'command = "other"', + "", + ].join("\n"), + ); + + await new CodexAdapter(workDir, homeDir).uninstall(); + + const text = readFileSync(cfgPath(), "utf8"); + expect(text).not.toContain("mcp_servers.graphctx"); + expect(text).not.toContain("GRAPHCTX_INJECT_TOTAL_BUDGET_TOKENS"); + expect(text).toContain("[mcp_servers.other]"); + expect(text).toContain('command = "other"'); + }); + + it("reinstall preserves a user's graphctx env subtable", async () => { + writeCfg( + [ + "[mcp_servers.graphctx]", + 'command = "graphctx"', + 'args = ["serve", "--mcp"]', + "", + "[mcp_servers.graphctx.env]", + 'GRAPHCTX_INJECT_TOTAL_BUDGET_TOKENS = "900"', + "", + ].join("\n"), + ); + + await new CodexAdapter(workDir, homeDir).install({ + workspaceDir: workDir, + binPath: "/abs/path/to/graphctx", + }); + + const text = readFileSync(cfgPath(), "utf8"); + expect(text).toContain('command = "/abs/path/to/graphctx"'); + expect(text).toContain("[mcp_servers.graphctx.env]"); + expect(text).toContain('GRAPHCTX_INJECT_TOTAL_BUDGET_TOKENS = "900"'); + expect(text.match(/\[mcp_servers\.graphctx\]/g)).toHaveLength(1); + }); + + it("reinstall replaces the block when the user annotated our header, instead of duplicating it", async () => { + writeCfg( + [ + "[mcp_servers.graphctx] # managed by graphctx", + 'command = "graphctx"', + 'args = ["serve", "--mcp"]', + "", + ].join("\n"), + ); + + await new CodexAdapter(workDir, homeDir).install({ + workspaceDir: workDir, + binPath: "/abs/path/to/graphctx", + }); + + const text = readFileSync(cfgPath(), "utf8"); + expect(text.match(/\[mcp_servers\.graphctx\]/g)).toHaveLength(1); + expect(text).toContain('command = "/abs/path/to/graphctx"'); + }); + + it("uninstall keeps servers below the block in a CRLF config", async () => { + writeCfg( + [ + 'model = "gpt-5"', + "", + "[mcp_servers.graphctx]", + 'command = "graphctx"', + 'args = ["serve", "--mcp"]', + "", + "[mcp_servers.exa] # crlf config", + 'api_key = "exa-key-123"', + "", + ].join("\r\n"), + ); + + await new CodexAdapter(workDir, homeDir).uninstall(); + + const text = readFileSync(cfgPath(), "utf8"); + expect(text).toContain('model = "gpt-5"'); + expect(text).toContain("[mcp_servers.exa] # crlf config"); + expect(text).toContain('api_key = "exa-key-123"'); + expect(text).not.toContain("mcp_servers.graphctx"); + }); +});