Repository navigation
codex: preserve other MCP servers when writing config - #223
Conversation
Replace line matching in the Codex config writer with TOML parsing so qmax-code only replaces its own [mcp_servers.qmax] table and keeps every other server, setting, and comment byte-for-byte. - Detect quoted (`[mcp_servers."qmax"]`) and commented headers; ignore table-like text inside multiline strings. - Refuse to write invalid TOML, unreadable files, or inline/dotted qmax definitions instead of risking unrelated settings. - Write atomically with owner-only permissions; follow an existing symlink rather than replacing it. - Error messages never include configuration values. - Setup detection shares the same parser.
QualityMax ReviewVerdict: COMMENT · Confidence: evidence-backed scan Re-review: Open the QualityMax check and select Re-review after it completes. Files eligible: 7 · Files reviewed: 7 · Files with findings: 0 · Findings: 0 · Inline cards: 0 Priority findings
Review gates
Important files
Change diagram — Flowflowchart TD
Start[Start WriteCodexMCPEntry] --> Lstat{Is Symlink?}
Lstat -- Yes --> Resolve[Resolve Symlink]
Resolve --> Read[Read Config File]
Lstat -- No --> Read
Read --> Exists{Entry Exists?}
Exists --> Upsert[Upsert Entry]
Upsert --> Temp[Create Temp File]
Temp --> Write[Write Updated Config]
Write --> Rename[Rename Temp to Path]
Rename --> End[Return Success]
Review lifecycleUse the inline cards to inspect evidence and suggested remediation. Re-run the QualityMax review after pushing a fix; unchanged cards are identified by their stable finding marker. Dismiss with a reason through the existing QualityMax/GitHub review feedback flow. 0 prior card(s) are stale/resolved on this head. Proof legend: GROUNDED deterministic scanner evidence · MODEL-ONLY advisory model judgment · MODEL-REVIEWED patch model judgment, not reproduction. QualityMax project results are available in the configured project. Receipt · commit |
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
![qualitymaxapp[bot]](https://avatars.githubusercontent.com/in/2748338?s=60&v=4){kind=small}
QualityMax Code HealthGrade A · 100/100 · PASS — 0 finding(s) in 7 file(s). No code-health findings. Full report · Commit f6c78c9 |
|
| Gate | Result |
|---|---|
| 🔍 AI diff review | ✅ Clean · gemini-3.1-flash-lite · completed · 6 eligible / 6 reviewed · gemini-3.1-flash-lite |
| 🔍 SAST | completed · 7 eligible / 7 reviewed · claude-haiku-4-5-20251001 |
| 🔍 Canonical PR review delivery | completed · 0 eligible / 0 reviewed · exact-head review #5907029234 and overview #6883855753 confirmed |
| 🔍 Repository runtime evidence | completed · 0 eligible / 0 reviewed · runtime execution passed for the assessed revision |
| 🔍 Runtime evidence policy | completed · 0 eligible / 0 reviewed · admissible runtime evidence satisfies this advisory runtime-only policy |
| 🧪 Repo Tests | ✅ 1562/1562 passed (go) |
| 🤖 AI Tests | ❌ 51/56 passed (go_test) — 1 issue(s) |
Powered by QualityMax — AI-Powered Test Automation
There was a problem hiding this comment.
🟡 Changes recommended
The config rewrite deletes unrelated comments, and two tests fail on Windows because of incompatible permission expectations and incomplete home-directory isolation.
3 open issues
What changed in this PR
This PR replaces line-based Codex configuration editing with TOML-aware updates to better protect user settings when registering qmax’s MCP server.
Changes:
- Parses table names and validates configuration before replacing qmax entries.
- Adds atomic file replacement, symlink handling, and errors that omit configuration values.
- Shares parser-based setup detection and adds preservation, failure-handling, and filesystem tests.
| File | Description |
|---|---|
| internal/setup/orch.go | Uses shared parser-based Codex entry detection. |
| internal/setup/orch_test.go | Tests quoted headers, multiline strings, and invalid TOML. |
| internal/agent/setup_orch_test.go | Adapts existing tests to the updated helper signatures. |
| internal/agent/codex_config.go | Implements TOML-aware updates and atomic file replacement. |
| internal/agent/codex_config_test.go | Adds configuration preservation and filesystem regression tests. |
| go.sum | Records TOML dependency checksums. |
| go.mod | Adds the pure-Go TOML parser dependency. |
Review effort: Balanced
| if !skip { | ||
| out.Write(data[start:offset]) | ||
| } |
| if err != nil { | ||
| t.Fatal(err) | ||
| } | ||
| if info.Mode().Perm() != 0600 { |
|
|
||
| func TestIsOrchInstalledCodexParsesTables(t *testing.T) { | ||
| home := t.TempDir() | ||
| t.Setenv("HOME", home) |
Summary
qmax-codewrites its own[mcp_servers.qmax]entry into~/.codex/config.toml. The previous line-matching writer could miss quoted or commented headers. It could also treat table-like text inside multiline strings as a table, which risked clobbering a user's other MCP servers or settings.This PR switches to TOML parsing:
[mcp_servers."qmax"],[ 'mcp_servers' . 'qmax' ]) and commented headers are detected.qmax-code setupdetection uses the same parser.Adds
github.com/pelletier/go-toml/v2(pure Go, no new runtime).Tests
New
internal/agent/codex_config_test.gocovers:internal/setupadds coverage for setup detection.Local runs:
go test ./...passed (19 packages)go vet ./...passedgo buildpassed[email protected] run --new-from-rev=origin/mainpassed (0 issues, run withCGO_ENABLED=0locally)Not tested: Windows, or concurrent edits by another process. Updates are not serialized.