fix(codex): read feature flags from a TOML parse, not a line scanner (#1295) - #1309
Conversation
…1295) `codex features enable multi_agent_v2` writes `enabled = true` correctly, and OpenCodex then read the file back and concluded it had not — so the dashboard toggle reported `codex feature command did not enable multi_agent_v2` and `ocx v2 status` kept printing OFF for a config `codex features list` reports as enabled. `tomlTableBody` ends a table at the first line matching `/^\s*\[/`, with no awareness of multi-line strings. A `"""` value whose prose begins with a bracketed tag therefore cut the table body mid-literal, and because `codex features enable` appends `enabled` at the END of the table, that key landed outside the extracted body. Key order alone decided the answer: identical semantics, `enabled` before the string read true, after it read false. The readers that answer these questions now use `Bun.TOML.parse` through `parsedTomlTable`. Table form, boolean form, and inline-table form all fall out of the parse rather than each needing its own regex, and prose that merely looks like an assignment is no longer one. `hasAgentsMaxThreads` uses `Object.hasOwn` rather than a value check: it gates a codex-rs boot refusal, so a present-but-unusable key must still be detected even when the getter declines to return it. `tomlTableBody` itself is unchanged. Twenty call sites consume its output, mostly by regex, so widening what it returns changes what all of them match — an earlier attempt at this fix made the scanner string-aware and gave `getAgentsEnabled`, `getAgentsMaxDepth`, and `getMaxConcurrentThreads` three new wrong answers. It remains the fallback for a document Bun's parser rejects, which is best-effort by construction and claims nothing about what Codex's own parser would accept. Two sibling readers had the same prose-as-assignment defect before this change and are fixed alongside: `isDefaultModeRequestUserInputEnabled` and the `[agents] max_threads` pair. Reported by @brunoflma with a minimal three-file reproduction that isolated the single variable, which is why this went straight to the scanner rather than through the Codex command.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughChangesTOML feature readers
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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 |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 493b418678
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const parsed = parsedTomlTable(content, "agents"); | ||
| if (parsed !== null) { | ||
| const value = parsed.max_threads; | ||
| return typeof value === "number" && Number.isInteger(value) && value >= 1 ? value : null; |
There was a problem hiding this comment.
When max_threads is a TOML float whose JavaScript value is integral, such as max_threads = 1.0 or 1e3, Bun.TOML.parse returns 1 or 1000, so this Number.isInteger check accepts it. Codex deserializes this field as usize and rejects floats, while the previous lexical reader returned null; consequently, status and transition flows now treat an unusable setting as valid and may migrate it to another key. Preserve the TOML numeric kind or validate the original lexeme before returning the value.
AGENTS.md reference: src/AGENTS.md:L10-L10
Useful? React with 👍 / 👎.
1 participant
Summary
Fixes #1295.
codex features enable multi_agent_v2writesenabled = truecorrectly, and OpenCodex read the file back and concluded it had not — so the dashboard toggle failed withmulti_agent_v2 transition failed: codex feature command did not enable multi_agent_v2, andocx v2 statuskept printing OFF for aconfig.tomlthatcodex features listreports as enabled. The Codex command was never at fault.tomlTableBodyends a table at the first line matching/^\s*\[/, unaware of multi-line strings. A"""value whose prose begins with a bracketed tag cuts the table body mid-literal — and sincecodex features enableappendsenabledat the end of the table, that key lands outside the extracted body. Key order alone decided the answer: with identical semantics,enabledbefore the string readtrueand after it readfalse.The readers that answer these questions now use
Bun.TOML.parsethrough a sharedparsedTomlTable. Table form, boolean form, and inline-table form all fall out of the parse instead of each needing its own regex, and prose that merely looks like an assignment stops being one.hasAgentsMaxThreadsusesObject.hasOwnrather than checking the value: it gates a codex-rs boot refusal, so a present-but-unusablemax_threadsmust still be detected even where the getter correctly declines to return it.tomlTableBodyitself is unchanged — only its comment. Twenty call sites consume its output, mostly by matching a regex against the returned text, so widening that text changes what all of them match. An earlier attempt at this fix made the scanner string-aware and thereby gavegetAgentsEnabled,getAgentsMaxDepth, andgetMaxConcurrentThreadsthree new wrong answers. It remains the fallback for a document Bun's parser rejects; that fallback is best-effort by construction and claims nothing about what Codex's own parser would accept, since they are separate implementations.Two sibling readers had the same prose-as-assignment defect before this change, and are fixed alongside rather than left inconsistent with the reader that shares the file:
isDefaultModeRequestUserInputEnabled, and the[agents] max_threadspair.Verification
bun run test— 10048 pass / 7 skip / 0 fail across 627 filesbun test tests/codex-v2-gate.test.ts— 100 pass / 0 failbun run typecheck— cleanbun run privacy:scan— passedsrc/codex/features.tsfails 8 distinct testsThe tests are deliberately one hazard per
test()block. Bundled as a single block they reported one failure under ablation, because Bun stops a block at its first failing expectation — so a later assertion could never run and still look covered. Split, the same ablation reports eight.Covered: bracketed prose in a
"""value, the same in a'''value, key-order independence, prose that contains a literalenabled = truewith no such key, a delimiter inside a comment, an escaped\""", a multi-line array, an array opening on the line after=,#inside a string, a header-shaped line inside a string, a following table's key never being read as this feature's, the unparseable-document fallback, and presence-vs-usability formax_threads.Checklist
codex features listalready reports.)config.tomlfeature flags only; no credential, auth, or workflow surface is touched, and nothing new is logged.)Reported by @brunoflma, whose minimal three-file reproduction isolated the single variable and identified
tomlTableBodydirectly — which is why this went straight to the scanner instead of through the Codex command.Summary by CodeRabbit
Bug Fixes
Tests
Documentation