fix(codex): serialize config writes and bind recovery to locked journal by luvs01 · Pull Request #6774 · lidge-jun/opencodex

luvs01 · 2026-10-08T21:15:49Z

Summary

Serialize cooperating Codex config writers across read/edit/write and rollback. Feature edits, injection, prompt projection, and restore share one lock. Nested writers require an explicit live handle; asynchronous acquisition waits at most two seconds.
Fence stale-lock takeover and initialization. Select the recovery journal and check its owner under the config lock so an older observation cannot restore a newer live session's journal. Preserve the existing incomplete-recovery and retained-journal warnings.
Make two account metadata tests deterministic with quota-response mocks, retaining all assertions and timeouts.
Carry the Codex writer work from source commit c7ac1a03dbb8675960a927c93435259b0e3e97b0, with original contributor credit recorded once in the commit. The lock slice spans 16 files; the supporting metadata-test fixture adds one file containing two mock lines, for 17 changed files total. Grok, coupon, and link changes are excluded.

Verification

Validation identity:

Candidate: 8f3916b4ce66406b3d8c3003e8a122841ce2e554.
Base: upstream dev at e6c6e13a22e465812f509a423990e9f09e32b1d0.
Tested tree: cfbfd66a5f115640e2a92f9b20c3121c407cadd8.
Rebased the previously reviewed foundation onto the latest dev in an isolated worktree. The three overlapping files merged without conflicts; upstream test registrations and Anthropic sidecar management behavior were preserved.
The repository test wrapper used pinned Bun 1.4.0. Runtime and owned-path component checks used Bun 1.4.2. Passing checks were reused when the source tree was unchanged.

Focused writer, feature, API, and CLI regressions: 305 passed, zero failed, including 39 lock/writer tests.

bun scripts/test.ts \
  ./tests/codex-integration/codex-v2-gate.test.ts \
  ./tests/codex-integration/codex-config-write-lock.test.ts \
  ./tests/codex-integration/codex-prompt-lock.test.ts \
  ./tests/codex-integration/codex-features-cache.test.ts \
  ./tests/cli/cli-agent-runtime-settings.test.ts \
  ./tests/cli/cli-v2-runtime.test.ts

Complete account API file: 347 passed, zero failed.

bun scripts/test.ts ./tests/codex-integration/codex-auth-api.test.ts

Incoming management API regression coverage: 20 passed, zero failed.

bun scripts/test.ts \
  ./tests/server/management-sidecar-anthropic-instance.test.ts \
  ./tests/claude-integration/claude-sidecar-override.test.ts

Identical two-file comparison command:

bun scripts/test.ts \
  ./tests/codex-integration/codex-inject-write-lock.test.ts \
  ./tests/codex-integration/codex-auth-api.test.ts

Candidate: 356 passed, eight failed.
Unchanged latest dev: 354 passed, ten failed.
All eight remaining failures are the same native injection/admission cases on both trees. This sandbox's system temporary directory fails the production ownership check (namespace_unsafe).
The other two upstream failures were quota-network timeouts in metadata-only tests. Earlier observation-only traces captured requests settling after the existing five-second deadline. Only those two fixtures were mocked; assertions, test deadlines, production transport, and ownership guards were retained.

Scoped changed run: 694 passed, eight failed across 11 selected files. These are the same eight native admission failures above. This was a scoped import-graph run, not the full suite.

bun scripts/test.ts \
  --changed=review-upstream/resume-dev \
  ./tests/codex-integration/codex-v2-gate.test.ts \
  ./tests/codex-integration/codex-config-write-lock.test.ts \
  ./tests/codex-integration/codex-prompt-lock.test.ts \
  ./tests/codex-integration/codex-features-cache.test.ts \
  ./tests/cli/cli-agent-runtime-settings.test.ts \
  ./tests/cli/cli-v2-runtime.test.ts \
  ./tests/codex-integration/codex-features-residual.test.ts \
  ./tests/codex-integration/codex-journal.test.ts \
  ./tests/codex-integration/codex-inject-write-lock.test.ts \
  ./tests/codex-integration/codex-inject-v1-reconcile.test.ts \
  ./tests/codex-integration/codex-journal-recovery.test.ts \
  ./tests/test-layout.test.ts \
  ./tests/test-layout-tooling.test.ts \
  ./tests/ci-workflows/file-size-ratchet.test.ts \
  ./tests/gui/gui-static.test.ts \
  ./tests/codex-integration/codex-pool-refresh-backoff.test.ts \
  ./tests/codex-integration/codex-auth-api.test.ts

