From e5997a45db7f7ff2a342f98d57369435764653c1 Mon Sep 17 00:00:00 2001 From: Josh Kappler Date: Sat, 4 Jul 2026 01:12:53 -0700 Subject: [PATCH] fix(adapters/codex): TOML splice no longer swallows neighboring config 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). --- README.md | 2 +- docs/STATUS.md | 6 +- src/adapters/codex/index.ts | 70 ++++++++++---- test/adapters/codex.test.ts | 179 ++++++++++++++++++++++++++++++++++++ 4 files changed, 235 insertions(+), 22 deletions(-) diff --git a/README.md b/README.md index 280762a..dcba52a 100644 --- a/README.md +++ b/README.md @@ -202,7 +202,7 @@ npx tsx src/cli.ts bench Expected current state: -- `npx vitest run`: 292 tests pass. +- `npx vitest run`: 299 tests pass. - `npm run pack:smoke`: packed tarball installs, runs the demo, and serves MCP from a clean temp app. - `npx tsx src/cli.ts eval all`: 19 gate suites pass. diff --git a/docs/STATUS.md b/docs/STATUS.md index fa0566b..d5e849d 100644 --- a/docs/STATUS.md +++ b/docs/STATUS.md @@ -76,9 +76,9 @@ |---|---|---| | `hook ` p95 | < 150ms | 15.51ms ✅ | -_Last updated: Private-key scanning now covers DSA, encrypted PEM, and PGP private key block headers, MCP `resolve_conflict` now evaluates user, workspace, and current-session facts together, `graphctx resolve` now refuses non-open-loop facts instead of silently superseding durable memory, Storage migration 0006 adds a composite invalidation conflict lookup index, Session-scoped invalidation now keeps identical open loops isolated across sessions while deduping repeats within a session, Demo seed memory now uses Runtime.rememberFact, open-loop session memory now carries current git anchors, CLI `tui --tab` now fails fast on invalid tabs, CLI/MCP fact-kind validation uses a shared `FACT_KINDS` domain enum, TUI promotion uses Runtime/Probation gates instead of direct activation, Runtime/TUI/CLI/MCP explicit remember paths share safe git-anchored invalidating writes, Runtime/TUI/MCP forget paths share git-anchored temporal closeout, MCP forget stamps git closeout anchors, retrieval candidates are scope-filtered before semantic reranking, retrieval candidates receive final workspace/session scope filtering, superseded facts close temporal validity with `t_expired` and commit anchors, retrieval recency ranking uses the runtime clock, ranking/conflict tests use deterministic fact IDs and clocks, vector embedding cache uses the runtime clock, outcome telemetry uses an injectable clock, injection anti-repetition ledger uses the runtime clock, promotion verification timestamps use the runtime clock, promotion gates rescan secret-shaped fact identity, why renders invalidating commit closeout anchors, revert revalidation respects branch representation, revert revalidation skips bad historical anchors, atomic temporal lifecycle closeout anchors, TUI read model redacts secret fact text, conflict notes redact identity/object secrets, structured why redacts evidence chain metadata, structured why redacts fact identity and scope strings, structured why redacts promotion audit text, structured why redacts git anchors, structured why redacts evidence payloads and tags, why output shows expired/invalidated temporal closeout, send-edge safety scans fact tags for secrets, command cards avoid false HEAD verification claims, low-trust claims blocked from PreToolUse guardrail injection, candidate facts kept out of vector search until promotion, lifecycle-synced vector index entries for expired/reactivated facts, secret-redacted semantic rerank text, retriever-level send-unsafe fact suppression, tag-update secret sensitivity restamping, secret-redacted FTS/vector fact indexing, FTS path-query fail-soft handling, runtime-level injection context redaction, hook tool-result redaction before injection context, shared retrieval-context redaction for CLI/MCP/hooks, atomic invalidation effects, supported-by temporal evidence edges, tool-arg redaction before retrieval, hook prompt redaction before retrieval, transcript-tail redaction before retrieval, symlink-safe workspace config reads, symlink-safe local store paths, symlink-safe AGENTS.md boot capsule writes, symlink-safe generic adapter marker writes, symlink-safe OpenCode config writes, symlink-safe Cursor mcp writes, symlink-safe Claude settings writes, workspace-confined CI workflow extraction, workspace-confined agent prose extraction, workspace-confined Docker/Compose extraction, workspace-confined test config extraction, workspace-confined tooling config extraction, workspace-confined tsconfig extraction, workspace-confined editorconfig extraction, workspace-confined runtime version extraction, workspace-confined lockfile extraction, workspace-confined package script evidence, shared realpath workspace evidence checks, symlink-aware injection staleness checks, generated-marker symlink skip, procedure verifier secret scanning, extraction subject secret scanning, hook session-id redaction, MCP session-reference secret refusal, open-loop session metadata secret refusal, explicit memory metadata secret refusal, packageManager canonical facts, package-manager-aware script extraction, runtime-version deterministic extraction, multi-cookie session redaction, MCP error redaction, fail-closed retrieval for unvalidated commit-scoped facts, semantic CLI recall ranking, auth/cookie header secret scanning, scoped semantic retrieval expansion, workspace-confined injection staleness checks, package metadata extraction, high-trust test/Docker/tooling/tsconfig deterministic extraction, high-trust Python toolchain extraction, high-trust pyproject package-manager extraction, pyproject dependency-group Python command extraction, high-entropy redaction hardening, repo-id-isolated temporal validity, measured hook latency, fail-closed adapter install/uninstall, typed CLI error formatting, answer-bearing coding query expansion, Python package-manager retrieval expansion, redacted TUI selected-fact detail panels, TUI page/jump keyboard navigation, composed TUI empty states with operator status surfaces, SessionStart suppression of superseded user preferences, chunk-safe TUI key decoding with left/right and j/k navigation, bounded secret-redacted TUI prompt input, vector-safe commit-hygiene query expansion, searchable TUI memory lists, and tox/nox Python test harness extraction. 292 tests, 19 gate suites green, all I1-I9 hold._ +_Last updated: Codex config splicing now survives commented table headers, array-of-tables sections, and graphctx env subtables without touching neighboring config, Private-key scanning now covers DSA, encrypted PEM, and PGP private key block headers, MCP `resolve_conflict` now evaluates user, workspace, and current-session facts together, `graphctx resolve` now refuses non-open-loop facts instead of silently superseding durable memory, Storage migration 0006 adds a composite invalidation conflict lookup index, Session-scoped invalidation now keeps identical open loops isolated across sessions while deduping repeats within a session, Demo seed memory now uses Runtime.rememberFact, open-loop session memory now carries current git anchors, CLI `tui --tab` now fails fast on invalid tabs, CLI/MCP fact-kind validation uses a shared `FACT_KINDS` domain enum, TUI promotion uses Runtime/Probation gates instead of direct activation, Runtime/TUI/CLI/MCP explicit remember paths share safe git-anchored invalidating writes, Runtime/TUI/MCP forget paths share git-anchored temporal closeout, MCP forget stamps git closeout anchors, retrieval candidates are scope-filtered before semantic reranking, retrieval candidates receive final workspace/session scope filtering, superseded facts close temporal validity with `t_expired` and commit anchors, retrieval recency ranking uses the runtime clock, ranking/conflict tests use deterministic fact IDs and clocks, vector embedding cache uses the runtime clock, outcome telemetry uses an injectable clock, injection anti-repetition ledger uses the runtime clock, promotion verification timestamps use the runtime clock, promotion gates rescan secret-shaped fact identity, why renders invalidating commit closeout anchors, revert revalidation respects branch representation, revert revalidation skips bad historical anchors, atomic temporal lifecycle closeout anchors, TUI read model redacts secret fact text, conflict notes redact identity/object secrets, structured why redacts evidence chain metadata, structured why redacts fact identity and scope strings, structured why redacts promotion audit text, structured why redacts git anchors, structured why redacts evidence payloads and tags, why output shows expired/invalidated temporal closeout, send-edge safety scans fact tags for secrets, command cards avoid false HEAD verification claims, low-trust claims blocked from PreToolUse guardrail injection, candidate facts kept out of vector search until promotion, lifecycle-synced vector index entries for expired/reactivated facts, secret-redacted semantic rerank text, retriever-level send-unsafe fact suppression, tag-update secret sensitivity restamping, secret-redacted FTS/vector fact indexing, FTS path-query fail-soft handling, runtime-level injection context redaction, hook tool-result redaction before injection context, shared retrieval-context redaction for CLI/MCP/hooks, atomic invalidation effects, supported-by temporal evidence edges, tool-arg redaction before retrieval, hook prompt redaction before retrieval, transcript-tail redaction before retrieval, symlink-safe workspace config reads, symlink-safe local store paths, symlink-safe AGENTS.md boot capsule writes, symlink-safe generic adapter marker writes, symlink-safe OpenCode config writes, symlink-safe Cursor mcp writes, symlink-safe Claude settings writes, workspace-confined CI workflow extraction, workspace-confined agent prose extraction, workspace-confined Docker/Compose extraction, workspace-confined test config extraction, workspace-confined tooling config extraction, workspace-confined tsconfig extraction, workspace-confined editorconfig extraction, workspace-confined runtime version extraction, workspace-confined lockfile extraction, workspace-confined package script evidence, shared realpath workspace evidence checks, symlink-aware injection staleness checks, generated-marker symlink skip, procedure verifier secret scanning, extraction subject secret scanning, hook session-id redaction, MCP session-reference secret refusal, open-loop session metadata secret refusal, explicit memory metadata secret refusal, packageManager canonical facts, package-manager-aware script extraction, runtime-version deterministic extraction, multi-cookie session redaction, MCP error redaction, fail-closed retrieval for unvalidated commit-scoped facts, semantic CLI recall ranking, auth/cookie header secret scanning, scoped semantic retrieval expansion, workspace-confined injection staleness checks, package metadata extraction, high-trust test/Docker/tooling/tsconfig deterministic extraction, high-trust Python toolchain extraction, high-trust pyproject package-manager extraction, pyproject dependency-group Python command extraction, high-entropy redaction hardening, repo-id-isolated temporal validity, measured hook latency, fail-closed adapter install/uninstall, typed CLI error formatting, answer-bearing coding query expansion, Python package-manager retrieval expansion, redacted TUI selected-fact detail panels, TUI page/jump keyboard navigation, composed TUI empty states with operator status surfaces, SessionStart suppression of superseded user preferences, chunk-safe TUI key decoding with left/right and j/k navigation, bounded secret-redacted TUI prompt input, vector-safe commit-hygiene query expansion, searchable TUI memory lists, and tox/nox Python test harness extraction. 299 tests, 19 gate suites green, all I1-I9 hold._ -_Quality counters: Tests: 292. Gate suites: 19._ +_Quality counters: Tests: 299. Gate suites: 19._ _Runtime-clock note: direct CLI, eval, and benchmark retrievers pass `Runtime.clock`, so pull and push retrieval use the same recency clock seam._ @@ -119,4 +119,4 @@ now have explicit scanners instead of relying only on entropy fallback._ | Code quality | ✅ | new `eval quality` passes 6/6: full-repo Biome, strict TS config/scripts, shared command-surface helpers for CLI help/docs/README reachability, eval-suite runner/test coverage, final README docs-as-code, and generated migration packaging guard | _Loop note: composite metric = (failing_gates × 100) + (un-perfected aspects); within-aspect -measured gains are recorded in the `memory` graph. Tests: 292, gate suites: 19 (`eval all` includes run/memory/promote/drift/retrieval/gate/security/branch/temporal/conflict/procedure/mcp/storage/telemetry/provenance/resilience/benchmarks/cli-docs-demo/quality)._ +measured gains are recorded in the `memory` graph. Tests: 299, gate suites: 19 (`eval all` includes run/memory/promote/drift/retrieval/gate/security/branch/temporal/conflict/procedure/mcp/storage/telemetry/provenance/resilience/benchmarks/cli-docs-demo/quality)._ 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"); + }); +});