fix(config): recover Codex MCP after closing marker loss

[astandrik]

What does this PR do?

Fixes reinstall after Codex deletes mcp_servers.node_repl and takes the closing MCP marker with it. It keeps #2467's placement of foreign tables after the managed block.

Before legacy migration, install recovers a lone opening marker only when the following CBM table and managed fields are recognizable. It preserves user keys, comments and child tables, restores the closer, and writes once atomically. Ambiguous ownership or a concurrent file change causes refusal without overwriting the file.

Evidence

• CLI regression: fails on main with reinstall returning -1; passes with the fix.
• Linux scripts/test.sh: 8468 passed, 0 failed, 9 platform skips; production guards passed.
• scripts/msan.sh config_toml_edit cli: 409 passed. Config fuzz: 20,000 runs, seed 1.
• Real Codex 0.149.1 and 0.153.4: 12 lifecycle cases passed, including recovery of the same file after base refused, preservation of user data, byte-idempotent reinstall and uninstall.
• Full CI lint passed. Raw Clang analysis compared with base: no new diagnostics; 18 existing memory findings unchanged. Raw logs were compared because the existing memory gate misses some diagnostic suffixes.

Native macOS/Windows and full TSan results will come from GitHub CI.

Checklist

• Every commit is signed off
• Tests pass (full Linux remote run)
• CI lint passes
• New behavior has a failing regression before the fix

Fixes #2228.

Written with OpenAI Codex on behalf of Anton Standrik (@astandrik).

[github-actions]

Thanks for opening this — it has been seen, and it is queued.

This note is automated, but it is not a brush-off: it exists so you know where your PR stands instead of having to guess from silence.

Current review status: working through a backlog. 0.9.1-rc.1 is out, so the release freeze that held reviews is over — but it left a large queue of open pull requests behind it, and we are reading through them oldest-first. The background is in discussion #1144.

What that means for this PR, concretely:

• It will not be closed for inactivity. No stale bot touches pull requests here.
• It may still sit a while before a human reads it. That is on us, not on you.
• Older PRs are read first, so a recent one is not being skipped — it is behind a queue.

Things that will genuinely speed it up whenever review does happen:

• Keep it rebased on main — the tree is moving quickly right now, and a conflicting branch cannot be reviewed as the diff you intended.
• Get CI green, or say which failures you believe are pre-existing.
• Keep the change to one claim. Bundled features and refactors get split before they get merged, which costs you a round trip.
• Every commit needs a sign-off (git commit -s) — CI enforces DCO.

If this fixes a bug, a reproduction we can run is worth more than a description of the symptom.

Thanks for contributing, and sorry in advance for the wait.

[Copilot]

Copilot review overview

🟡 Changes recommended

Array-table ownership and dotted-key conflicts can still produce invalid or incorrectly transformed TOML.

Review effort: Balanced
Findings: 2

Open (2)

• Prevent conflicting headers from surviving array-owned block replacement · New
• Reject dotted-key conflicts with managed body assignments · New

What changed in this PR

Updates managed TOML editing to preserve foreign content and user-defined keys within marker spans.

Changes:

• Merges or removes only owned TOML tables and descendants.
• Updates Codex, Grok, and Kimi integrations for table-aware removal.
• Adds regression, CLI, and fuzz coverage.

File	Description
src/​cli/​config_toml_edit.c	Implements table-aware span editing.
src/​cli/​config_toml_edit.h	Documents and exposes the updated API.
src/​cli/​cli.c	Supplies owned headers for client removal.
tests/​test_config_toml_edit.c	Adds TOML regression coverage.
tests/​test_cli.c	Adds Codex integration coverage.
tests/​fuzz/​cbm_fuzz.c	Fuzzes table-headed managed blocks.
tests/​fuzz/​corpus/​config/​toml-foreign-span.seed	Adds a foreign-span seed.
scripts/​memory-core-baseline.txt	Updates the memory lint baseline.

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[astandrik]

Converted to draft while I run another round of checks on a small cleanup of this change. I'll mark it ready for review once they pass.

[DeusData]

Thank you so much, @astandrik, for this PR and especially for the careful side-by-side recheck with real Codex app-server writes. That comparison found a real gap. We also owe you an apology for the order of events: #2467, for the same issue, was merged today without your PR and your 10-02 results being weighed against it first. That is on us.

Your finding holds on current main. When Codex removes mcp_servers.node_repl through config/value/write, the closing marker goes with it. The next install then refuses (exit 1, config.toml untouched), exactly as you showed, while #2434 handles that case.

Would you be up for rebasing #2434 onto main as the follow-up for that case? main now keeps foreign tables by moving them below the closing marker (#2467). What is still missing is a safe recovery when the closing marker is gone. Your app-server reproduction (delete node_repl, then reinstall) would make the ideal regression test next to the existing ones in tests/test_cli.c. If you would rather not, just say so and we will take it from here with you credited.

I have reopened #2228 so the remaining case stays tracked. Thanks again for the thorough work!

[astandrik]

Thank you so much, @astandrik, for this PR and especially for the careful side-by-side recheck with real Codex app-server writes. That comparison found a real gap. We also owe you an apology for the order of events: #2467, for the same issue, was merged today without your PR and your 10-02 results being weighed against it first. That is on us.

Your finding holds on current main. When Codex removes mcp_servers.node_repl through config/value/write, the closing marker goes with it. The next install then refuses (exit 1, config.toml untouched), exactly as you showed, while #2434 handles that case.

Would you be up for rebasing #2434 onto main as the follow-up for that case? main now keeps foreign tables by moving them below the closing marker (#2467). What is still missing is a safe recovery when the closing marker is gone. Your app-server reproduction (delete node_repl, then reinstall) would make the ideal regression test next to the existing ones in tests/test_cli.c. If you would rather not, just say so and we will take it from here with you credited.

I have reopened #2228 so the remaining case stays tracked. Thanks again for the thorough work!

Updated this PR on top of main, keeping #2467’s layout. It now recovers the missing closing marker while preserving user keys and foreign tables.
The regression in tests/test_cli.c fails on base and passes with the fix. With real Codex 0.149.1 and 0.153.4 app-server writes, base refuses after deleting node_repl; the updated installer repairs that same file. Reinstall is byte-idempotent, and uninstall preserves unrelated content.
Full Linux ASan/UBSan, focused MSan, CI lint and 20,000 config-fuzz runs passed. Results are in the PR description.

[Copilot]

Warning

Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.

Copilot review overview

Review effort: Lite
Findings: 1 · 2

Open (3)

• toml_assignment_t assignment; is uninitialized before being passed to toml_parse_assignment(),… · New
• The return value of write_test_file() is ignored here, so a failed write could surface later as a… · New
• This assertion only checks that snprintf() returned a positive value, but it does not detect… · New

Resolved since last review (2)

• Reject dotted-key conflicts with managed body assignments
• Prevent conflicting headers from surviving array-owned block replacement

[Copilot]

Warning

Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.

Copilot review overview

Review effort: Lite
Findings: 1 · 1

Open (2)

• The relative key-path is derived via byte slicing at declared.header.path.len, which is likely to… · New
• This test hardcodes /tmp, which will fail on non-POSIX environments (notably Windows) and on… · New

Resolved since last review (3)

• toml_assignment_t assignment; is uninitialized before being passed to toml_parse_assignment(),…
• This assertion only checks that snprintf() returned a positive value, but it does not detect…
• The return value of write_test_file() is ignored here, so a failed write could surface later as a…