Here review-upstream/resume-dev is a local fetched ref pointing to the exact base SHA above.

Layout and file-size guards: 28 passed, zero failed.

bun scripts/test.ts \
  ./tests/test-layout.test.ts \
  ./tests/test-layout-tooling.test.ts \
  ./tests/ci-workflows/file-size-ratchet.test.ts

These checks passed:

bun run typecheck
bun run structure:check
bun run privacy:scan

A local Bun 1.4.2 component fixture passed seven checks using a supplied, owned database path: real SQLite/config-lock exclusion across processes, one-shot publication, explicit held-handle writes, commit/rollback, and symlink refusal. It also verified that normal production admission still refuses the unsafe system namespace without changing config bytes. This component evidence does not establish successful admission through the normal production namespace.

Full-suite exception: a full local run was disproportionate to this scoped foundation in the available worktree environment. Focused regressions and the scoped changed run were completed; the eight native admission failures remain documented and require validation in a normal environment.

The full local suite, successful normal-namespace native admission, macOS, Windows, and an actual browser session remain unexecuted locally. This foundation changes no GUI paths. Current-head remote CI will be inspected after publishing. Keep this PR in draft while the outstanding validation and review are completed.

The lock coordinates cooperating OpenCodex writers. Existing witness and byte checks remain necessary for native or manual writers outside that coordination.

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.

Coordinate cooperating config writers through one explicit lock handle. Fence stale-lock takeover and initialization, and select journal replay and live-owner checks within the same config write section. Stub quota transport in two metadata-only test fixtures; retain every assertion and timeout while keeping synthetic credentials offline. Carries the Codex config writer work from c7ac1a0, with reproduced lock takeover and journal selection regressions fixed. Co-authored-by: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>

coderabbitai · 2026-10-08T21:16:00Z

Important

Draft PR not reviewed

Draft PRs are not automatically reviewed by default.

Trigger a manual review

To automatically review draft PRs, update your CodeRabbit configuration:

reviews:
  auto_review:
    drafts: true

Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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

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

github-actions · 2026-10-08T21:16:03Z

✅ Deterministic PR hygiene checks passed.

…nsation The locked compensation hardening deliberately defers journal restoration until both config and profile are restored. The integration expectation still assumed independent journal restoration from origin/dev and the #6774 carry (51c1f18), so a real EACCES on the profile produced the additional journal entry and failed hosted CI despite preserving all current bytes. Retain production behavior: the wp3 contract requires the current journal to remain recovery authority when artifact compensation fails. Update the expected deferred journal diagnostic and cover matching, absent, and different journal preimages, asserting that the current journal survives in every case. Historical-function replay confirms that the older compensation deletes a current journal for an absent preimage after the profile restore has failed. Co-authored-by: 김상훈 <luvs01@hanmail.net> Co-authored-by: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>

