fix(combos): a pool-held 429 must not park the combo target past the pool

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;
       },

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 <window> 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 <window> 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
 

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", () => {