fix(adapters/codex): TOML splice no longer swallows neighboring config#7
Open
JoshKappler wants to merge 1 commit into
Open
fix(adapters/codex): TOML splice no longer swallows neighboring config#7JoshKappler wants to merge 1 commit into
JoshKappler wants to merge 1 commit into
Conversation
The block-end scan only recognized bare [header] lines, so a header with an inline comment or an [[array-of-tables]] header below the graphctx block did not end it: uninstall and re-install deleted everything up to the next bare header (verified: it removed a user's [mcp_servers.exa] block including its api_key). The find was equally strict, so a user comment on our own header made install append a duplicate [mcp_servers.graphctx] table (invalid TOML). Uninstall also orphaned [mcp_servers.graphctx.env] subtables, which implicitly re-create the server table. - src/adapters/codex/index.ts: replace the next-header regex with a line-aware scan (tableHeaderKey): tolerates leading whitespace, inline comments, [[array]] tables, and CRLF. Uninstall removes the block plus its graphctx.* subtables; re-install replaces only the main block so a user's env subtable survives updates. Trailing blank and comment-only lines stay with the following table, so a user's comment above their next server is untouched. - test/adapters/codex.test.ts: 7 new tests: commented next header, [[array-of-tables]], reinstall not swallowing servers below, env subtable removed on uninstall / preserved on reinstall, annotated graphctx header replaced not duplicated, CRLF config. - README.md + docs/STATUS.md: test count 292 -> 299. Verified: npx tsc --noEmit clean; npx biome check src test clean; npx vitest run test/adapters/codex.test.ts 13/13; full npx vitest run 299 tests with only the 12 pre-existing win32 failures on this Windows box (.cmd execFile + AF_UNIX socket, untouched here; same set fails on unmodified main).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The block-end scan in the codex adapter only recognizes bare
[header]lines. A header with an inline comment ([mcp_servers.exa] # search) or an[[array-of-tables]]header below the graphctx block doesn't end it, so uninstall and reinstall delete everything from the graphctx header down to the next bare header. My repro removed a user's[mcp_servers.exa]server, API key included. Two related edges: a user comment on the graphctx header itself makes install append a duplicate[mcp_servers.graphctx]table (invalid TOML, Codex won't parse it), and uninstall leaves[mcp_servers.graphctx.env]subtables behind, which implicitly re-create the server table.First install is safe (graphctx appends itself last). The trigger is anything added below the block, then an uninstall or a reinstall in place.
Changes:
src/adapters/codex/index.ts: replaced the next-header regex with a line-aware scan (tableHeaderKey). It handles leading whitespace, inline comments,[[array]]tables, and CRLF. Uninstall removes the block plus itsgraphctx.*subtables so nothing orphaned re-creates the server. Reinstall replaces only the main block, so a user's env subtable survives updates. Trailing blank and comment-only lines stay with the following table, so a comment above the user's next server is untouched.test/adapters/codex.test.ts: 7 new tests. Commented next header,[[array-of-tables]], reinstall not swallowing servers below, env subtable removed on uninstall and preserved on reinstall, annotated graphctx header replaced instead of duplicated, CRLF config.README.md+docs/STATUS.md: test count 292 -> 299.Verification (Windows 11, Node 22):
Note: all three PRs bump the README/STATUS test counters off the same base (292), so whichever merges second and third will conflict there. I'll rebase the counters as each one lands.