Repository navigation
fix(combos): peek the round-robin ring in the combo's sibling check - #7230
vadymhimself wants to merge 1 commit into
Conversation
The combo layer asks the generic OAuth rotator only whether a healthy sibling exists; it never dispatches to the account the rotator names. Under pool.kernel round-robin the rotator committed that pick, advancing the ring and making it sticky to an account no request was sent to. Add a `commit` flag to rotateGenericOAuthAccountOn429 (default true). The combo passes false: the failed account is still cooled, but the ring is only peeked. Co-Authored-By: Claude Code <noreply@anthropic.com>
|
⏳ DRAFT
What to do
Review readiness checklist
✅ 4/4 boxes ticked. This pull request was already a draft. Its draft status will be preserved after every issue above is resolved. |
|
@coderabbitai full review |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info
📝 Walkthrough
Merge Risk: ⚪ Minimal · up to
|
✅ Action performedFull review finished. |
|
Being carried into #7236 with your authorship preserved; this PR will be closed when the bundle lands. |
#7230, #7231, #7232, #7233) (#7236) Carries four vadymhimself follow-ups onto dev: - #7230: the combo sibling check peeks the round-robin ring without advancing it, while the failed account's cooldown is still recorded. - #7231: a Codex account move after a 402 is recorded as quota recovery, separate from 429 rate recovery, through persistence, metrics, analytics and Logs labels. - #7232: the Anthropic server-tool input buffer is capped on the incoming fragment (4,096 code units), so one oversized fragment cannot exceed the bound. - #7233: refused-then-served usage rows are priced from the served attempt, and client cancels keep an unknown cost. A usage-less refusal never changes a row's pricing classification. A row with a leading refusal summarizes exactly like the same row without it, under dev's existing partial-cost contract. Co-authored-by: Vadym O <bolein95@gmail.com>
|
Landed on dev via #7236 in a02cecd, carried with Co-authored-by: Vadym O bolein95@gmail.com. That bundle is reviewed and validated (lidge full suite with 0 new failures, mini Windows focused 48/48, hosted CI green). #7233's pricing kept dev's existing partial-cost contract and pins refusal invariance. Details are on #7236. Thank you, @vadymhimself. Closing this one as superseded by that landing. |
Summary
core-combo.tssibling check): withpool.kernelon and the generic OAuthround-robinstrategy, a combo-level 429 after the account budget was spent calledrotateGenericOAuthAccountOn429only to test whether a healthy sibling exists. That rotator commitspickRoundRobinAccount, so the pool cursor moved to an account no request was sent to.rotateGenericOAuthAccountOn429/rotateGenericOAuthAccountOnRefusaltakecommit = true. The combo passesfalse, so the spent account is still cooled, but the successor is only peeked via the existingpeekRoundRobinAccount.Follow-up to the batch-2 bundle (#7224/#7225), which landed at a head from before this review finding was fixed on the source PR. The same commit was offered on the bundle.
Verification
bun test tests/oauth/generic-oauth-combo-account-cooldown.test.tsgives 2 pass, 1 fail. After a 4-of-5-spent request the ring stays on account 5, which was never sent.bun x tsc --noEmit: clean.tests/test-layout.test.tsandtests/ci-workflows/file-size-ratchet.test.ts: pass.Checklist
🤖 Generated with Claude Code
Review readiness checklist
This PR stays in draft until every box below is ticked. Tick all four boxes once the requirements are met:
Required local validation passed; commands, results, and any full-suite exception are documented.
I pushed my PR to a recent dev commit (at most 10 behind; a maintainer may still ask for the exact tip before merge).
I resolved all correct Codex and CodeRabbit findings.
My PR is ready for review.
Summary by CodeRabbit