fix(codex): prevent duplicate [projects] tables from breaking config.toml load#7926
fix(codex): prevent duplicate [projects] tables from breaking config.toml load#7926Cainiaooo wants to merge 1 commit into
Conversation
…toml load Orca-launched Codex could fail at config load with `duplicate key` because the managed config.toml ended up with two [projects....] tables that decode to the same TOML key -- one basic string (written by Orca) and one literal string (written by the Codex CLI). The error recurred on every launch because the config mirror re-emitted the collision, so manual edits never stuck. Two format-agnosticity gaps caused and preserved this on Windows: - upsertProjectTrustLevelInContent only matched double-quoted (basic) project headers, so an existing single-quoted (literal) header was not found and a duplicate basic table was appended. It now matches by decoded + normalized key and collapses any pre-existing decoded-duplicate to a single table. - The config mirror deduped project sections by raw header text, so basic vs literal / slash / drive-case variants of one path both survived the merge. It now dedupes by decoded key and also collapses system-side duplicates in both the merge and fresh-mirror paths. Project keys are compared via a shared decode/normalize helper: Windows-style paths fold separators and drive-letter case, while POSIX paths stay case-sensitive. hooks.state keys are untouched -- their slash variants are intentionally distinct. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (6)
📝 WalkthroughWalkthroughThis PR adds a new module, codex-project-trust-key.ts, that decodes and normalizes TOML 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
2 participants
Summary
On Windows, Codex launched through Orca could crash at config load with
failed to load configuration: ...\config.toml:NNN: duplicate key. The managedconfig.tomlaccumulated two[projects....]tables that decode to the same TOML key — a basic string (double quotes, escaped backslashes) written by Orca and a literal string (single quotes, raw backslashes) written by the Codex CLI. TOML compares keys by decoded value, so the two collide and Codex refuses to load the file. It recurred on every launch (the config mirror re-emitted the collision), so deleting the offending line never stuck.This PR makes project-trust reads/writes compare
[projects....]headers by their decoded, normalized key across both the trust writer and the config mirror, and collapses any pre-existing decoded-duplicate to a single table.Root cause / fix
upsertProjectTrustLevelInContentmatched only double-quoted (basic) headers, so an existing literal header was not found and a duplicate basic table was appended. It now scans by decoded + normalized key and collapses all duplicate matches into one table.codex-project-trust-key.ts: Windows-style paths fold separators and drive-letter case; POSIX paths stay case-sensitive (SSH/WSL-safe).hooks.statekeys are deliberately left untouched — their slash variants are genuinely distinct keys.Screenshots
No visual change (config-file handling only).
Testing
pnpm lintpnpm typecheckpnpm testpnpm buildNote: the
pnpmsuite was not run in my environment (dependencies not installed there). I verified the changed modules end-to-end by driving the real code (upsertProjectTrustLevelInContentandsyncSystemConfigIntoManagedCodexHome) against temporary configs — all new and regression scenarios pass — andnode config/scripts/check-max-lines-ratchet.mjsreports no new bypasses. New tests cover: literal↔basic match, slash/drive-case drift, POSIX case sensitivity, pre-existing duplicate collapse, system-side dedup (merge + fresh mirror), and ahooks.stateno-dedup guard. Please rely on CI for the fullpnpmgate.AI Review Report
Reviewed with Codex over two rounds. Round 1 flagged two P2 robustness gaps: (1) the upsert updated only the first matching table and left a pre-existing decoded-duplicate behind; (2) the mirror could still emit a system-side basic/literal duplicate when the runtime had no matching section (both the merge and fresh-mirror paths). Both were fixed — collapse-all in the upsert, and system-side dedup in
stripRuntimeOwnedTomlSections— and re-review found no discrete correctness issues.Cross-platform: this is Windows-focused; macOS/Linux/POSIX case sensitivity is explicitly preserved (POSIX paths are never folded), and the SSH/WSL case (POSIX paths under a Windows host, already-canonical remote paths) is handled by content-based normalization rather than
process.platform. No keyboard shortcuts, labels, or shell behavior are touched; paths flow through decode/normalize helpers rather than raw separators.Security Audit
The change only reads and rewrites
~/.codex/config.tomland Orca's managed runtimeconfig.toml. No command execution, network, auth, secrets, or IPC surface is touched. Writes continue to use the existing atomic write +.bakrotation, and byte-preservation of unrelated config is retained (edits stay scoped to[projects....]sections). Input is untrusted TOML from disk; parsing is bounded single-line string decoding with no eval or regex-DoS risk.Notes
CODEX_HOME); the runtime config self-heals on the next launch once this ships.~/.codex/config.toml(from before this fix) is collapsed the next time Orca marks that project trusted, but is not otherwise migrated.🤖 Generated with Claude Code