setup.go hand-rolls a ~240-line TOML editor for Codex config

[CBosch101]

Surfaced by the detritus code survey.

What: setup.go:663-899 reimplements TOML table upsert — including inline-table brace matching and string-aware entry splitting (findMatchingBrace, splitInlineTOMLEntries) — ~240 lines of bespoke parser to edit Codex's config.toml without a TOML library.

Why it matters: a partial hand-parser over someone else's config format is a standing correctness/maintenance liability — the exact "every addition must justify its cost / reject fragility" the doctrine preaches.

Direction: use a real TOML library (BurntSushi/toml or pelletier/go-toml) for the read-modify-write, or narrow the surface it must handle and document the constraint.

[CBosch101]

Looked at this closely and I'd recommend against the naive library swap — flagging the tradeoff before anyone spends effort on a change that would regress.

Why the obvious fix regresses. upsertTOMLTable (setup.go:702) does a surgical, in-place edit of the user's Codex config.toml: it removes/replaces only the one [mcp_servers.detritus] form and leaves every other table, comment, and the user's ordering/formatting byte-for-byte intact (removeTOMLTableForms, the inline-table path, etc.). Neither BurntSushi/toml nor pelletier/go-toml v2 preserves comments/ordering on a Unmarshal → modify → Marshal round-trip (v2 marshals from a map/struct; v1's comment-aware Tree API is deprecated). So a library swap would rewrite the whole file the user hand-maintains — dropping their comments and reordering tables — which is strictly worse than the status quo for a config a person edits by hand. That's the same "reject fragility / every addition must justify its cost" the issue cites, but pointing the other way: the dependency's cost here is a real user-facing regression.

What I'd do instead (the issue's own alternative direction — "narrow the surface and document the constraint"):

• Keep the in-place editor (it exists for a real reason), but document the exact, narrow TOML surface it supports at the function boundary (single/dotted table headers + inline parent tables; string values only via tomlString) so it's clear what's intentionally out of scope.
• Add round-trip / idempotency tests if they're missing — re-running the upsert is a no-op, an existing [mcp_servers.detritus] is replaced not duplicated, and unrelated tables + comments survive. That converts "bespoke parser is scary" into "bespoke parser is pinned by tests."

If comment/formatting loss on config.toml is actually acceptable to you, the library swap becomes trivial and I'll do it — but that's a product call, so I didn't want to make it unilaterally. Happy to take either path on your word.