fix(codex): keep two host-lane records that a failed read used to destroy (STA-4735) by brennanb2025 · Pull Request #15289 · stablyai/orca

brennanb2025 · 2026-08-18T09:11:06Z

	Files	Added	Deleted	Net
Test	2	$\color{#1a7f37}{\Huge{\mathbf{+}}}$311	0	$\color{#1a7f37}{\Huge{\mathbf{+}}}$311
Prod	3	$\color{#1a7f37}{\Huge{\mathbf{+}}}$77	$\color{#cf222e}{\Huge{\mathbf{−}}}$7	$\color{#1a7f37}{\Huge{\mathbf{+}}}$70

ELI5

Two files on the host Codex lane were rewritten or replaced because Orca "couldn't find" them — when in fact it just couldn't read them for a moment. One of them is the record of which hook approvals you made versus which Orca made, and losing it means your approval is silently reclassified as Orca's and never promoted again.

What Changed

1. The hook-trust provenance record

.orca-hook-trust-provenance.json is the only thing separating a config.toml trust entry Orca wrote from one the user approved inside Codex. snapshotCodexRuntimeHookTrustProvenance rebuilt it from the current config.toml on every install and refresh — including when the existing record could not be read.

The chain is what makes it permanent, and it runs entirely within one install pass:

promoteCodexRuntimeHookApprovalsToSystem (hook-service.ts:1271) reads the provenance, gets null from the unreadable file, and bails — so a genuine user approval is not promoted.
snapshotCodexRuntimeHookTrustProvenance (hook-service.ts:1391) then overwrites the record from the current config, which now says Orca wrote that entry.
Every later pass compares against that record, sees a match, and skips the approval as "not an approval".

One denied read, and the approval is gone for good.

Only the unreadable case is preserved. An absent or malformed record is still rebuilt, because resetting those is the intent — conflating the two would wedge a user on a corrupt file forever. That distinction has its own test.

2. `fileContentsEqual`

try { return existsSync(targetPath) && readFileSync(targetPath, 'utf-8') === contents }
catch { return false }

false reaches writeRuntimeAuth as "these differ", which sends it to the unconditional write below — replacing a refresh token Codex may have rotated moments earlier with Orca's stale copy. Codex uses rotating refresh tokens, so consuming a rotation twice invalidates the credential permanently; this is the same permanent-logout outcome STA-4422 was reported for.

It now reports a difference only when the bytes were actually compared, and the caller refuses.

Why

These are the two host-lane instances left over after #15046 and #15093. They were catalogued while planning STA-4606 and moved out of it because that ticket is the WSL lane.

Two things deliberately left alone

fileContentsMatchExpected's !existsSync has exactly the same collapse. I wrote the fix and a test for it, then found the test passed with the fix reverted: the write it guards is writeFileAtomicallyIfUnchanged, whose rename-and-compare re-checks the real file and refuses on its own. Rather than ship a guard no test can drive, the fix is reverted and the reasoning is a comment in the code.

The three writeRuntimeAuthAtPath call sites have the same overwrite shape, but all three are on the WSL lane, which STA-4606 restructures. Making it refuse there without that ticket's lane bookkeeping would leave lastWrittenWslAuthJsonByDistro recording a baseline for a write that never happened — a new bug traded for an old one.

Linked Issue

Fixes STA-4735. Same defect class as #15046, #15093, #15279, #15287.

Visual Proof

N/A — no UI change. The observable difference is that a file survives, which the tests assert byte-for-byte.

Testing

Two files, 5 tests: 2 driving the fix, 3 anchoring it.

The rig models one denial coherently — a denied path throws from readFileSync/lstatSync/statSync and reports false from existsSync.

Mutation-checked:

Reverted	Result
`hook-trust-promotion.ts`	"leaves a provenance record it could not read" → red
`runtime-home-service.ts`	"refuses the write instead of overwriting a token it could not read" → red

The credential test also asserts the injected fault was actually consumed by the code under test, so it cannot pass by never reaching the read.

Anchors: a genuinely absent provenance file is still seeded, a malformed one is still replaced, and a readable runtime auth.json is still written normally. Each of these would break under a fix that simply stopped writing.

1068/1068 across codex + codex-accounts.
check-changed-code-quality.mjs: 0 findings. Full pnpm typecheck clean.
Automated tests added
Manual: not run. The trigger is a file-denial race reproduced deterministically by the rig.

Merge note

Carries a byte-identical copy of src/shared/definitive-filesystem-absence.ts, also added by #15279 and #15287, so all three merge cleanly in any order. No dependency between them.

AI Disclosure

N/A — internal.

…troy (STA-4735) `snapshotCodexRuntimeHookTrustProvenance` rebuilt `.orca-hook-trust-provenance.json` from the current `config.toml` on every install and refresh, including when the existing record could not be read. That record is the only thing separating a trust entry Orca wrote from one the user approved inside Codex, so rewriting it after a failed read stamps the approval as Orca-written — and `promoteCodexRuntimeHookApprovalsToSystem`, which runs earlier in the same pass and had already bailed on the same unreadable file, then skips it on every later pass too. One denied read, permanent loss. Only the unreadable case is preserved. A malformed or absent record is still rebuilt, because resetting those IS the intent; conflating the two would wedge a user on a corrupt file forever. `fileContentsEqual` returned `false` from a bare `catch`, so "I could not read this" reached `writeRuntimeAuth` as "these differ" and sent it to the unconditional write below — replacing a refresh token Codex may have rotated a moment earlier with Orca's stale copy. It now reports the difference only when the bytes were actually compared, and the caller refuses instead. `fileContentsMatchExpected`'s `!existsSync` has the same collapse but is left alone deliberately: the write it guards is `writeFileAtomicallyIfUnchanged`, whose rename-and-compare re-checks the real file and refuses on its own, so classifying there would add a guard no test can drive. Noted in a comment. The three `writeRuntimeAuthAtPath` call sites have the same overwrite shape but are all on the WSL lane, which STA-4606 restructures; refusing there without its lane bookkeeping would set a baseline for a write that never happened.

coderabbitai · 2026-08-18T09:13:49Z

📝 Walkthrough

Walkthrough

The change adds definitive filesystem-absence detection for ENOENT and ENOTDIR. Runtime authentication writes now stop when auth.json is unreadable. File comparisons distinguish matches, differences, and read failures. Hook-trust provenance reads and snapshot writes apply the same distinction. Regression tests cover unreadable, missing, malformed, and readable stale files.

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
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.
Title check	✅ Passed	The title clearly summarizes the main fix: preventing failed reads from destroying two host-lane records.
Description check	✅ Passed	The description covers the required change, rationale, issue, testing, visual proof, and AI disclosure sections with sufficient detail.

✨ Finishing Touches

📝 Generate docstrings

Create stacked PR
Commit on current branch

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.}

coderabbitai

🧹 Nitpick comments (1)

src/main/codex-accounts/runtime-home-service.ts (1)

2016-2020: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Shorten the changed TypeScript comments.

Keep only the non-obvious safety invariant. Move detailed regression history to the test name or issue reference.

src/main/codex-accounts/runtime-home-service.ts#L2016-L2020: reduce the credential-overwrite rationale to one short comment.

src/main/codex-accounts/runtime-home-service.ts#L2071-L2075: reduce the comparison contract comment to one short comment.

src/main/codex-accounts/runtime-home-service.ts#L2091-L2095: reduce the conditional-write note to one short comment.

src/main/codex-accounts/sta-4735-unreadable-host-lane-writes.test.ts#L16-L17: remove or shorten the file-level regression summary.

src/main/codex-accounts/sta-4735-unreadable-host-lane-writes.test.ts#L138-L141: shorten the expected-behavior explanation.

src/main/codex-accounts/sta-4735-unreadable-host-lane-writes.test.ts#L153-L154: shorten the readable-file test rationale.

src/main/codex/hook-trust-promotion.ts#L47-L50: reduce the null contract comment to one short statement.

src/main/codex/hook-trust-promotion.ts#L91-L98: reduce the provenance preservation rationale to one short statement.

src/main/codex/sta-4735-hook-trust-provenance-overwrite.test.ts#L7-L11: remove or shorten the file-level regression summary.

src/main/codex/sta-4735-hook-trust-provenance-overwrite.test.ts#L126-L128: shorten the expected-behavior explanation.

src/main/codex/sta-4735-hook-trust-provenance-overwrite.test.ts#L133-L134: shorten the absent-file test rationale.

src/main/codex/sta-4735-hook-trust-provenance-overwrite.test.ts#L142-L144: shorten the malformed-file test rationale.

As per coding guidelines: “Comments must be concise, non-obvious, and brief—prefer one line.”

Source: Coding guidelines

ℹ️ Review info

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro Plus

Run ID: a71fb037-ca10-43b1-b24c-cdd9063279cf

📥 Commits

Reviewing files that changed from the base of the PR and between 96d88dd and 8c17131.

📒 Files selected for processing (5)

src/main/codex-accounts/runtime-home-service.ts
src/main/codex-accounts/sta-4735-unreadable-host-lane-writes.test.ts
src/main/codex/hook-trust-promotion.ts
src/main/codex/sta-4735-hook-trust-provenance-overwrite.test.ts
src/shared/definitive-filesystem-absence.ts

Included review availability: Your plan includes up to 10 reviews per rolling hour; 4 remain after this review.

brennanb2025 · 2026-08-18T19:32:46Z

Verdict

PASS — I found no release-blocking or correctness issues in the PR after rebasing it onto current origin/main. The changed host-lane guards preserve unreadable records and still allow the intended first-launch, malformed-file recovery, and readable-file write paths.

Findings

No new issues found, so there are no additional file:line fixes to request. The reviewed behavior is now:

src/main/codex-accounts/runtime-home-service.ts:2015-2022,2077-2082 distinguishes a byte comparison that actually ran from a failed read; only ENOENT/ENOTDIR is treated as absence, while other errors refuse the overwrite and retry later.
src/main/codex/hook-trust-promotion.ts:99-112 preserves an existing provenance file when it cannot be read, while still rebuilding an absent or malformed file.
src/shared/definitive-filesystem-absence.ts:14-16 centralizes the narrow absence allowlist.

Sweep results

Full src/main/codex and src/main/codex-accounts suites: 1068 passed, 6 skipped.
pnpm typecheck after pnpm install --frozen-lockfile: passed.
node config/scripts/check-changed-code-quality.mjs: 0 new findings across 5 changed files.
git diff --check: clean.
Security/supply-chain, crash/retry, resource/performance, functional, persisted-state/backward-compatibility, cross-platform, and remote/SSH review of the changed span: clean; no UI, renderer, or mobile surface is touched, so no screenshots or Electron QA apply.
Sibling writeRuntimeAuthAtPath call sites remain unchanged deliberately: all three are WSL-lane paths whose bookkeeping is being reworked by STA-4606; refusing there now would record a baseline for a write that did not occur. The expectedContents === null / !existsSync guard remains unchanged deliberately because writeFileAtomicallyIfUnchanged re-checks the real file and is the effective refusal point; the reverted guard had no testable effect.

Mutation checks

Reverted src/main/codex-accounts/runtime-home-service.ts: named test refuses the write instead of overwriting a token it could not read went red because the injected denied read was not consumed and the stale write path remained active.
Reverted src/main/codex/hook-trust-promotion.ts: named test leaves a provenance record it could not read exactly as it found it went red because the denied provenance read was rebuilt from current config.
Both mutation checks were run against the rebased parent with the new test and shared predicate copied in; the anchor tests remained green in each mutation run.

…troy (STA-4735) (stablyai#15289) `snapshotCodexRuntimeHookTrustProvenance` rebuilt `.orca-hook-trust-provenance.json` from the current `config.toml` on every install and refresh, including when the existing record could not be read. That record is the only thing separating a trust entry Orca wrote from one the user approved inside Codex, so rewriting it after a failed read stamps the approval as Orca-written — and `promoteCodexRuntimeHookApprovalsToSystem`, which runs earlier in the same pass and had already bailed on the same unreadable file, then skips it on every later pass too. One denied read, permanent loss. Only the unreadable case is preserved. A malformed or absent record is still rebuilt, because resetting those IS the intent; conflating the two would wedge a user on a corrupt file forever. `fileContentsEqual` returned `false` from a bare `catch`, so "I could not read this" reached `writeRuntimeAuth` as "these differ" and sent it to the unconditional write below — replacing a refresh token Codex may have rotated a moment earlier with Orca's stale copy. It now reports the difference only when the bytes were actually compared, and the caller refuses instead. `fileContentsMatchExpected`'s `!existsSync` has the same collapse but is left alone deliberately: the write it guards is `writeFileAtomicallyIfUnchanged`, whose rename-and-compare re-checks the real file and refuses on its own, so classifying there would add a guard no test can drive. Noted in a comment. The three `writeRuntimeAuthAtPath` call sites have the same overwrite shape but are all on the WSL lane, which STA-4606 restructures; refusing there without its lane bookkeeping would set a baseline for a write that never happened.

coderabbitai Bot reviewed Aug 18, 2026

View reviewed changes

brennanb2025 merged commit ddaaa46 into main Aug 18, 2026
47 checks passed

brennanb2025 mentioned this pull request Aug 19, 2026

fix(codex): stop rebuilding shared Codex state from a read that failed (STA-4823) #15417

Merged

2 tasks done

Navigation Menu

Repository navigation

fix(codex): keep two host-lane records that a failed read used to destroy (STA-4735) - #15289

fix(codex): keep two host-lane records that a failed read used to destroy (STA-4735)#15289
brennanb2025 merged 1 commit into
mainfrom
brennanb2025/sta-4735-host-lane-residuals

brennanb2025 commented Aug 18, 2026 •

edited by github-actions Bot

Loading

Uh oh!

coderabbitai Bot commented Aug 18, 2026 •

edited

Loading

Walkthrough

Uh oh!

coderabbitai Bot left a comment

Uh oh!

brennanb2025 commented Aug 18, 2026 •

edited

Loading

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Conversation

brennanb2025 commented Aug 18, 2026 • edited by github-actions Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

ELI5

What Changed

1. The hook-trust provenance record

2. fileContentsEqual

Why

Two things deliberately left alone

Linked Issue

Visual Proof

Testing

Merge note

AI Disclosure

Uh oh!

coderabbitai Bot commented Aug 18, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

brennanb2025 commented Aug 18, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Verdict

Findings

Sweep results

Mutation checks

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

brennanb2025 commented Aug 18, 2026 •

edited by github-actions Bot

Loading

2. `fileContentsEqual`

coderabbitai Bot commented Aug 18, 2026 •

edited

Loading

brennanb2025 commented Aug 18, 2026 •

edited

Loading