Skip to content

fix(responses): rotate a pooled API key on a native passthrough 429 - #7323

Closed
vadymhimself wants to merge 1 commit into
lidge-jun:devfrom
vadymhimself:fix/responses-passthrough-key-pool-429
Closed

vadymhimself wants to merge 1 commit into
lidge-jun:devfrom
vadymhimself:fix/responses-passthrough-key-pool-429

Conversation

@vadymhimself

@vadymhimself vadymhimself commented Oct 18, 2026 •

Copy link
Copy Markdown
Contributor

Summary

  • The native Responses passthrough returns before the adapter recovery loop that rotates pooled API keys. A key-pool provider (two or more apiKeyPool entries, active-key selection) therefore returned a 429 to the client on this wire, and kept using the same key on the next request, while the identical refusal rotates to the next key on the adapter dispatch, continuation and native Chat paths.
  • New helper rotatePassthroughKeyOn429 (src/server/responses/passthrough-key-rotation.ts) adds the missing arm after the same-key retryOn429 loop and before any output: it rotates through rotateProviderTransportOn429 (which records the cooldown), rebinds the reasoning-replay scope, and refetches through the existing rebuildAndRefetch("key-429"), then re-enters the passthroughRecovery loop like the other arms.
  • Guards: synthesized replay refusals do not rotate or cool a key; the shared send budget is checked before the 429 body is cancelled, so a spent budget still returns the real 429 with its Retry-After; distributed (round-robin) selections keep the existing cool-without-replay behaviour from feat(providers): carry opt-in API key round-robin and per-key pacing (#6937) #7135 unchanged.
  • The helper lives in its own module because passthrough-dispatch.ts sits 4 lines under the file-size threshold; the dispatch file grows by 3 lines. The module is registered in tests/helpers/responses-core-source.ts, and the hasKeyPoolFailover recovery-site contract in tests/oauth/generic-oauth-failover.test.ts moves from 7 to 8 with the new site named.

Verification

  • New test native Responses rotates a non-distributed key pool on 429 before output in tests/responses/responses-native-key-cooldown.test.ts, run before the src change: bun test tests/responses/responses-native-key-cooldown.test.ts -> 17 pass, 1 fail (Expected: 200, Received: 429). After the change: 18 pass, 0 fail. The test also asserts the second send carries the second key and the first key is cooling.
  • Existing distributed cases in the same file (native Responses cools genuine distributed 429 without replay, synthetic-refusal and opt-out cases) pass unchanged.
  • bun test tests/oauth/generic-oauth-failover.test.ts tests/responses/responses-core-modules.test.ts tests/ci-workflows/structure-ssot.test.ts -> pass.
  • bun x tsc --noEmit -> exit 0. bun scripts/file-size-ratchet.ts -> passed (passthrough-dispatch.ts 1999 lines).
  • bun run test:changed (Bun 1.4.3 via OCX_TEST_RUNNER_BUN) -> 15927 pass, 34 skip, 2 fail across 637 files. One failure was the rotator-count contract above, fixed in this PR (67 pass after). The other, tests/cli/cli-connect-readiness.test.ts "a rejected preferred runtime falls back without rewriting the saved selection", also fails on an unmodified upstream/dev checkout on this machine (local-environment dependent) and is unrelated.
  • Full bun run test was not run locally; the remainder is left to CI.

Checklist

  • Scope stays focused and avoids unrelated cleanup.
  • Docs or release notes were updated when needed. (One sentence added to structure/transports/responses-failover.md.)
  • Security-sensitive changes were reviewed for secrets, auth, and unsafe defaults. (Credential selection only moves within the configured key pool, through the existing rotator and its cooldown/ownership rules; no secrets are logged.)

Source: carried from the vadymhimself fork build (passthrough hunk of commit 8fb1bbb), re-authored against the current passthroughRecovery loop and limited to non-distributed pools so #7135's distributed contract is untouched.

🤖 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

  • New Features

    • Native Responses requests can retry with another eligible API key after an upstream rate-limit response, subject to the request’s retry budget.
    • Key-pool documentation now describes failover, admission, cooldown, cancellation, and replay behavior.
  • Tests

    • Added coverage verifying that a rate-limited key is cooled down and the request retries with another key.

The native Responses passthrough returns before the adapter recovery loop
that rotates pooled keys, so a key-pool provider's 429 was returned to the
client while the same refusal rotates on the adapter and chat-native wires.
Rotate to the next key before output, inside the shared send budget.
Distributed round-robin selection keeps its cool-without-replay behaviour.

Co-Authored-By: Claude Code <noreply@anthropic.com>
@github-actions

Copy link
Copy Markdown
Contributor

✅ Deterministic PR hygiene checks passed.

@github-actions github-actions Bot added the bug Something isn't working label Oct 18, 2026
@github-actions

github-actions Bot commented Oct 18, 2026 •

Copy link
Copy Markdown
Contributor

⏳ DRAFT

  • review readiness checklist open (0/4 boxes ticked).

What to do

  • Tick all four boxes in the PR description once you're done (currently 0/4).

Review readiness checklist

  • ⬜ 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.

0/4 boxes ticked.

This PR stays in draft until every box above is ticked.

@github-actions
github-actions Bot marked this pull request as draft October 18, 2026 15:49
@coderabbitai

coderabbitai Bot commented Oct 18, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: Repository: lidge-jun/opencodex/.coderabbit.yaml
  • Review profile: ASSERTIVE
  • Plan: Advanced
  • Run ID: 17c7814a-480d-476a-b8b5-7dc982f327af
📥 Commits

Reviewing files that changed from the base of the PR and between 8640c2f and 21cdedd.

📒 Files selected for processing: 6
  • src/server/responses/passthrough-dispatch.ts
  • src/server/responses/passthrough-key-rotation.ts
  • structure/transports/responses-failover.md
  • tests/helpers/responses-core-source.ts
  • tests/oauth/generic-oauth-failover.test.ts
  • tests/responses/responses-native-key-cooldown.test.ts

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 3 remain after this review.

📝 Walkthrough

Native Responses attempts passthrough key rotation for eligible upstream 429 responses when the transient send budget remains. A successful rotation replaces the route provider and resumes the recovery loop. The change also adds test coverage and updates failover documentation.

Changes: Native Responses failover

Layer / File(s) Summary
Passthrough 429 rotation helper
src/server/responses/passthrough-key-rotation.ts
Adds rotatePassthroughKeyOn429, which checks that the response is a replayable 429, the provider supports key-pool failover, and the attempted key selection is not distributed. On successful rotation, it cancels the response body, updates the route provider, binds the reasoning replay scope, and calls refetch.
Dispatch integration and validation
src/server/responses/passthrough-dispatch.ts, tests/responses/responses-native-key-cooldown.test.ts, tests/helpers/responses-core-source.ts, tests/oauth/generic-oauth-failover.test.ts, structure/transports/responses-failover.md
Dispatch attempts rotation within the transient send budget and continues the recovery loop with the rotated response. A test checks retry with the second key and cooldown on the first. Supporting test inventory, guard-count expectations, and failover documentation are updated.

Priority: ⬇️ Low

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix

Merge Risk: ⚪ Minimal · up to 21cde

The new key-rotation path retains the existing response behavior when a replacement send fails. No merge-blocking issue is established.

Pre-merge checks: Passed 5
Check name Status Explanation
Description Check Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check Passed The title clearly and concisely describes the primary change: rotating a pooled API key when native Responses passthrough receives a 429 response.
Docstring Coverage Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 2 functions across 5 files. (1 skipped: 1 …
Linked Issues check Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check Passed Check skipped because no linked issues were found for this pull request.
Full details: Docstring Coverage

Explanation

Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 2 functions across 5 files. (1 skipped: 1 unsupported.)

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@Ingwannu

Copy link
Copy Markdown
Owner

Intake/scoped review at 21cdeddfabb10c8853ce9335c46507938300c70b.

The native passthrough 429 branch uses the existing key-pool selector, excludes distributed selection and synthetic non-replayable replies, then rebinds replay ownership before a bounded rebuild. Preserve attempt identity and the exhausted-budget control without cooling a key for a local refusal. Credential/failover review remains required; keep Draft.

Current-head functional workflows are awaiting fork approval; that is pending execution, not a passing test result. No local suite was run for this branch in this intake. No workflow approval, new scan, live configuration change or merge was performed. Required review/readiness gates remain in force.

@lidge-jun

Copy link
Copy Markdown
Owner

Landed on dev as part of the dispatch/usage/failover bundle #7389 (squash bddd61f), with Co-authored-by credit. Adaptations are listed in #7389 (ratchet move for #7323/#7373; the Kiro hop is now fenced to the adapter the failed leg actually resolved). Closing as absorbed. Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants