diff --git a/src/server/responses/core-combo.ts b/src/server/responses/core-combo.ts index 622a5a026ee..9079f01a3df 100644 --- a/src/server/responses/core-combo.ts +++ b/src/server/responses/core-combo.ts @@ -48,7 +48,12 @@ import { routeConcreteModel, comboRouteDecisionTrace } from "../../router"; import { memoryModelRouteReason } from "./memory-models"; import { poolAccountProviderLabel } from "../../providers/label"; import { getAccountSet } from "../../oauth/store"; -import { formatAnthropicProviderForLog, getAnthropicAccountHealthSnapshot } from "../../oauth/anthropic-routing"; +import { + formatAnthropicProviderForLog, + getAnthropicAccountHealthSnapshot, + getAnthropicPoolRetryAfterSeconds, + getEligibleAnthropicAccounts, +} from "../../oauth/anthropic-routing"; import { codexAccountLogLabel } from "../../codex/account-label"; import { codexQuotaScopeForModel, getCodexQuotaHealthSnapshot } from "../../codex/routing"; import { isCanonicalOpenAiForwardProvider } from "../../providers/openai-tiers"; @@ -119,6 +124,33 @@ function cooledPoolAccountLabel(config: OcxConfig, providerName: string, modelId } +/** + * True only for the Anthropic OAuth pool's OWN "all accounts cooled" 429. Its Retry-After is the + * earliest per-account cooldown restated (`getAnthropicPoolRetryAfterSeconds`), which the pool + * keeps enforcing locally, so it must not also park the combo target: an account added, resumed or + * cleared a minute later would otherwise sit ignored until the target expired, up to the 24h + * server-delay ceiling. + * + * Provenance, not just current state: the target must be the OAuth pool, no account may have been + * identified for the failure, and the pool must be holding the wait right now. An upstream 429 on + * a pool account always cools and names that account, and an API-key provider never qualifies, so + * a genuine upstream Retry-After still parks the target in full. + */ +export function isAnthropicPoolLocalRefusal( + config: OcxConfig, + providerName: string, + status: number, + failedAccount: string | undefined, + now = Date.now(), +): boolean { + return providerName === "anthropic" + && config.providers[providerName]?.authMode === "oauth" + && status === 429 + && failedAccount === undefined + && getEligibleAnthropicAccounts(now).length === 0 + && getAnthropicPoolRetryAfterSeconds(now) !== null; +} + /** * A combo's execution policy is DECLARED by the combo, not inherited from the single-target * profile. @@ -988,8 +1020,20 @@ export async function executeComboResponses( }); const failedTargetKey = targetKey(pick.target); let failedTargetCooldownRecorded = false; + // The dispatch rewrote this to name the pool account that actually served the turn. + const failedAccount = cooledPoolAccountLabel( + config, + pick.target.provider, + pick.target.model, + poolAccountProviderLabel(childLog.provider, pick.target.provider), + ); + // The target still cools on the pool's own refusal (the anti-hammer guard), but for the local + // fallback rather than the pool's restated per-account Retry-After. + const poolLocalRefusal = isAnthropicPoolLocalRefusal( + config, pick.target.provider, failure.response.status, failedAccount, failureNow, + ); const nextPick = advanceComboAfterFailure(config, pick, { - retryAfter: failure.retryAfter, + retryAfter: poolLocalRefusal ? undefined : failure.retryAfter, resetAt: failure.resetAt, cooldownMs: combo.cooldownMs, now: failureNow, @@ -998,13 +1042,7 @@ export async function executeComboResponses( status: failure.response.status, code: failure.upstreamCode, message: failure.classificationText, - // The dispatch rewrote this to name the pool account that actually served the turn. - failedAccount: cooledPoolAccountLabel( - config, - pick.target.provider, - pick.target.model, - poolAccountProviderLabel(childLog.provider, pick.target.provider), - ), + failedAccount, onCooldownRecorded: target => { failedTargetCooldownRecorded ||= targetKey(target) === failedTargetKey; }, diff --git a/structure/runtime.md b/structure/runtime.md index 85e0a73cb16..c620b6ef18b 100644 --- a/structure/runtime.md +++ b/structure/runtime.md @@ -504,7 +504,7 @@ This is also why the classifier cannot duplicate visible output. Native byte str Regression coverage: `tests/responses/responses-forward-prompt-envelope.test.ts`, `tests/routing/router-combo-failover-classification.test.ts`, `tests/routing/routing-policy-fallback.test.ts`, `tests/helpers/combo-context-overflow-cases.ts`, and `tests/server/server-combo-failover-e2e.test.ts`. -`src/combos/failover.ts` uses a 10-minute fallback for a spent account usage window (codes `usage_limit_exceeded`, `usage_limit_reached`, `1308`, or `usage limit reached` / `usage limit has been reached` / `token-plan quota has been exhausted` prose, including HTTP 502) and for provider-scoped credential or billing failure codes such as `invalid_api_key` and `insufficient_quota`. This duration does not change failure classification or cooldown scope; upstream retry/reset signals and configured durations retain precedence. It caps explicit upstream `Retry-After` target cooldowns at 24 hours while reset-derived, configured, and fallback cooldowns remain capped at 10 minutes. `src/combos/resolve.ts` omits a target cooldown for a 429 only when the failed child identifies one account in an Anthropic OAuth or canonical Codex pool and that account has a live pool cooldown. `src/server/responses/core-combo.ts` checks the pool's health record before passing that attribution; unknown accounts, ambiguous labels, API-key and generic OAuth providers, and other failures keep their ordinary target cooldown. +`src/combos/failover.ts` uses a 10-minute fallback for a spent account usage window (codes `usage_limit_exceeded`, `usage_limit_reached`, `1308`, or `usage limit reached` / `usage limit has been reached` / `token-plan quota has been exhausted` prose, including HTTP 502) and for provider-scoped credential or billing failure codes such as `invalid_api_key` and `insufficient_quota`. This duration does not change failure classification or cooldown scope; upstream retry/reset signals and configured durations retain precedence. It caps explicit upstream `Retry-After` target cooldowns at 24 hours while reset-derived, configured, and fallback cooldowns remain capped at 10 minutes. `src/combos/resolve.ts` omits a target cooldown for a 429 only when the failed child identifies one account in an Anthropic OAuth or canonical Codex pool and that account has a live pool cooldown. `src/server/responses/core-combo.ts` checks the pool's health record before passing that attribution; unknown accounts, ambiguous labels, API-key and generic OAuth providers, and other failures keep their ordinary target cooldown. The Anthropic OAuth pool's own all-accounts-cooled 429 is the one unattributed exception: `isAnthropicPoolLocalRefusal` in `core-combo.ts` requires the `anthropic` provider with `authMode: "oauth"`, status 429, no identified failed account, no eligible pool account and a live pool retry-after. The target then still cools, but for the local fallback instead of the restated per-account `Retry-After`, so a resumed or newly added account is not ignored until a 24-hour target cooldown expires. An upstream 429 on an API-key provider or on an identified pool account keeps its full `Retry-After`. Regression coverage: `tests/routing/always-on-429-failover.test.ts`. ## Combo default effort precedence diff --git a/tests/routing/always-on-429-failover.test.ts b/tests/routing/always-on-429-failover.test.ts index c94bcfd85d7..8b6ac1195d1 100644 --- a/tests/routing/always-on-429-failover.test.ts +++ b/tests/routing/always-on-429-failover.test.ts @@ -14,8 +14,13 @@ import { afterEach, beforeEach, describe, expect, test } from "bun:test"; import { mkdtempSync } from "node:fs"; import { tmpdir } from "node:os"; import { join } from "node:path"; +import { clearComboTargetCooldowns, coolComboTarget, isComboTargetInCooldown } from "../../src/combos/failover"; +import { createTranslatorBudget } from "../../src/lib/translator-budget"; +import { executeComboResponses, isAnthropicPoolLocalRefusal } from "../../src/server/responses/core-combo"; +import type { ResponsesDispatchers } from "../../src/server/responses/core-options"; import { clearAnthropicAccountPoolState, + getAnthropicPoolRetryAfterSeconds, getEligibleAnthropicAccounts, hasAnthropicFailoverQuorum, isAnthropicAccountPoolEnabled, @@ -136,6 +141,110 @@ describe("Anthropic reactive 429 failover without the pool flag", () => { expect(rotateAnthropicAccountOn429(poolAbsent(), ids[0]!, null)).toBe(ids[1]); expect(rotateAnthropicAccountOn429(poolAbsent(), ids[1]!, null)).toBeNull(); }); + + // The combo layer keys its target cooldown off this. While the pool holds the wait, its 429 + // Retry-After is only its own earliest account cooldown restated, so the combo must not also + // park the target on it -- otherwise an account added a minute later sits ignored until the + // target cooldown expires, which for a weekly window is the 24h server-delay ceiling. + test("only the OAuth pool's own all-cooled 429 is a local refusal", async () => { + const ids = await seedAccounts(2); + const oauth = poolAbsent(); + const refusal = (config = oauth, account?: string) => + isAnthropicPoolLocalRefusal(config, "anthropic", 429, account); + expect(refusal()).toBe(false); + const weekly = String(4 * 86_400); + expect(rotateAnthropicAccountOn429(oauth, ids[0]!, weekly)).toBe(ids[1]); + // One account still eligible: an upstream 429 here is about THAT account, not the pool. + expect(refusal()).toBe(false); + expect(rotateAnthropicAccountOn429(oauth, ids[1]!, weekly)).toBeNull(); + expect(getAnthropicPoolRetryAfterSeconds()).toBeGreaterThan(86_400); + expect(refusal()).toBe(true); + // Provenance, not just pool state: an identified account or an API-key provider is upstream + // speaking, so its Retry-After must still park the target in full. + expect(refusal(oauth, "anthropic-p0000000")).toBe(false); + const apiKey = { ...oauth, providers: { anthropic: { ...oauth.providers.anthropic!, authMode: "key" } } } as OcxConfig; + expect(refusal(apiKey)).toBe(false); + expect(isAnthropicPoolLocalRefusal(oauth, "anthropic", 503, undefined)).toBe(false); + // A fresh account makes the pool usable again at once, which a parked target would ignore. + await saveCredential("anthropic", { + access: "access-new", refresh: "refresh-new", expires: Date.now() + 3_600_000, + accountId: "uuid-new", email: "new@example.test", + } as never); + expect(refusal()).toBe(false); + }); + + // The combo-layer half, mirroring core-combo's call site: the target still cools (the + // anti-hammer guard), but for the local fallback rather than the pool's multi-day Retry-After. + // A single account is deliberately never cooled by rotation (see the no-op test above), so the + // pool cannot hold the wait there and an upstream Retry-After keeps parking the target as before. + test("a pool-held 429 cools the combo target for minutes, not for the pool's Retry-After", async () => { + const ids = await seedAccounts(2); + const weekly = String(4 * 86_400); + rotateAnthropicAccountOn429(poolAbsent(), ids[0]!, weekly); + rotateAnthropicAccountOn429(poolAbsent(), ids[1]!, weekly); + const now = Date.now(); + const local = isAnthropicPoolLocalRefusal(poolAbsent(), "anthropic", 429, undefined, now); + expect(local).toBe(true); + const target = { provider: "anthropic", model: "claude-opus-5" }; + clearComboTargetCooldowns("pool-held"); + coolComboTarget("pool-held", target, { now, retryAfter: local ? undefined : weekly, status: 429 }); + expect(isComboTargetInCooldown("pool-held", target, now + 1)).toBe(true); + expect(isComboTargetInCooldown("pool-held", target, now + 10 * 60_000)).toBe(false); + clearComboTargetCooldowns("pool-held"); + }); + + // Production wiring, not a mirror of it: drive the real combo failure path with a child that + // answers exactly what the pool answers when every account is cooled -- a 429 carrying the + // earliest account reset as Retry-After. The control sends the SAME 429 while an account is + // still eligible, so the only difference between the two runs is who is holding the wait. + test("the combo failure path withholds only the pool's own Retry-After from the target", async () => { + const weekly = String(4 * 86_400); + const target = { provider: "anthropic", model: "claude-opus-5" }; + const config = { + ...poolAbsent(), + combos: { waterfall: { strategy: "failover", targets: [{ ...target }] } }, + } as OcxConfig; + const poolRefusal: ResponsesDispatchers = { + async handleResponses() { + return new Response( + JSON.stringify({ error: { type: "rate_limit_error", message: "All Anthropic OAuth accounts are temporarily rate-limited" } }), + { status: 429, headers: { "content-type": "application/json", "retry-after": weekly } }, + ); + }, + async handleComboResponses() { throw new Error("nested combo dispatch is not expected"); }, + }; + const run = async () => { + const body = { model: "combo/waterfall", input: "hi", stream: false }; + const budget = createTranslatorBudget(); + try { + return await executeComboResponses( + new Request("http://127.0.0.1/v1/responses", { + method: "POST", headers: { "content-type": "application/json" }, body: JSON.stringify(body), + }), + body, "waterfall", config, { model: "", provider: "" }, { translatorBudget: budget }, poolRefusal, + ); + } finally { + budget.dispose(); + } + }; + + // Control: one account is still eligible, so this 429 is upstream speaking and parks in full. + const ids = await seedAccounts(2); + rotateAnthropicAccountOn429(config, ids[0]!, weekly); + clearComboTargetCooldowns("waterfall"); + const controlAt = Date.now(); + expect((await run()).status).toBe(429); + expect(isComboTargetInCooldown("waterfall", target, controlAt + 10 * 60_000 + 1_000)).toBe(true); + + // Pool-held: every account cooled. The target still cools, but only for the local fallback. + rotateAnthropicAccountOn429(config, ids[1]!, weekly); + clearComboTargetCooldowns("waterfall"); + const heldAt = Date.now(); + expect((await run()).status).toBe(429); + expect(isComboTargetInCooldown("waterfall", target, heldAt + 1)).toBe(true); + expect(isComboTargetInCooldown("waterfall", target, heldAt + 10 * 60_000 + 1_000)).toBe(false); + clearComboTargetCooldowns("waterfall"); + }); }); describe("proactive Anthropic routing stays opt-in", () => {