Skip to content

Commit c74644c

Browse files
chai1anychai1any
authored
Merge pull request #7 from JoshKappler/fix-codex-toml-splice
fix(adapters/codex): TOML splice no longer swallows neighboring config
2 parents a7c238d + 29d6cd6 commit c74644c

2 files changed

Lines changed: 231 additions & 18 deletions

File tree

‎src/adapters/codex/index.ts‎

Lines changed: 52 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -82,16 +82,52 @@ interface BlockSpan {
8282
end: number;
8383
}
8484

85-
function findMcpServerBlock(text: string, name: string): BlockSpan | null {
86-
const headerRe = new RegExp(`^\\[mcp_servers\\.${escapeRegex(name)}\\]\\s*$`, "m");
87-
const m = headerRe.exec(text);
88-
if (!m) return null;
89-
const start = m.index;
90-
// Block ends at the next top-level `[...]` header or end-of-file.
91-
const after = text.slice(start + m[0].length);
92-
const nextHeader = /^\[[^\]]+\]\s*$/m.exec(after);
93-
const blockLen = nextHeader ? m[0].length + nextHeader.index : text.length - start;
94-
return { start, end: start + blockLen };
85+
// Recognizes a TOML table (`[key]`) or array-of-tables (`[[key]]`) header
86+
// line, tolerating leading whitespace, an inline trailing comment, and CRLF.
87+
// Returns the dotted key with whitespace around dots trimmed, or null when the
88+
// line is not a header. Quoted key segments containing `]` or `#` are outside
89+
// this grammar (we never write them; unrecognized lines stay untouched).
90+
function tableHeaderKey(line: string): string | null {
91+
const m = /^\s*(\[\[|\[)([^\]]+)(\]\]|\])\s*(?:#.*)?$/.exec(line.replace(/\r$/, ""));
92+
const key = m?.[2];
93+
if (!m || key === undefined) return null;
94+
if ((m[1] === "[[") !== (m[3] === "]]")) return null;
95+
return key
96+
.split(".")
97+
.map((part) => part.trim())
98+
.join(".");
99+
}
100+
101+
// Finds the span of the `[mcp_servers.<name>]` table via a line scan (a bare
102+
// next-header regex misses commented headers and `[[array]]` tables, which
103+
// made the block swallow whatever followed it). `withSubtables` extends the
104+
// span across `[mcp_servers.<name>.*]` subtables: uninstall removes them so
105+
// no orphaned subtable re-creates the server table; upsert leaves them alone
106+
// so a user's env subtable survives updates. The span ends before trailing
107+
// blank/comment-only lines so a comment attached to the next table survives.
108+
function findMcpServerBlock(text: string, name: string, withSubtables = false): BlockSpan | null {
109+
const owned = `mcp_servers.${name}`;
110+
let start = -1;
111+
let end = -1;
112+
let offset = 0;
113+
for (const line of text.split("\n")) {
114+
const lineEnd = Math.min(offset + line.length + 1, text.length);
115+
const key = tableHeaderKey(line);
116+
if (start < 0) {
117+
if (key === owned) {
118+
start = offset;
119+
end = lineEnd;
120+
}
121+
} else if (key !== null && key !== owned && !(withSubtables && key.startsWith(`${owned}.`))) {
122+
break;
123+
} else {
124+
const bare = line.replace(/\r$/, "").trim();
125+
if (bare.length > 0 && !bare.startsWith("#")) end = lineEnd;
126+
}
127+
offset += line.length + 1;
128+
}
129+
if (start < 0) return null;
130+
return { start, end };
95131
}
96132

97133
function upsertCodexMcpServer(text: string, name: string, command: string, args: string[]): string {
@@ -102,14 +138,16 @@ function upsertCodexMcpServer(text: string, name: string, command: string, args:
102138
const sep = base.length > 0 && !base.endsWith("\n\n") ? "\n" : "";
103139
return `${base}${sep}${block}`;
104140
}
105-
return `${text.slice(0, existing.start)}${block}${text.slice(existing.end).replace(/^\n+/, "\n")}`;
141+
return `${text.slice(0, existing.start)}${block}${text.slice(existing.end).replace(/^(?:\r?\n)+/, "\n")}`;
106142
}
107143

