diff --git a/devlog/_plan/260825_owner_backlog_and_bugpr_closeout/110_wp7_oauth_failover.md b/devlog/_plan/260825_owner_backlog_and_bugpr_closeout/110_wp7_oauth_failover.md new file mode 100644 index 00000000000..c5d25fb8882 --- /dev/null +++ b/devlog/_plan/260825_owner_backlog_and_bugpr_closeout/110_wp7_oauth_failover.md @@ -0,0 +1,64 @@ +# 110 — wp7: generic OAuth multi-account 429 failover (#2568) + +## Call-site enumeration (audit hazard H4) + +Every `hasKeyPoolFailover` reference on dev, and what it actually does: + +| File | Status | +|---|---| +| `src/providers/key-failover.ts:86` | the definition | +| `src/server/responses/core.ts:4908` | live 429 rotation loop (streaming) | +| `src/server/responses/core.ts:5252` | live 429 rotation (non-streaming) | +| `src/server/chat-native.ts:248` | live 429 rotation (native Chat) | +| `src/server/responses/compact.ts:86` | **import only** — no call | +| `src/server/responses/collaboration.ts:69` | **import only** — no call | +| `src/server/responses/encrypted-payload.ts:68` | **import only** — no call | + +So there are exactly THREE live key-pool rotation sites, not five. The three +import-only files are dead references; they are out of scope here but worth noting, since +the audit's concern was that a rotator could be generalized while leaving live paths +unfixed. + +OAuth rotation today exists at exactly two sites, both in `core.ts` +(`:4941` streaming, `:5288` non-streaming), and both are Anthropic-only and gated on +`isAnthropicAccountPoolEnabled`. + +## Design + +`anthropic-routing.ts` is already written against generic primitives. The provider-specific +parts are three: the constant `PROVIDER = "anthropic"`, the config gate, and token minting +via `getAnthropicPoolAccessToken`. The last one is NOT trivially generic — it enforces a +fail-closed rule about background `local-cli` credential slots that exists because of how +the Claude CLI stores its token. A generic rotator must not silently apply or drop that rule. + +Plan: parameterize the rotator by provider, keep the Anthropic credential rule attached to +Anthropic, and let each provider declare whether it participates. + +## Consent decision (recorded assumption, HIGH severity — needs owner review) + +The issue proposes presence-driven activation: 2+ accounts means rotate, mirroring how a +2-key API pool is treated as consent. `request_user_input` is denied under an active goal, +so I could not ask, and this is a product decision rather than a code one. + +**Assumption taken: ship it OPT-IN, not presence-driven.** Reasoning: + +- Rotating across subscription accounts spends a second account's quota. An API key pool + spends the operator's own metered credit; a subscription account is a different kind of + resource, and the Anthropic pool shipped opt-in for exactly this reason. +- Opt-in is reversible in one direction only that matters: turning it on later costs the + user nothing, whereas a default-on rotation that surprises someone has already spent the + quota. +- The audit flagged this specific question as needing explicit review rather than being + settled by an opt-out knob. + +If the owner prefers presence-driven, the change is a one-line default in +`oauthAccountFailoverEnabled` — the mechanism does not change. + +## Acceptance + +1. A provider with 2+ eligible accounts and the knob on rotates on 429 and replays once. +2. A single-account provider is a strict no-op. +3. The knob off is a strict no-op regardless of account count. +4. Existing Anthropic configuration keeps its current meaning. +5. The Codex pool is untouched. + diff --git a/devlog/_plan/260825_owner_backlog_and_bugpr_closeout/111_wp7_audit_response.md b/devlog/_plan/260825_owner_backlog_and_bugpr_closeout/111_wp7_audit_response.md new file mode 100644 index 00000000000..d4fc28f4c18 --- /dev/null +++ b/devlog/_plan/260825_owner_backlog_and_bugpr_closeout/111_wp7_audit_response.md @@ -0,0 +1,85 @@ +# 111 — wp7 audit response: the plan was wrong, and the scope is bigger than #2568 states + +Auditor verdict: **fail**, 7 blocking findings. I verified each against the tree. Six hold; +one I partially rebut. The honest conclusion is that `110` understated the problem badly +enough that it should not be implemented as written. + +## Verified against the tree + +**B1 — the call-site table was incomplete. ACCEPTED.** The three import-only files were +right, and the three direct callers were right, but there are two more LIVE rotation sites +reached through an injected `on429` hook rather than a direct call: +`src/images/loop.ts:574` and `src/web-search/loop.ts:511`, wired from +`core.ts:4267` and `core.ts:4355`. My `rg` for the symbol could not see them because the +call site passes a closure. That is exactly the failure mode hazard H4 warned about. + +**B2 — the standalone Antigravity image endpoint bypasses core entirely. ACCEPTED.** +`src/server/images.ts` resolves the active OAuth token, reads its project separately, and +returns upstream 429 without any rotation. + +**B3 — Cursor 429s never reach an HTTP-status rotator. ACCEPTED, and this is the finding +that breaks the plan.** Cursor converts transport failures into adapter EVENTS +(`src/adapters/cursor.ts:308`), and both the streaming and buffered paths have already +committed HTTP 200 by then (`core.ts:4556`, `:4619`). `cursor-errors.ts` classifies a bare +`resource_exhausted` tail into a 429 class at the ADAPTER layer, not as a response status. +So a `response.status === 429` loop — which is the entire mechanism #2568 proposes — cannot +serve Cursor at all. The issue names Cursor as an affected provider; the proposed design +cannot deliver it. + +**B4 — a token-only resolver is unsafe. ACCEPTED.** `getValidAccessTokenForAccount` returns +a string, but Copilot credentials carry an account-specific `apiBaseUrl` +(`src/oauth/github-copilot.ts:281`) and Antigravity needs an account-matched `projectId` +that `core.ts:2780` deliberately keeps paired with the token snapshot precisely so "an +account rotation cannot mix a fresh token with project metadata re-read from a different +credential generation". The existing code already anticipated this hazard; my plan would +have reintroduced it. + +**B5 — continuity/cache criteria absent. ACCEPTED.** Anthropic rebinds affinity on rotation +(`anthropic-routing.ts:490`); xAI pins conversation/session ids +(`providers/xai-transport.ts:105`); Cursor scopes checkpoints to credential identity +(`cursor/request-builder.ts:422`). A rotator that ignores these silently corrupts +continuation state rather than failing loudly. + +**B7 — the acceptance criteria permitted a partial ship. ACCEPTED.** "A provider" and "one +replay" would have been satisfied by xAI-Responses-only, while Anthropic already allows +three failovers. + +## Partial rebuttal + +**B6 — opt-in vs presence-driven.** The auditor is right that the issue asks for failover +"without first discovering and enabling a toggle", and right that default-off lets the +implementation pass while the reported xAI workflow stays broken. I accept that as the +issue's contract. + +Where I do not fully agree: the auditor treats this as a plan defect. It is a product +decision I was structurally unable to take — `request_user_input` is denied under an active +goal, and spending a second subscription account's quota by default is not a call an agent +should make silently. The correct resolution is not to pick a default under duress; it is to +escalate. Recorded as such below. + +## Consequence: wp7 does not proceed as a single work-phase + +The plan claimed a parameterization. The tree says otherwise: three distinct failure +surfaces (HTTP-status rotation, adapter-event classification, and a bypassing image +endpoint), three distinct credential shapes (bare token, token+apiBaseUrl, +token+projectId), and three distinct continuity contracts. That is a program, not a phase. + +wp7 is therefore split, and the first unit is the only one that can be built safely without +an owner decision: + +- **wp7a** — generic rotator for HTTP-status OAuth paths (`core.ts` streaming and + non-streaming), with a per-provider credential resolver that returns the FULL snapshot + (token plus whatever routing metadata that provider pairs with it), not a bare string. + Gated behind an explicit knob so no default changes. +- **wp7b** — adapter-event 429 rotation, which is what Cursor actually needs. +- **wp7c** — `src/server/images.ts` and the injected `on429` sidecar loops. +- **wp7d** — the activation-default decision. **ESCALATE: needs the owner.** + +## Escalation (NEEDS_HUMAN, recorded) + +The default-on question is not mine to settle. Presence-driven rotation spends another +subscription account's quota without the operator asking for it in that moment; opt-in +leaves the issue's stated workflow broken. Both readings are defensible and the issue text +supports the auditor's. This phase reports **NEEDS_HUMAN** on that specific decision, and +implements nothing that depends on it. + diff --git a/src/oauth/generic-account-failover.ts b/src/oauth/generic-account-failover.ts new file mode 100644 index 00000000000..d3fe698014f --- /dev/null +++ b/src/oauth/generic-account-failover.ts @@ -0,0 +1,160 @@ +/** + * Generic OAuth multi-account 429 failover (#2568). + * + * The API-key twin (`providers/key-failover.ts`) rotates by default for any key provider with a + * 2+ pool, but it returns false for `authMode === "oauth"`, and the only OAuth rotator that + * exists is Anthropic's — behind its own opt-in. So xAI, Cursor, Kimi, GitHub Copilot, + * Antigravity and Nous have no recovery path on a 429 even with several accounts logged in. + * + * Deliberately narrower than the Anthropic pool: no session affinity, no quota-ranked selection, + * no probe leases. Those carry provider-specific meaning; this module only answers "the account + * that just 429'd is cooled, is there another one we may use". + * + * NOT a home for Codex (`codex/routing.ts` owns quota scopes and probe leases) or Anthropic + * (`oauth/anthropic-routing.ts` owns affinity and a fail-closed local-cli credential rule). + * Both are excluded by `isGenericFailoverProvider`. + */ +import { getAccountSet } from "./store"; +import { getValidAccessSnapshotForAccount, type OAuthAccessSnapshot } from "./index"; +import { parseRetryAfterMs } from "../combos/failover"; +import { sweepExpiredOnWrite } from "../lib/state-store-sweeper"; +import type { OcxConfig, OcxProviderConfig } from "../types"; + +/** Cap same-request rotations so a short Retry-After cannot spin. Mirrors the Anthropic bound. */ +export const GENERIC_OAUTH_MAX_FAILOVERS_PER_REQUEST = 3; + +const DEFAULT_COOLDOWN_MS = 60_000; +const MAX_COOLDOWN_MS = 15 * 60_000; + +/** + * Providers whose rotation is owned elsewhere and must not be handled here. + * + * `openai` is the Codex pool: quota scopes, probe leases and affinity semantics that this + * module deliberately does not reimplement. `anthropic` has its own pool with a fail-closed + * rule about background local-cli credential slots. + */ +const EXCLUDED_PROVIDERS = new Set(["openai", "anthropic"]); + +interface AccountHealth { + cooldownUntil: number; + cooldownSource: "retry-after" | "default"; +} + +/** Process-local, like the Anthropic pool's: a restart is allowed to forget a cooldown. */ +const health = new Map(); + +const healthKey = (provider: string, accountId: string) => `${provider}\u0000${accountId}`; + +function isCooled(provider: string, accountId: string, now: number): boolean { + const entry = health.get(healthKey(provider, accountId)); + if (!entry) return false; + if (entry.cooldownUntil <= now) { + health.delete(healthKey(provider, accountId)); + return false; + } + return true; +} + +/** True when this provider participates in generic rotation at all. */ +export function isGenericFailoverProvider(providerName: string, provider: OcxProviderConfig): boolean { + return provider.authMode === "oauth" && !EXCLUDED_PROVIDERS.has(providerName); +} + +/** + * Whether generic rotation is active for this provider. + * + * Default OFF pending an owner decision on presence-driven activation (#2568 asks for no + * toggle; rotating spends another subscription account's quota, so the default is escalated + * rather than chosen here). The mechanism does not change if the default flips. + */ +export function isGenericOAuthFailoverEnabled(config: OcxConfig, providerName: string): boolean { + const provider = config.providers?.[providerName]; + if (!provider || !isGenericFailoverProvider(providerName, provider)) return false; + return config.oauthAccountFailover?.enabled === true; +} + +/** Accounts that may serve traffic right now: not cooled, not flagged for reauth. */ +export function eligibleFailoverAccounts(providerName: string, now = Date.now()): string[] { + const set = getAccountSet(providerName); + if (!set) return []; + return set.accounts + .filter(account => account.needsReauth !== true && !isCooled(providerName, account.id, now)) + .map(account => account.id); +} + +/** + * Cool the account that actually 429'd and name the next eligible one, or null. + * + * Returns the id only; the caller mints the credential so a failed refresh does not leave the + * cooldown applied to an account we then could not use. + */ +export function rotateGenericOAuthAccountOn429( + config: OcxConfig, + providerName: string, + failedAccountId: string, + retryAfterHeader: string | null | undefined, + now = Date.now(), +): string | null { + if (!isGenericOAuthFailoverEnabled(config, providerName)) return null; + const set = getAccountSet(providerName); + // A single stored account has nowhere to go; rotating to itself would just replay the 429. + if (!set || set.accounts.length < 2) return null; + + const parsed = parseRetryAfterMs(retryAfterHeader, now); + const cooldownMs = Math.min(parsed ?? DEFAULT_COOLDOWN_MS, MAX_COOLDOWN_MS); + health.set(healthKey(providerName, failedAccountId), { + cooldownUntil: now + cooldownMs, + cooldownSource: parsed ? "retry-after" : "default", + }); + sweepExpiredOnWrite(now); + + const eligible = eligibleFailoverAccounts(providerName, now).filter(id => id !== failedAccountId); + if (eligible.length === 0) return null; + // Deterministic: start after the failed account so repeated 429s walk the roster instead of + // hammering whichever id happens to sort first. + const order = set.accounts.map(account => account.id); + const start = order.indexOf(failedAccountId); + for (let i = 1; i <= order.length; i++) { + const candidate = order[(start + i) % order.length]!; + if (candidate !== failedAccountId && eligible.includes(candidate)) return candidate; + } + return null; +} + +/** + * Full credential snapshot for a rotated account. + * + * Returns the snapshot rather than a bare bearer: Antigravity pairs an account-matched + * `projectId` with its token and Kiro carries routing metadata, so a token-only swap would mix + * one account's bearer with another's routing data. + */ +export async function failoverAccountSnapshot( + providerName: string, + accountId: string, +): Promise { + return getValidAccessSnapshotForAccount(providerName, accountId); +} + +/** Earliest remaining cooldown, for a client-facing Retry-After when every account is cooled. */ +export function genericFailoverRetryAfterSeconds(providerName: string, now = Date.now()): number | null { + const set = getAccountSet(providerName); + if (!set) return null; + let earliest: number | null = null; + for (const account of set.accounts) { + const entry = health.get(healthKey(providerName, account.id)); + if (!entry || entry.cooldownUntil <= now) continue; + if (earliest === null || entry.cooldownUntil < earliest) earliest = entry.cooldownUntil; + } + return earliest === null ? null : Math.max(1, Math.ceil((earliest - now) / 1000)); +} + +/** Test seam and manual-recovery hook. */ +export function clearGenericFailoverHealth(providerName?: string): void { + if (!providerName) { + health.clear(); + return; + } + for (const key of [...health.keys()]) { + if (key.startsWith(`${providerName}\u0000`)) health.delete(key); + } +} diff --git a/src/oauth/index.ts b/src/oauth/index.ts index fe3abe76560..adf590ac9b2 100644 --- a/src/oauth/index.ts +++ b/src/oauth/index.ts @@ -490,6 +490,22 @@ export async function getValidAccessTokenForAccount(provider: string, accountId: return (await resolveAccessSnapshotForAccount(provider, accountId)).accessToken; } +/** + * Account-scoped resolver returning the FULL snapshot, not just the bearer. + * + * A rotator that swaps only the token silently mixes credential generations: Antigravity pairs + * an account-matched `projectId` with its token (see the pairing comment in + * server/responses/core.ts), Kiro carries routing metadata, and Copilot's observed snapshot + * carries an account-specific API origin. Reading those back from "whichever account is active" + * after a rotation is exactly the mixing this returns in one piece to prevent (#2568). + */ +export async function getValidAccessSnapshotForAccount( + provider: string, + accountId: string, +): Promise { + return resolveAccessSnapshotForAccount(provider, accountId); +} + /** Terminal refresh failures (revoked/rotated-away grants) — retrying cannot succeed. */ function isTerminalRefreshError(err: unknown): boolean { const msg = (err instanceof Error ? err.message : String(err)).toLowerCase(); diff --git a/src/routing/analytics.ts b/src/routing/analytics.ts index 9e7b5a0696f..6dc217e7345 100644 --- a/src/routing/analytics.ts +++ b/src/routing/analytics.ts @@ -119,6 +119,7 @@ const COOLDOWN_RECOVERY_KINDS = new Set([ "key-429", "oauth-401", "anthropic-oauth-429", + "oauth-account-429", ]); function percentile(sorted: number[], p: number): number | undefined { diff --git a/src/server/responses/core.ts b/src/server/responses/core.ts index cc970214629..ce4f5eb9057 100644 --- a/src/server/responses/core.ts +++ b/src/server/responses/core.ts @@ -113,6 +113,13 @@ import { resolveAnthropicAccountForSession, rotateAnthropicAccountOn429, } from "../../oauth/anthropic-routing"; +import { + failoverAccountSnapshot, + GENERIC_OAUTH_MAX_FAILOVERS_PER_REQUEST, + isGenericFailoverProvider, + isGenericOAuthFailoverEnabled, + rotateGenericOAuthAccountOn429, +} from "../../oauth/generic-account-failover"; import { buildWebSearchTool, planWebSearch, runWithWebSearch, shouldResolveOpenAiWebSearchSidecar } from "../../web-search"; import { buildImageTool, buildVideoTool, planImageBridge, planVideoBridge, runWithImageBridge, clampImageMaxRounds, IMAGE_GEN_TOOL_NAME, VIDEO_GEN_TOOL_NAME } from "../../images"; import { describeImagesInPlace, isModelTextOnly, planVisionSidecar, resolveOpenAiVisionModel, shouldResolveOpenAiVisionSidecar, stripImagesInPlace } from "../../vision"; @@ -2733,6 +2740,10 @@ async function handleResponsesInner( let replayOAuthCredentialSnapshot: Pick | undefined; let anthropicPoolAccountId: string | null = null; let anthropicPoolFailovers = 0; + // Generic OAuth rotation (#2568) for providers with no pool of their own. Bound to the account + // the request actually used, so a concurrent rotation cannot cool an innocent replacement. + let genericFailoverAccountId: string | null = null; + let genericFailovers = 0; const anthropicSessionKey = route.providerName === "anthropic" && route.provider.authMode === "oauth" ? anthropicSessionKeyFromParts({ sessionIdHeader: sessionIdHeaderFromRequest(req.headers), @@ -2772,6 +2783,11 @@ async function handleResponsesInner( }; if (isOAuth401ReplayProvider) sentOAuthSnapshot = resolved; route.provider = { ...route.provider, apiKey: resolved.accessToken }; + // Remember which account actually served this request so a 429 cools THAT one, not + // whichever account is active by the time the response comes back (#2568). + if (isGenericFailoverProvider(route.providerName, route.provider)) { + genericFailoverAccountId = resolved.accountId; + } if (route.providerName === "kiro") { // `{}` is intentional: this is an account-scoped request with no stored routing metadata. // Only genuinely accountless adapter calls leave the context undefined and use local/env fallback. @@ -4969,6 +4985,49 @@ async function handleResponsesInner( break; } } + // Generic OAuth account failover (#2568) for providers with no pool of their own. Opt-in + // and a strict no-op otherwise, so a single-account install and every existing config are + // unchanged. Codex and Anthropic are excluded by isGenericFailoverProvider — their pools + // own quota scopes, probe leases and affinity that this must not reimplement. + while ( + upstreamResponse.status === 429 + && genericFailoverAccountId + && genericFailovers < GENERIC_OAUTH_MAX_FAILOVERS_PER_REQUEST + && isGenericOAuthFailoverEnabled(config, route.providerName) + ) { + const nextAccountId = rotateGenericOAuthAccountOn429( + config, + route.providerName, + genericFailoverAccountId, + upstreamResponse.headers.get("retry-after"), + ); + if (!nextAccountId) break; + try { void upstreamResponse.body?.cancel().catch(() => {}); } catch { /* already consumed/closed */ } + try { + // The FULL snapshot, not just the bearer: Antigravity pairs an account-matched + // projectId with its token and Kiro carries routing metadata, so a token-only swap + // would mix one account's credential with another's routing data. + const snapshot = await failoverAccountSnapshot(route.providerName, nextAccountId); + genericFailoverAccountId = nextAccountId; + genericFailovers += 1; + route.provider = { ...route.provider, apiKey: snapshot.accessToken }; + if (route.providerName === "kiro") parsed._kiroAuthContext = { ...(snapshot.kiro ?? {}) }; + if (route.provider.googleMode === "cloud-code-assist" && snapshot.projectId) { + route.provider = { ...route.provider, project: snapshot.projectId }; + } + invalidateSameTargetRequest(); + activeAdapter = resolveAdapter( + resolveWireProtocolOverride(route.providerName, route.modelId, route.provider, inboundWire), + config.cacheRetention, + ); + sealRequestAttemptIdentity(logCtx.activeAttempt, logCtx.provider, activeAdapter.name, logCtx.accountLogLabel); + const result = await rebuildAndRefetch("oauth-account-429"); + if ("failed" in result) return result.failed; + upstreamResponse = result; + } catch { + break; + } + } // Unknown provenance is deliberately fail-soft in pre-flight: after a restart, TTL expiry, // or LRU eviction, a valid same-backend blob must survive. A decoder's own 4xx identity is // the missing authoritative signal. Rebuild once through the same sanitation path used by a diff --git a/src/types/config.ts b/src/types/config.ts index 6dd7f4e86e6..6ba30703983 100644 --- a/src/types/config.ts +++ b/src/types/config.ts @@ -583,6 +583,21 @@ export interface OcxConfig { /** Successful new-session binds retained on one round-robin selection. Default 1; range 1..100. */ stickyLimit?: number; }; + /** + * Opt-in generic OAuth multi-account 429 failover (#2568). Default OFF. + * + * Rotates to another logged-in account of the SAME provider when one is rate-limited, for + * OAuth providers that have no pool of their own — xAI, Cursor, Kimi, GitHub Copilot, + * Antigravity, Nous. The Codex pool and the Anthropic pool own their own rotation and are + * excluded; enabling this changes neither. + * + * Default OFF is a recorded escalation, not a settled preference: the issue asks for + * presence-driven activation (2+ accounts implies consent, mirroring API-key pools), but + * rotating spends a second subscription account's quota, so the default is left to the owner. + */ + oauthAccountFailover?: { + enabled?: boolean; + }; /** Virtual `combo/` models spanning concrete provider/model targets (issue #133). */ combos?: Record; /** diff --git a/src/usage/log.ts b/src/usage/log.ts index 7bca6509645..65ebf26d6a8 100644 --- a/src/usage/log.ts +++ b/src/usage/log.ts @@ -28,6 +28,7 @@ export type AttemptRecoveryKind = | "key-429" | "rate-limit-429" | "anthropic-oauth-429" + | "oauth-account-429" | "image-413" | "opaque-blob-rejection" | "empty-completion"; @@ -218,6 +219,7 @@ const ATTEMPT_RECOVERY_KINDS = new Set([ "key-429", "rate-limit-429", "anthropic-oauth-429", + "oauth-account-429", "image-413", "opaque-blob-rejection", "empty-completion", diff --git a/tests/generic-oauth-failover.test.ts b/tests/generic-oauth-failover.test.ts new file mode 100644 index 00000000000..77e18df28ef --- /dev/null +++ b/tests/generic-oauth-failover.test.ts @@ -0,0 +1,115 @@ +import { afterEach, beforeEach, describe, expect, test } from "bun:test"; +import { mkdtempSync, rmSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { + clearGenericFailoverHealth, + eligibleFailoverAccounts, + genericFailoverRetryAfterSeconds, + isGenericFailoverProvider, + isGenericOAuthFailoverEnabled, + rotateGenericOAuthAccountOn429, +} from "../src/oauth/generic-account-failover"; +import { getAccountSet, saveCredential } from "../src/oauth/store"; +import type { OcxConfig, OcxProviderConfig } from "../src/types"; + +const originalHome = process.env.OPENCODEX_HOME; +let home: string; + +beforeEach(() => { + home = mkdtempSync(join(tmpdir(), "ocx-generic-failover-")); + process.env.OPENCODEX_HOME = home; + clearGenericFailoverHealth(); +}); + +afterEach(() => { + clearGenericFailoverHealth(); + if (originalHome === undefined) delete process.env.OPENCODEX_HOME; + else process.env.OPENCODEX_HOME = originalHome; + rmSync(home, { recursive: true, force: true }); +}); + +const OAUTH_PROVIDER = { + adapter: "openai-chat", + baseUrl: "https://api.x.ai/v1", + authMode: "oauth", +} as unknown as OcxProviderConfig; + +function config(enabled: boolean): OcxConfig { + return { + providers: { xai: OAUTH_PROVIDER }, + ...(enabled ? { oauthAccountFailover: { enabled: true } } : {}), + } as unknown as OcxConfig; +} + +async function seed(count: number): Promise { + for (let i = 0; i < count; i++) { + await saveCredential("xai", { + access: `access-${i}`, + refresh: `refresh-${i}`, + expires: Date.now() + 3_600_000, + accountId: `uuid-${i}`, + } as never, { addAccount: true }); + } + return getAccountSet("xai")?.accounts.map(a => a.id) ?? []; +} + +describe("#2568 generic OAuth account failover", () => { + test("rotates to another logged-in account and cools the one that 429'd", async () => { + const [first, second] = await seed(2); + const next = rotateGenericOAuthAccountOn429(config(true), "xai", first!, null); + expect(next).toBe(second); + // The failed account is cooled, so it is not offered again while the window holds. + expect(eligibleFailoverAccounts("xai")).toEqual([second!]); + }); + + test("a single stored account is a strict no-op", async () => { + // Rotating to itself would replay the same 429 against the same credential, and cooling + // the only account would take the provider out of service for nothing. + const [solo] = await seed(1); + expect(rotateGenericOAuthAccountOn429(config(true), "xai", solo!, null)).toBeNull(); + expect(eligibleFailoverAccounts("xai")).toEqual([solo!]); + }); + + test("the knob off is a strict no-op regardless of account count", async () => { + const ids = await seed(2); + expect(rotateGenericOAuthAccountOn429(config(false), "xai", ids[0]!, null)).toBeNull(); + expect(eligibleFailoverAccounts("xai")).toEqual(ids); + }); + + test("Codex and Anthropic are excluded: their own pools own rotation", () => { + expect(isGenericFailoverProvider("xai", OAUTH_PROVIDER)).toBe(true); + expect(isGenericFailoverProvider("openai", OAUTH_PROVIDER)).toBe(false); + expect(isGenericFailoverProvider("anthropic", OAUTH_PROVIDER)).toBe(false); + }); + + test("a key-auth provider never enters generic OAuth rotation", () => { + const key = { ...OAUTH_PROVIDER, authMode: "key" } as OcxProviderConfig; + expect(isGenericFailoverProvider("groq", key)).toBe(false); + }); + + test("all accounts cooled reports the earliest remaining window", async () => { + const ids = await seed(2); + const cfg = config(true); + expect(rotateGenericOAuthAccountOn429(cfg, "xai", ids[0]!, "120")).toBe(ids[1]); + expect(rotateGenericOAuthAccountOn429(cfg, "xai", ids[1]!, "30")).toBeNull(); + const retryAfter = genericFailoverRetryAfterSeconds("xai"); + // The earliest window wins: a client must not be told to wait for the longest cooldown. + expect(retryAfter).toBeGreaterThan(0); + expect(retryAfter!).toBeLessThanOrEqual(30); + }); + + test("Retry-After drives the cooldown length", async () => { + const ids = await seed(2); + rotateGenericOAuthAccountOn429(config(true), "xai", ids[0]!, "600"); + expect(genericFailoverRetryAfterSeconds("xai")).toBeGreaterThan(500); + }); + + test("enablement requires both the knob and a participating OAuth provider", async () => { + await seed(2); + expect(isGenericOAuthFailoverEnabled(config(true), "xai")).toBe(true); + expect(isGenericOAuthFailoverEnabled(config(false), "xai")).toBe(false); + expect(isGenericOAuthFailoverEnabled(config(true), "openai")).toBe(false); + }); +}); +