fix(codex): keep a request-owned main out of shared active state - #5663
Conversation
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.
|
✅ Deterministic PR hygiene checks passed. |
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 (6)
✨ 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 |
![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: 538b37bf40
ℹ️ 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".
| }); | ||
| }); | ||
|
|
||
| describe("a request-owned main serves the request without becoming the shared active account", () => { |
There was a problem hiding this comment.
This added block grows codex-pool-rotation.test.ts from 1,865 to 2,132 lines; because the file has no committed cap and the ratchet threshold is 2,000, evaluate classifies it as NEW_OVERSIZED, so the file-size ratchet: repository CI gate will fail. Move these scenarios into a sibling test file and register that file in both test-layout maps.
AGENTS.md reference: AGENTS.md:L267-L270
Useful? React with 👍 / 👎.
리뷰 · 우선순위 61 / 80이 PR은 요청이 자기 메인 계정 열쇠를 들고 왔을 때, 그 선택이 다음 요청의 공용 활성 계정으로 남지 않게 합니다. 기준 브랜치는 이번 변경은 사용량 자동 전환, 라운드로빈, fill-first, 우선순위 선점, 묶인 스레드 재평가는 CI에서 통과했습니다. 저장된 main이 살아 있으면 커서가 main으로 움직이는 대조도 통과했습니다. test 1/4과 test 2/4는 실패합니다. 위의 라인 - 라인 - 같은 파일, 만료된 임시 우회 테스트 두 개 - 503을 세 번 적은 뒤 첫 선택이 라인 - 메인테이너의 판단이 필요한 지점
너의 추천 머지하지 마세요. 테스트 파일을 2000줄 아래로 나누세요. 만료 우회 테스트는 기존 503 우회가 이 댓글은 grok-bot이 작성했습니다 |
1 participant
Summary
A request that carries its own main credential (
CodexAccountUsabilityOptions.requestOwnedMainCredential, #5024) makes the storedmainaccount an ordinary pool candidate for that request only. #5654 stopped three writes inresolveCodexAccountForThreadDetailedfrom recording such a pick as the shared active account. The same resolve still reached other writers of shared active state with the request's selection options:pickUnboundStrategyAccount, round-robin and fill-first/reset-first branches (runtime cursor)applyQuotaAutoSwitchunder the defaultquotastrategy (persistedactiveCodexAccountId)applyFailureFailoverBefore: when any of these picked
mainfor a request that owned the main credential, later requests that do not carry that credential readmainas the effective, or even the persisted, active account. No credential moved between callers; only the account id was recorded, but routing, the dashboard and the pin check followed a choice that belonged to one request.After:
sharesActiveSelection(accountId, selectionOptions)is exported fromsrc/codex/routing/selection.tsand guards every shared active-state write reachable with a request's selection options. The picked account still serves the request unchanged; thread affinity and ring bookkeeping are untouched. The local closure insrc/codex/routing.tsis replaced by the shared helper (the file shrinks from 1618 to 1616 lines).Post-response promotion needs no change: a caller-owned main resolves to an auth context with
accountId: null, so a request-owned retry never placesmaininpromoteAccountId.Verification
tests/codex-integration/codex-pool-rotation.test.tsadds five request-owned scenarios resolved throughresolveCodexAccountForThreadDetailed(quota auto-switch, round-robin and fill-first new sessions, priority preemption, bound-thread re-evaluation, expired transient detour). Each asserts the request resolves tomainwhileconfig.activeCodexAccountIdandgetEffectiveActiveCodexAccountId(config)keep the operator's account, and each has a control with stored main live showing the cursor does move tomain.structure/providers/openai-tiers.mdrecords the rule.Checklist
Summary by CodeRabbit