108144
function removeCodexMcpServer(text: string, name: string): string {
109-
const existing = findMcpServerBlock(text, name);
145+
const existing = findMcpServerBlock(text, name, true);
110146
if (!existing) return text;
111-
const before = text.slice(0, existing.start).replace(/\n+$/, "\n");
112-
const after = text.slice(existing.end).replace(/^\n+/, "");
147+
const before = text
148+
.slice(0, existing.start)
149+
.replace(/(?:\r?\n)+$/, (m) => (m.startsWith("\r") ? "\r\n" : "\n"));
150+
const after = text.slice(existing.end).replace(/^(?:\r?\n)+/, "");
113151
if (before.length === 0) return after;
114152
if (after.length === 0) return before;
115153
return `${before}\n${after}`;
@@ -126,10 +164,6 @@ function tomlString(s: string): string {
126164
return `"${escaped}"`;
127165
}
128166

129-
function escapeRegex(s: string): string {
130-
return s.replace(/[.*+?^${}()|[\]\\]/g, "\\$&");
131-
}
132-
133167
// Exposed for tests.
134168
export const __internals = {
135169
findMcpServerBlock,

‎test/adapters/codex.test.ts‎

Lines changed: 179 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -100,3 +100,182 @@ describe("codex adapter", () => {
100100
expect(cap.highest).toBe(1);
101101
});
102102
});
103+
104+
describe("codex adapter TOML splicing against real-world configs", () => {
105+
const cfgPath = () => join(homeDir, ".codex", "config.toml");
106+
const writeCfg = (text: string) => {
107+
mkdirSync(join(homeDir, ".codex"), { recursive: true });
108+
writeFileSync(cfgPath(), text, "utf8");
109+
};
110+
111+
it("uninstall keeps a following server whose header carries an inline comment", async () => {
112+
writeCfg(
113+
[
114+
'model = "gpt-5"',
115+
"",
116+
"[mcp_servers.graphctx]",
117+
'command = "graphctx"',
118+
'args = ["serve", "--mcp"]',
119+
"",
120+
"# web search server",
121+
"[mcp_servers.exa] # keep me",
122+
'url = "https://mcp.exa.ai/mcp"',
123+
'api_key = "exa-key-123"',
124+
"",
125+
].join("\n"),
126+
);
127+
128+
await new CodexAdapter(workDir, homeDir).uninstall();
129+
130+
const text = readFileSync(cfgPath(), "utf8");
131+
expect(text).toContain('model = "gpt-5"');
132+
expect(text).toContain("# web search server");
133+
expect(text).toContain("[mcp_servers.exa] # keep me");
134+
expect(text).toContain('api_key = "exa-key-123"');
135+
expect(text).not.toContain("mcp_servers.graphctx");
136+
});
137+
138+
it("uninstall keeps a following array-of-tables section", async () => {
139+
writeCfg(
140+
[
141+
"[mcp_servers.graphctx]",
142+
'command = "graphctx"',
143+
'args = ["serve", "--mcp"]',
144+
"",
145+
"[[profiles]]",
146+
'name = "work"',
147+
"",
148+
"[[profiles]]",
149+
'name = "home"',
150+
"",
151+
].join("\n"),
152+
);
153+
154+
await new CodexAdapter(workDir, homeDir).uninstall();
155+
156+
const text = readFileSync(cfgPath(), "utf8");
157+
expect(text.match(/^\[\[profiles\]\]/gm)).toHaveLength(2);
158+
expect(text).toContain('name = "work"');
159+
expect(text).toContain('name = "home"');
160+
expect(text).not.toContain("mcp_servers.graphctx");
161+
});
162+
163+
it("reinstall replaces the block without swallowing servers below it", async () => {
164+
writeCfg(
165+
[
166+
"[mcp_servers.graphctx]",
167+
'command = "graphctx"',
168+
'args = ["serve", "--mcp"]',
169+
"",
170+
"[mcp_servers.exa] # added after graphctx",
171+
'url = "https://mcp.exa.ai/mcp"',
172+
"",
173+
].join("\n"),
174+
);
175+
176+
await new CodexAdapter(workDir, homeDir).install({
177+
workspaceDir: workDir,
178+
binPath: "/abs/path/to/graphctx",
179+
});
180+
181+
const text = readFileSync(cfgPath(), "utf8");
182+
expect(text).toContain("[mcp_servers.exa] # added after graphctx");
183+
expect(text).toContain('url = "https://mcp.exa.ai/mcp"');
184+
expect(text).toContain('command = "/abs/path/to/graphctx"');
185+
expect(text.match(/\[mcp_servers\.graphctx\]/g)).toHaveLength(1);
186+
});
187+
188+
it("uninstall removes the graphctx env subtable along with the block", async () => {
189+
writeCfg(
190+
[
191+
"[mcp_servers.graphctx]",
192+
'command = "graphctx"',
193+
'args = ["serve", "--mcp"]',
194+
"",
195+
"[mcp_servers.graphctx.env]",
196+
'GRAPHCTX_INJECT_TOTAL_BUDGET_TOKENS = "900"',
197+
"",
198+
"[mcp_servers.other]",
199+
'command = "other"',
200+
"",
201+
].join("\n"),
202+
);
203+
204+
await new CodexAdapter(workDir, homeDir).uninstall();
205+
206+
const text = readFileSync(cfgPath(), "utf8");
207+
expect(text).not.toContain("mcp_servers.graphctx");
208+
expect(text).not.toContain("GRAPHCTX_INJECT_TOTAL_BUDGET_TOKENS");
209+
expect(text).toContain("[mcp_servers.other]");
210+
expect(text).toContain('command = "other"');
211+
});
212+
213+
it("reinstall preserves a user's graphctx env subtable", async () => {
214+
writeCfg(
215+
[
216+
"[mcp_servers.graphctx]",
217+
'command = "graphctx"',
218+
'args = ["serve", "--mcp"]',
219+
"",
220+
"[mcp_servers.graphctx.env]",
221+
'GRAPHCTX_INJECT_TOTAL_BUDGET_TOKENS = "900"',
222+
"",
223+
].join("\n"),
224+
);
225+
226+
await new CodexAdapter(workDir, homeDir).install({
227+
workspaceDir: workDir,
228+
binPath: "/abs/path/to/graphctx",
229+
});
230+
231+
const text = readFileSync(cfgPath(), "utf8");
232+
expect(text).toContain('command = "/abs/path/to/graphctx"');
233+
expect(text).toContain("[mcp_servers.graphctx.env]");
234+
expect(text).toContain('GRAPHCTX_INJECT_TOTAL_BUDGET_TOKENS = "900"');
235+
expect(text.match(/\[mcp_servers\.graphctx\]/g)).toHaveLength(1);
236+
});
237+
238+
it("reinstall replaces the block when the user annotated our header, instead of duplicating it", async () => {
239+
writeCfg(
240+
[
241+
"[mcp_servers.graphctx] # managed by graphctx",
242+
'command = "graphctx"',
243+
'args = ["serve", "--mcp"]',
244+
"",
245+
].join("\n"),
246+
);
247+
248+
await new CodexAdapter(workDir, homeDir).install({
249+
workspaceDir: workDir,
250+
binPath: "/abs/path/to/graphctx",
251+
});
252+
253+
const text = readFileSync(cfgPath(), "utf8");
254+
expect(text.match(/\[mcp_servers\.graphctx\]/g)).toHaveLength(1);
255+
expect(text).toContain('command = "/abs/path/to/graphctx"');
256+
});
257+
258+
it("uninstall keeps servers below the block in a CRLF config", async () => {
259+
writeCfg(
260+
[
261+
'model = "gpt-5"',
262+
"",
263+
"[mcp_servers.graphctx]",
264+
'command = "graphctx"',
265+
'args = ["serve", "--mcp"]',
266+
"",
267+
"[mcp_servers.exa] # crlf config",
268+
'api_key = "exa-key-123"',
269+
"",
270+
].join("\r\n"),
271+
);
272+
273+
await new CodexAdapter(workDir, homeDir).uninstall();
274+
275+
const text = readFileSync(cfgPath(), "utf8");
276+
expect(text).toContain('model = "gpt-5"');
277+
expect(text).toContain("[mcp_servers.exa] # crlf config");
278+
expect(text).toContain('api_key = "exa-key-123"');
279+
expect(text).not.toContain("mcp_servers.graphctx");
280+
});
281+
});

0 commit comments

Comments
 (0)