[WRONG BRANCH] fix(codex): fence entitlement credential refreshes - #393
[WRONG BRANCH] fix(codex): fence entitlement credential refreshes#393luvs01 wants to merge 1 commit into
Conversation
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
Important
This repository does not receive automatic reviews because it has fewer than 10 stars. ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: 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: 620f3c8db0
ℹ️ 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".
| ): Promise<CodexModelEntitlementSnapshot> { | ||
| const resolve = deps.resolve ?? resolveCodexModelEntitlements; | ||
| const lease = (deps.acquireNativeMain ?? tryAcquireNativeMainProfileClaim)(); | ||
| if (!lease) return resolve(config, excludeNativeMain(options)); |
There was a problem hiding this comment.
When startup ownership acquisition or journal recovery is still pending, startNativeMainStartupLifecycle reports native-main traffic as blocked, but the startup flow in src/cli/index.ts proceeds from startServer to syncCodexOnStartIfEnabled without awaiting that lifecycle. This branch therefore lets syncCatalogModels commit a Pool-only entitlement snapshot, removing main-only account-gated models from the on-disk Codex catalog; when recovery later succeeds, nothing automatically reconverges that catalog, so the models remain absent until a separate sync occurs. Treat this as a non-committable/retryable snapshot for catalog writers, or schedule convergence when the startup gate opens.
Useful? React with 👍 / 👎.
| // A foreign exclusive holder or an unsupported claim filesystem makes | ||
| // main unavailable; it must not suppress independent Pool discovery. | ||
| if (!(error instanceof NativeProfileError)) throw error; | ||
| return await resolve(config, excludeNativeMain(options)); |
There was a problem hiding this comment.
When the cross-process claim is busy or unavailable, this fallback performs the entire Pool-only resolution before the finally releases the process-local native-main lease. A concurrent local profile switch therefore sees a native-main request in progress and waits for unrelated Pool credential refreshes/model fetches; those can outlast the switch's 10-second drain deadline and make the switch fail with MAIN_REQUESTS_ACTIVE even though the fallback never reads main. Release the lease before starting the independent Pool-only resolution.
Useful? React with 👍 / 👎.
|
✅ Deterministic PR hygiene checks passed. |
github-actions
Bot
changed the title
⏳ DRAFT
What to do
Its title has been prefixed with |
1 participant
Motivation
auth.jsonto be refreshed or rewritten while native-main lifecycle, recovery, or profile-switch drains intend the physical native identity to be untouchable.Description
resolveAdmittedCodexModelEntitlementsthat acquires the process-local native-main lease, uses a cross-process shared claim for the credential read (so refreshes are fenced), and excludesMAIN_CODEX_ACCOUNT_IDwhen lifecycle admission cannot be obtained or the shared claim is unavailable (file:src/codex/model-entitlement-admission.ts).GET /v1/modelshandler to use the admitted/fenced resolver instead of calling the refresh-capable resolver directly (file:src/server/index.ts).src/codex/catalog/sync.ts,src/codex/convergence.ts).tests/codex-model-entitlement-admission.test.ts).Testing
bun test tests/codex-model-entitlement-admission.test.ts tests/codex-model-entitlements.test.ts, which reported 23 passing tests and 0 failures across those files.bun run typecheck, which succeeded (no emits reported).bun run privacy:scan, which passed.bun run test:changedbut it could not run due to the local branch not resolving the upstream comparison ref; focused tests above were used instead as recommended by repo guidance.Codex Task