…d recovery to it (carry #6774) (#6811) * fix(codex): serialize config writes and bind recovery to locked journal Coordinate cooperating config writers through one explicit lock handle. Fence stale-lock takeover and initialization, and select journal replay and live-owner checks within the same config write section. Stub quota transport in two metadata-only test fixtures; retain every assertion and timeout while keeping synthetic credentials offline. Carries the Codex config writer work from c7ac1a0, with reproduced lock takeover and journal selection regressions fixed. Co-authored-by: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> * fix(codex): harden the config write lock carry Key the lock by the canonical write destination and revalidate it immediately before every rename, including prompt durable writes and the Windows replace retries. Keep journal selection, recovery, removal and cleanup inside one held section, and leave an absent Codex home alone. Record host and process-start evidence with each lock and claim. Only a same-host owner proven dead is taken over; foreign-host, hostless and unverifiable records report a non-retryable unsafe state naming the lock. Validate namespace types and POSIX ownership; on Windows the claims directory is hardened through the existing ACL helper. Bound acquisition with a finite timeout on a monotonic clock, release the prompt store lock when config acquisition throws, and pin native feature children to the held canonical home. A native transition refuses when the locked file is not config.toml, and an alias retarget during the child restores the locked file from its preimage. Co-authored-by: 김상훈 <luvs01@hanmail.net> Co-authored-by: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> * fix(codex): recover pre-staging config on native validation failures Capture canonical bytes or absence before transition staging and protect all later validations, publications, child errors, and postconditions in one recovery scope. Restore only the witnessed canonical destination; retain the original private preimage when replacement makes recovery unsafe. Cover alias retargets before wrapper entry and initial spawn validation, canonical replacement before spawn, and failed standalone toggle rollback. Co-authored-by: 김상훈 <luvs01@hanmail.net> Co-authored-by: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> * test(codex): preserve recovery journal after unreadable profile compensation The locked compensation hardening deliberately defers journal restoration until both config and profile are restored. The integration expectation still assumed independent journal restoration from origin/dev and the #6774 carry (51c1f18), so a real EACCES on the profile produced the additional journal entry and failed hosted CI despite preserving all current bytes. Retain production behavior: the wp3 contract requires the current journal to remain recovery authority when artifact compensation fails. Update the expected deferred journal diagnostic and cover matching, absent, and different journal preimages, asserting that the current journal survives in every case. Historical-function replay confirms that the older compensation deletes a current journal for an absent preimage after the profile restore has failed. Co-authored-by: 김상훈 <luvs01@hanmail.net> Co-authored-by: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> * fix(codex): pass rename validation through a hooks object The Lab activation walk follows free calls; an optional callback parameter invoked as a free call has no declaration to inspect. Carry it as hooks.validateBeforeRename like the atomic writer does. Co-authored-by: 김상훈 <luvs01@hanmail.net> Co-authored-by: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> * Avoid per-acquisition Windows subprocesses in config write locks Cache lazy owner identity probes, including unavailable results, and probe other process starts only when liveness permits takeover. Rely on profile ACLs for claims directories and separate contender startup from acquisition deadlines. Co-authored-by: 김상훈 <luvs01@hanmail.net> Co-authored-by: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> * fix(codex): acquire Windows config locks without PowerShell identity probes Resolve reg.exe through the trusted Windows system directory and query MachineGuid with argv, strict parsing, a three-second timeout and a process-local cache. Read Linux machine-id files directly; retain macOS identity probes. Record this Windows process's start identity as unknown without spawning. Missing start evidence disables only the PID-reuse comparison: live or unknown PIDs remain busy, while matching-host owners proven dead twice remain recoverable past the grace window. Failed host discovery still publishes hostless records that later contenders must preserve as unsafe. Cover registry parsing, command failure, caching, trusted executable resolution, and unknown-start takeover. Align Windows journal and unsafe writer fixtures with the canonical production home; retain the contention deadline while separately bounding successful Windows initialization. Document the identity sources and conservative recovery contract. Co-authored-by: 김상훈 <luvs01@hanmail.net> Co-authored-by: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> * fix(tests): isolate Windows Codex home aliases Only set, inspect, and restore Codex_Home on platforms with case-sensitive environment names. On Windows that assignment overwrites CODEX_HOME and makes the held-home fixture operate on B instead of A. Keep the ORCA override removal assertion on every platform and retain the POSIX case-variant checks. Strip all inherited CODEX_HOME case variants before spawning the injector fixture. A duplicate Windows environment name can select the inherited home, where v2 is off, so reconciliation never calls the toggle. Preserve the injector result and effective home in diagnostics even when no child ran. The legacy eligibility mock bypasses coordinator namespace admission, and the canonical junction/lock checks succeed with the isolated environment; no production-path change is needed for this fixture defect. Verification with Bun 1.4.0 on macOS: - Simulated case-insensitive child environment: 0 pass / 1 fail before the environment cleanup, 1 pass / 0 fail after it; effective home and skipped toggle matched the Windows failure mechanism. Native Windows not rerun. - bun scripts/test.ts with codex-config-write-lock, codex-prompt-lock, codex-inject-write-lock, codex-v2-gate, both layout files, and file-size ratchet: 305 pass / 1 skip / 0 fail across seven files. - bun run typecheck: exit 0. Co-authored-by: 김상훈 <luvs01@hanmail.net> Co-authored-by: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> * fix(codex): canonicalize lock destinations with native realpath On Windows the JavaScript realpath can leave 8.3 short components unexpanded, so the same config could canonicalize to different strings depending on whether the file existed, and a held handle was refused as unsafe. Use realpathSync.native for every lock destination, home and alias comparison, as the Codex path module already does. Reproduced and verified with an 8.3 TEMP on a native Windows host. Co-authored-by: 김상훈 <luvs01@hanmail.net> Co-authored-by: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> * fix(codex): return contention for Windows lock sharing violations Retry claim, reservation, evidence and quarantine filesystem operations through the existing bounded Windows retry envelope. Exhaustion stays busy and cannot authorize takeover, including sharing contention followed by EEXIST. Revalidate the release token before each retry and preserve writer errors. Co-authored-by: 김상훈 <luvs01@hanmail.net> Co-authored-by: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> * fix(codex): recover abandoned claims and revalidate every retry Remember completed local reservations whose bounded cleanup fails and retry their removal before acquisition, preserving active reservations. Revalidate captured parent and entry identities and exact claim evidence before destructive attempts so replacements survive retry sleeps. Preserve normal ENOENT handling when a scanned peer releases its claim. Co-authored-by: 김상훈 <luvs01@hanmail.net> Co-authored-by: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> * fix(codex): fence stale lock takeover and cleanup retries Share claim evidence guards with reusable prompt/config locks. Revalidate observed bytes, inode and parent before every takeover or cleanup attempt, and bind exclusive creation and release retries to their captured namespace. Cover sharing-error replacement races deterministically on all three codes. Co-authored-by: 김상훈 <luvs01@hanmail.net> Co-authored-by: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> * fix(codex): keep malformed stale lock records as unsafe evidence The fenced stale-record reader parsed the observed body without the token and pid shape gate, so an expired dead-owner record missing its token could be quarantined. Apply the same gate as readRecord. Co-authored-by: 김상훈 <luvs01@hanmail.net> Co-authored-by: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> * fix(codex): carry Windows retry callbacks as step members The Lab activation walk follows free calls and cannot inspect callback parameters. Pass the operation and its observers in one step object, as rename validation already does. Co-authored-by: 김상훈 <luvs01@hanmail.net> Co-authored-by: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> --------- Co-authored-by: 김상훈 <luvs01@hanmail.net> Co-authored-by: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>

lidge-jun · 2026-10-09T09:38:55Z

Thank you, @luvs01! This change landed on dev through the maintainer carry #6811 (squash commit c3bbaaa), which credits you with a Co-authored-by trailer. The carry keeps your serialized Codex config writes and locked-journal recovery, and adds Windows fixes found on a native Windows run: lock identity no longer depends on PowerShell lookups, 8.3 short paths are normalized, and sharing violations retry instead of failing the lock. Closing this PR as superseded by #6811.

github-actions Bot added the bug Something isn't working label Oct 8, 2026

This was referenced Oct 9, 2026

fix(cli): admit update restart past an inactive service record bound to a full record fingerprint #6808

Merged

fix(codex): serialize config writers under one canonical lock and bind recovery to it (carry #6774) #6811

Merged

lidge-jun closed this Oct 9, 2026

Navigation Menu

Repository navigation

fix(codex): serialize config writes and bind recovery to locked journal - #6774

fix(codex): serialize config writes and bind recovery to locked journal#6774
luvs01 wants to merge 1 commit into
lidge-jun:devfrom
luvs01:publication/codex-config-write-lock-20261008

luvs01 commented Oct 8, 2026

Uh oh!

coderabbitai Bot commented Oct 8, 2026

Draft PR not reviewed

Uh oh!

github-actions Bot commented Oct 8, 2026

Uh oh!

lidge-jun commented Oct 9, 2026

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

luvs01 commented Oct 8, 2026

Summary

Verification

Checklist

Uh oh!

coderabbitai Bot commented Oct 8, 2026

Draft PR not reviewed

Uh oh!

github-actions Bot commented Oct 8, 2026

Uh oh!

lidge-jun commented Oct 9, 2026

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants