fix(codex): keep a request-owned main selection out of the shared active account - #5654
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. |
|
Navigate logical layers of code changes, visualize relationships, and explore their blast radius. Note Currently processing new changes in this PR. This may take a few minutes, please wait... ⚙️ Run configurationConfiguration used: Repository: lidge-jun/opencodex/.coderabbit.yaml Review profile: ASSERTIVE Plan: Advanced Run ID: 📒 Files selected for processing (3)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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 |
github-actions
Bot
added
the
intake: hygiene-blocked
|
![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: 0326a4f5cd
ℹ️ 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".
| // operator's persisted activeCodexAccountId is left untouched either way, which is | ||
| // the thing the preference exists to protect. | ||
| promoteActiveCodexAccount(config, strategyPick); | ||
| if (sharesActiveSelection(strategyPick)) promoteActiveCodexAccount(config, strategyPick); |
There was a problem hiding this comment.
For a non-model-scoped request using round-robin or fill-first, pickUnboundStrategyAccount(..., commitSharedActive = !modelScopedSelection) has already called rememberActiveCodexAccount(config, "__main__") before this guard runs. The quota path similarly remains able to commit main through preemption, applyQuotaAutoSwitch, or applyFailureFailover. Therefore, whenever normal ordering selects the request-owned bearer, it can still become the runtime or persisted shared active account and affect later callers. Propagate the no-commit decision into all selection helpers rather than guarding only these post-selection writes.
Useful? React with 👍 / 👎.
* fix(codex): keep request-owned main out of shared active state A request carrying its own main credential makes stored main an ordinary pool candidate for that request only. #5654 guarded three shared-active writes in resolveCodexAccountForThreadDetailed with a local sharesActiveSelection closure, but the same resolve still reached other writers with the request's selectionOptions: pickUnboundStrategyAccount (round-robin and fill-first/reset-first), applyQuotaAutoSwitch, applyFailureFailover, priority preemption, bound-thread quota re-evaluation, and the expired transient-hold detour adoption. When any of them picked main, later requests without the credential read main as the effective or persisted active account. Export sharesActiveSelection from routing/selection.ts and guard every shared-active write reachable with a request's selectionOptions. The picked account still serves the request unchanged; only the shared cursor write is skipped. The post-response promoteAccountId path needs no metadata: a caller-owned main auth context carries accountId null, so a request-owned retry can never place main in promoteAccountId, and a null accountId early-returns before the promote sites. Regression coverage in codex-pool-rotation.test.ts exercises each guarded writer through resolveCodexAccountForThreadDetailed with a request-owned main, plus a stored-main-live control per scenario proving the cursor does move when the credential is not request-owned. * test(codex): exercise bound request-owned paths in their own scope * test(codex): move the request-owned main cases into their own file
1 participant
Summary
Full-platform CI on dev failed three
Codex auth contextcooldown-fallback tests on Linux, macOS and the macOS control lane after #5024 landed on top of #5609.#5024 lets main take part in Pool ordering when a request carries its own main bearer. When the active Pool account was in cooldown,
resolveCodexAccountForThreadDetailedthen picked that request-owned main as the fallback and saved it withsetActiveCodexAccount, which also writes the config file. Every later request followed that saved main, including a caller with an unreadable identity and a caller that owns the cooled subscription, so they were served instead of refused, andactiveCodexAccountIdread__main__instead of the operator's Pool account.This adds
requestOwnedMainCredentialto the selection options.resolveCodexAuthContextsets it for request-scoped main credentials, and the resolver no longer writes main back as the shared active account (or promotes it) when main is live only through that request's bearer. Main still wins the selection for that request, which is what #5019 asked for.Verification
bun test tests/codex-integration/codex-auth-context.test.ts: 80 pass, 0 fail. With the source change reverted the same file fails exactly the three dev CI tests (77 pass, 3 fail).bun testover the ten codex-integration routing, pool, auth-context, account-selection and hard-lock files: 467 pass, 0 fail.bun x tsc --noEmit: exit 0.Checklist
dev