fix(codex): preserve TOML values and routing marker ownership

[luvs01]

Summary

• Fix repeated routing-marker accumulation: provider-table cleanup recognizes both the exact short marker and the exact emitted # Auto-injected by opencodex (undo: ocx restore) marker. Reinjecting a managed configuration remains byte-idempotent.
• Determine ownership from structural TOML assignments and exact adjacent markers, or the journal's recorded value where that existing contract applies. Marker quotations in user comments, URLs, and multiline strings remain user data.
• Preserve complete managed assignment spans and multiline values during removal and rollback. Removal accepts valid signed 64-bit scalar integers that Bun cannot decode into a JavaScript safe integer without weakening validation for malformed or incomplete assignments.
• Add regression coverage for LF/CRLF/BOM, quoted keys, multiline basic/literal strings, child tables, user comments, journal fallback, and large integers; update the owning architecture documentation and user troubleshooting guidance.

Verification

Current conflict-resolution checkpoint

• Current HEAD: 55839fb96c6d5eba8e619363975c915235c8fd8d; tree: 24dbe2dfeee4984f70a6700e588e6deeaa8d1b3d. Integrated dev 592c5cfc043cd5b69e8aea0f12b9a0644cc50612 by a fast-forward update of this PR branch to a two-parent integration commit; no target branch or PR was merged.
• Only the two test-inventory conflicts required resolution. Both the configuration-preservation test and all three new credits tests are retained in both inventories. The five TOML production modules and original regression files are byte-identical to the previous PR head.
• Linux/Bun 1.4.0: bun test tests/test-layout.test.ts tests/test-layout-tooling.test.ts passed 18 tests, 555 assertions. Typecheck, structure, privacy and file-size checks passed. git diff --cached --check origin/dev passed; unrelated upstream planning documents contain pre-existing EOF whitespace and were not edited.
• bun test tests/codex-integration/codex-config-preservation.test.ts produced 75 pass / 2 fail. Both artifact-failure rollback cases fail because the fixture's synthetic failure stage was not reached. Running those same two cases against the unchanged previous head bbf349aee2ba32b59cbe25d70373dcd105968734 reproduced both failures in this cloud environment. Their cause remains unclassified; neither this run nor the control is a pass.
• The original Windows 215-test run and previous hosted checks below are historical evidence, not tests of this new commit. New exact-HEAD hosted CI and the conflict-resolution review have passed, as detailed below. The previously failing local cases also passed in the hosted run; their local failure cause is still unclassified. No timeout, assertion or security setting was relaxed.

Previous-head verification record

Publication preparation is based on dev commit 6538001e2fdf54ef657f5057e8c3cbd3249d8e59 (2.74.0). The existing affected runtime files, documentation, and provider-table tests are unchanged from the independently reviewed patch base; both test inventories preserve newer upstream additions.

Validated and published head: bbf349aee2ba32b59cbe25d70373dcd105968734; tree 9693b6adaaeab33103d6eb1e2233ecb9354b3cf3. The remote Git tree and commit match the locally tested objects exactly. The latest observed dev is 2405624f3987293a8fd607a889dcb61e3a376bd7, two commits ahead of the base. Those upstream changes are not incorporated into this branch; the PR is mergeable and its current-head hosted CI completed successfully. The latest readiness check observed dev at cda8ff4e4eb5c48d7af1f5b108e6551fbdc1e5d3, three commits ahead of the base, within the documented recent-dev limit.

Previous-head local results:

• Focused configuration injection/restoration regressions: 215 passed, 0 failed, eight files, 1,498 assertions.
• bun test tests/test-layout.test.ts tests/test-layout-tooling.test.ts: 18 passed, 0 failed.
• bun run typecheck, bun run privacy:scan, bun run structure:check, bun scripts/file-size-ratchet.ts, and git diff --check 6538001e2fdf54ef657f5057e8c3cbd3249d8e59 HEAD: passed.
• bun run lint:gui:if-changed: exited 0 with the official no-GUI-changes skip; GUI lint itself was not run.
• In docs-site, bun install --frozen-lockfile and bun run build: exited 0; build generated 545 HTML pages and completed its search index.
• bun run test:changed: failed with exit 124 at the unchanged 900-second budget. Incomplete output contains 6,106 passing-test lines and 86 failing-test lines; those are partial log counts, not a final suite summary. Failures include hook/test/request/CLI timeouts and later API assertions. Their individual causes are not all established. This run is retained and is not represented as passing.

