fix(codex): skip empty native-profile stage sweeps (#1120) - #1134
Conversation
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. 🗂️ Base branches to auto review (2)
Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
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 |
|
✅ Deterministic PR hygiene checks passed. |
![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: 94c5ccf596
ℹ️ 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".
| * can never be mistaken for an unused profile subsystem. | ||
| */ | ||
| stageSweepRequired(): boolean { | ||
| for (const path of [this.context.stageRegistryPath, this.context.stagingRoot]) { |
There was a problem hiding this comment.
When an upgraded installation still has preview-era state under context.legacyRootDir—especially staging/<homeId>/auth.json—but neither current stage path exists, both lstatSync calls return ENOENT and startup skips the sweep. Previously, sweepStages() entered withLock(), where assertNoLegacyNativeProfileState() rejected this state and kept the native-main gate blocked; this change instead marks traffic ready while legacy staged credentials remain unresolved. Include legacy-state validation in this preflight and treat any legacy state or inspection failure as sweep-required.
AGENTS.md reference: AGENTS.md:L218-L224
Useful? React with 👍 / 👎.
2 participants
Summary
Stops a zero-profile installation from fencing itself on a stage-sweep lock it never needed.
runOwnedStageSweep(src/codex/native-profile-startup.ts:131) always calledmanager.sweepStages(), andsweepStages(src/codex/native-profile-manager.ts:916) entered the locked path even when there was no stage registry and no staging tree at all. A transient lock or Windows ACL failure then left the native-main gate instage-cleanup-required, native requests surfaced a 503 that read like upstream capacity pressure, anddoctor/recovercontended on the same profile transaction — the catch-22 in the report.The sweep is now skipped only when both the stage registry and this instance's staging tree are provably absent. Any artifact that is present, or any path that cannot be proven absent, still enters the existing locked fail-closed cleanup, and owner/claim protection for the physical native-main credential is untouched. The 503 now identifies local native-profile maintenance instead of upstream capacity.
Closes #1120.
Attribution
This is @Ingwannu's work from #1124, cherry-picked with their authorship intact. #1124 stays open for them. The absence-only framing is the right instinct here: the regression test pins that a zero-stage install cannot be fenced and that present-or-unreadable state still fails closed, which is what keeps this safe on a credential path.
Reported by @MarcusNeufeldt (#1120), whose report correctly separated "the lock file exists" from "a transaction is held".
Stack 2 of the 260806 attribution campaign, stacked on #1133. Planning unit:
devlog/_plan/260806_stacked_bug_campaign/.Verification
bun test tests/native-profile-stage-lifecycle.test.ts tests/codex-auth-context.test.ts tests/native-profile-drain-server.test.ts— 51 pass, 0 failbun run typecheck— exit 0bun run privacy:scan— passedbun run testvia the repository prepush gate — passedChecklist