The exact focused regression command was:

bun test tests/codex-integration/codex-config-preservation.test.ts tests/codex-integration/codex-inject.test.ts tests/codex-integration/codex-injected-marker.test.ts tests/codex-integration/codex-provider-table-retention.test.ts tests/codex-integration/codex-inject-retained-table.test.ts tests/codex-integration/codex-restore-app-rewrite.test.ts tests/codex-integration/codex-web-search-switch.test.ts tests/codex-integration/codex-catalog-restore.test.ts

The full local suite and combined prepush command have not completed. Under the resource exception in AGENTS.md, the focused set above covers the changed ownership, assignment-span and rollback behavior, including read-as-data layout guards. Repeated broad runs are disproportionately expensive on this Windows host: the import-connected run reached its fixed 15-minute limit and the dependency/documentation work also took substantial time. Remaining broad import-connected/full-suite and integration coverage is left to hosted CI; the failed local aggregate remains a failure, not a substitute pass. Previous-head review readiness was based on the focused resource exception and the successful hosted checks below; the failed local aggregate is retained as a failure.

The earlier investigation's bun run test:changed at 2.73.0 hit the unchanged 900-second main budget (exit 124). Its incomplete output included failures; two representative request-timeout cases also failed on the immutable unmodified base. Not every historical failure was classified. That failed run is preserved, and no timeout is disabled or increased. Any focused-validation exception and coverage left to CI will be documented explicitly before review readiness.

Checklist

• Scope stays focused and avoids unrelated cleanup.
• Docs or release notes were updated when needed.
• Security-sensitive changes were reviewed for secrets, auth, and unsafe defaults (independent source review and privacy scan; synthetic regression data only; no authentication transport, destination, workflow, or dependency changes).

Summary by CodeRabbit

Summary by CodeRabbit

• Bug Fixes
  • Configuration updates and removal now better preserve unrelated settings, comments, multiline values, byte-order marks, and line endings.
  • Improved detection of managed provider and routing settings, including recovery when marker comments are missing or values have been edited.
  • Improved handling of large integers and invalid or incomplete assignments to help prevent configuration corruption.
• Documentation
  • Expanded sign-in troubleshooting guidance with details about configuration recovery and preservation.

Focused follow-up of two local integration failures

bun test tests/codex-integration/codex-composed-acceptance.test.ts --test-name-pattern "#1802|A-reduced" reproduced two failures at the published head: a request TimeoutError and CLI watchdog: ocx ensure (0 pass, 2 fail, 6 filtered). These exact failure modes were also reproduced in the retained immutable, unmodified 2.73.0 baseline. The composed fixture and the five affected TOML source modules have identical Git blob hashes in that baseline and the 2.74.0 publication base. This is a limited comparison; it is not a new full 2.74.0 baseline run and does not classify the other aggregate failures. No timeout was increased or disabled.

The initial CodeRabbit draft-skip status is not counted as a completed code review. The subsequent explicitly requested full review completed for the published head, with no actionable comments and no unresolved inline threads.

Previous-head hosted verification and review readiness

• Cross-platform CI completed successfully for head bbf349aee2ba32b59cbe25d70373dcd105968734: all four test shards, aggregate ci, gates, storage policy, API usage, docs build, structure, Docker, Windows/Ubuntu keyring and Windows/Ubuntu npm-global smokes passed.
• PR target, hygiene and React Doctor checks passed. Skipped Windows/macOS full matrices, native/desktop, setup/remote-helper and hosted privacy jobs are not counted as passing; the local privacy scan did pass.
• CodeRabbit's full review covered all 11 changed files at the exact published head and generated no actionable comments. Its docstring coverage warning (39.53% against the default 80% threshold) remains a warning, not a passed check. The CodeRabbit review status itself completed successfully; no unresolved review threads were present.
• The signed-radix concern in the other review was checked against the actual helper: six signed hex/octal/binary inputs returned null without throwing. The expression permits a sign only on its decimal alternative. No production change was needed for that non-reproduced finding.
• The implementation-phase independent reviews and boundary fixtures are retained; no implementation changes were made during publication. The additional GitHub @codex review request has no confirmed response and is not counted as a completed GitHub Codex review.
• At the previous-head checkpoint the branch met the recent-dev criterion and was mergeable. Focused local validation, the explicit full-suite resource exception, hosted CI, and independent/CodeRabbit review provided the previous readiness evidence; this does not attest to the new integration commit. Maintainer approval remains a separate merge requirement. No merge, deployment, installed-runtime update or user configuration cleanup is requested.

Renewed review-readiness checkpoint

• CI 36721850681 passed at 55839fb96c6d5eba8e619363975c915235c8fd8d, including all four test shards, aggregate, docs and applicable gates/smokes. Skipped optional jobs are not counted as passing.
• Shard 1 logs show 77 pass / 0 fail for the complete configuration-preservation file, including both artifact-failure rollback cases. This verifies the affected behavior in CI without relabeling the historical local failures as passes.
• CodeRabbit reviewed both changed inventory files from bbf349a... to this exact head and reported no actionable comments. The original runtime source review remains applicable to the byte-identical five TOML modules. No unresolved inline findings are present.
• The branch now includes inspected dev 592c5cfc..., and GitHub reports no conflict. Review readiness is restored; human maintainer approval and any merge remain separate.

[coderabbitai]

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Repository: lidge-jun/opencodex/.coderabbit.yaml

Review profile: ASSERTIVE

Plan: Advanced

Run ID: 8aaa2938-0aec-4e56-a980-36358475d579

📥 Commits

Reviewing files that changed from the base of the PR and between bbf349a and 55839fb.

📒 Files selected for processing (2)

• scripts/test-layout/layout.json
• tests/fixtures/test-layout-expected.json

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 8 remain after this review.

---

📝 Walkthrough

Walkthrough

Codex TOML injection and restoration now use structural source-line parsing for ownership checks and assignment edits. The changes preserve BOMs, line endings, multiline content, and user-owned values across config transformations. New integration tests cover preservation, rollback, and fallback restoration.

Changes

Configuration preservation

Layer / File(s)	Summary
Structural TOML parsing and ownership evidence src/codex/toml-source-lines.ts, src/codex/injected-marker.ts	Added source-line helpers for root boundaries, assignment spans, integer validation, and whitespace normalization. Routing ownership now requires an exact marker on the adjacent structural line.
Injection, ownership, and removal src/codex/inject/config-toml.ts, src/codex/inject/provider-table.ts, src/codex/inject/remove.ts	Config transformations and provider removal now parse structural assignments and remove or update complete spans. Provider-table ownership also recognizes the adjacent routing marker.
Preservation tests and recovery guidance tests/codex-integration/*, scripts/test-layout/layout.json, tests/fixtures/test-layout-expected.json, docs-site/src/content/docs/troubleshooting/codex-cannot-sign-in.md, structure/codex-home.md	Added tests for TOML preservation, ownership, rollback, and restoration. Updated test-layout mappings and documented recovery rules.

Priority: ⬇️ Low

Estimated code review effort: 4 (Complex) | ~45 minutes

Change: Bug fix

Merge Risk: ⚪ Minimal · up to 55839

The added preservation tests are registered consistently and were exercised in sharded CI. The local changed-mode timeout did not identify a failing test, so no concrete merge-blocking risk is established.

Architecture Summary

Architecture risk: 🔵 Low · up to 55839

The change affects 5 systems.

Changed systems: src, tests, docs-site, scripts, structure

Architecture concerns
No architecture-level concerns identified.

Review details

Systems and components

• observed — src (service) was modified; 5 changed files map to changed impact.
• observed — tests (service) was modified; 3 changed files map to changed impact.
• observed — docs-site (service) was modified; 1 changed file maps to changed impact.
• observed — scripts (service) was modified; 1 changed file maps to changed impact.

Before / after behavior

• observed — Modified behavior in docs-site/src/content/docs/troubleshooting/codex-cannot-sign-in.md: The manual-recovery instructions now describe marker and journal-based ownership checks, preservation of user comments and multiline strings, complete web_search assignment restoration, assignment validation and 64-bit integer handling, refusal of invalid or incomplete assignments, and fallback blank-line compaction.
• observed — Modified behavior in src/codex/inject/config-toml.ts: Imports source-line parsing and structural-whitespace helpers, and replaces the previous TOML string parser and marker constant imports.
• observed — Modified behavior in src/codex/inject/config-toml.ts: The EOL comment now distinguishes structural edits that retain physical endings from the injection/removal pipeline’s LF normalization and write-time conversion.
• observed — Modified behavior in src/codex/inject/config-toml.ts: setRootOpenaiBaseUrl now delegates to a shared source-line setter. The setter recognizes only structural root URL assignments directly preceded by the routing marker, preserves user-owned assignments, refreshes owned assignments, and inserts new marker/key lines before the root ends while retaining source formatting.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 39.53% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 43 functions across 7 files. (2 skipped: …	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly and concisely describes the main changes: preserving TOML values and enforcing routing-marker ownership during Codex configuration updates.

Full details: Docstring Coverage

Explanation

Docstring coverage is 39.53% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 43 functions across 7 files. (2 skipped: 2 unsupported.)

• Fix all pre-merge checks with AI

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create a new PR

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[github-actions]

✅ Deterministic PR hygiene checks passed.

[luvs01]

@coderabbitai full review

@codex review

Please review head bbf349aee2ba32b59cbe25d70373dcd105968734 while this PR remains draft. The scope is exact marker ownership and preservation of TOML assignment spans and multiline values during injection/removal/rollback. Please check malformed-input rejection and valid signed 64-bit integer removal as well. Local validation and the retained 900-second aggregate failure are documented in Verification; this is a review request, not a merge or release request.

[coderabbitai]

✅ Action performed

Full review finished.

[lidge-jun]

리뷰 · 우선순위 68 / 80

이 PR은 Codex config.toml을 주입·제거할 때 사용자 설정이 깨지거나, 소유 표시 주석이 쌓이던 문제를 고칩니다. 베이스는 dev입니다.

예전에는 줄 단위 split("\n")와 includes("# Auto-injected by opencodex")에 기대 마커·키를 찾았습니다. 그 결과 (1) 사용자 주석·URL·여러 줄 문자열 안에 마커 글자만 있어도 “우리 것”으로 오인하고, (2) 여러 줄 값·큰 정수·BOM·CRLF를 자르거나 섞을 수 있었습니다. 이번 변경은 toml-source-lines의 구조적 줄·할당 구간을 공통으로 쓰고, 소유는 “바로 위 구조적 줄이 짧은 마커 또는 (undo: ocx restore) 형태와 정확히 같을 때” 또는 저널에 기록된 값으로만 인정합니다. 프로바이더 테이블 정리도 두 마커 형태를 모두 받아, 다시 넣어도 바이트가 같은지 회귀 테스트로 잠갔습니다. 로그인 문제 해결 문서와 structure/codex-home.md에도 같은 규칙을 적어 두었습니다. types/config 중복으로 닫을 PR은 없습니다.

라인 - src/codex/toml-source-lines.ts sourceAssignmentSpan · 부호 있는 16/8/2진 정수(+0x…, -0x…)는 정규식에는 잡히지만 BigInt("+0x…")가 바로 던집니다. 거부 테스트는 toThrow()만 보므로 통과하지만, 호출부 메시지는 “불완전 할당”이 아니라 BigInt 예외로 남습니다. 십진 큰 정수는 테스트로 잘 덮여 있습니다.

라인 - 검증 · 집중 회귀(8파일 215통과)와 typecheck·structure·docs build는 본문에서 통과로 적혀 있습니다. 로컬 bun run test:changed는 900초 예산 exit 124로 실패했고, 실패 줄에 타임아웃·API 단언이 섞여 있습니다. 호스트 CI는 이 시점 기준 test 1/4·2/4 통과, 3/4·4/4는 아직 pending입니다. draft 유지도 본문과 맞습니다.

라인 - 동작 변화(의도) · 소유 판정이 부분 문자열에서 “줄 전체 일치”로 좁아졌습니다. # Auto-injected by opencodex …처럼 뒤에 말을 붙인 커스텀 주석은 더 이상 라우팅 소유로 안 봅니다. 사용자 데이터를 지키려는 방향이라 맞지만, 그런 변형 마커를 쓰던 설정이 있다면 restore/재주입 경로를 한 번 확인하는 편이 좋습니다.

메인테이너의 판단이 필요한 지점

draft를 ready로 올리기 전에 호스트 test 3/4·4/4(및 관련 shard)가 초록인지 보고 머지할지입니다. 로컬 test:changed 실패는 작성자가 “통과로 치지 않는다”고 적어 두었고, 변경 범위의 소유·할당 구간은 집중 스위트로 꽤 단단합니다. BigInt 부호 진수 예외를 이 PR에서 null 거부로 다듬을지, 현실 설정에 거의 없으니 후속으로 둘지도 정하면 됩니다.

너의 추천

집중 테스트·문서·의도한 소유 경계는 설득력 있습니다. CI 남은 shard가 초록이면 draft만 풀고 머지해도 됩니다. 부호 진수 BigInt throw는 가능하면 sourceAssignmentSpan에서 catch해 null로 돌려, 호출부의 “불완전 할당” 거부와 메시지를 맞추면 좋습니다. preview deploy 이야기는 생략합니다.

이 댓글은 grok-bot이 작성했습니다

[luvs01]

Thanks for the review. I checked the signed-radix concern at head bbf349aee2ba32b59cbe25d70373dcd105968734 against the actual helper: +0x7fffffffffffffff, -0x7fffffffffffffff, +0o777, -0o777, +0b111, and -0b111 all return null from sourceAssignmentSpan without throwing. The optional sign belongs only to the decimal alternative; the hex/octal/binary alternatives are unsigned, and the expression is anchored. Signed nondecimal literals are not valid TOML integers. The existing regression also rejects signed hex through the removal paths. No production change is needed for this finding.

Cross-platform CI is now successful for this exact head: https://github.com/lidge-jun/opencodex/actions/runs/36693484295 . The failed local 900-second run and the limited baseline comparison remain documented rather than treated as passing. I am still checking the requested review results before changing the draft state; no merge or runtime application is requested.

[lidge-jun]

Maintainer integration into dev (MAINTAINERS.md, dev-only exception) by @lidge-jun.

• Head 55839fb96c: Cross-platform CI 36721850681 (pull_request, attempt 1) passed all four test shards, gates, docs and structure.
• Independent review including security review of the config-write path: PASS. Marker accumulation is fixed by consuming both marker forms (src/codex/inject/provider-table.ts), rollback captures complete validated assignment spans (src/codex/inject/config-toml.ts), ownership checks preserve user data, and atomic writes/rollback stay intact.
• Local integration: merged together with fix(devin): bound held signature-type payloads #6315, fix(devin): score family axes sparsely with own-key targets #6324 and fix(codex): preserve TOML values and routing marker ownership #6308 onto dev b933aa2; bun run typecheck passes and the PRs' own test files pass (197 pass / 0 fail).
• scripts/ci/assert-mergeable-review.sh --maintainer-integration OK at this head. Cross-platform CI for the final dev tip runs once after this landing batch.

Thanks @luvs01!