diff --git a/src/oauth/generic-account-failover.ts b/src/oauth/generic-account-failover.ts index c8cbd8e042..9fca39601c 100644 --- a/src/oauth/generic-account-failover.ts +++ b/src/oauth/generic-account-failover.ts @@ -483,7 +483,8 @@ export function noteGenericPoolSelection( * 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. + * cooldown applied to an account we then could not use. `commit: false` still cools the failed + * account but only peeks the round-robin ring, for callers that will not dispatch the answer. */ export function rotateGenericOAuthAccountOn429( config: OcxConfig, @@ -492,9 +493,10 @@ export function rotateGenericOAuthAccountOn429( retryAfterHeader: string | null | undefined, now = Date.now(), requestedModelId?: string | null, + commit = true, ): string | null { return rotateGenericOAuthAccountOnRefusal(config, providerName, failedAccountId, "rate", - retryAfterHeader, now, requestedModelId); + retryAfterHeader, now, requestedModelId, undefined, commit); } /** Kiro adds account-scoped refusal classes; all other providers retain their 429 policy. */ @@ -507,6 +509,7 @@ export function rotateGenericOAuthAccountOnRefusal( now = Date.now(), requestedModelId?: string | null, monthlyCooldownMs?: number, + commit = true, ): string | null { if (kind === "other" || (providerName !== "kiro" && kind !== "rate")) return null; if (!isGenericOAuthFailoverEnabled(config, providerName)) return null; @@ -566,8 +569,8 @@ export function rotateGenericOAuthAccountOnRefusal( } if (strategy === "round-robin") { // PICK here, not peek: the failure already happened and this answer is the one being used, - // so the ring genuinely advances. - return pickRoundRobinAccount( + // so the ring genuinely advances. A caller that only asks whether a sibling exists peeks. + return (commit ? pickRoundRobinAccount : peekRoundRobinAccount)( genericPoolKey(providerName), candidates, genericStickyLimit(config, providerName), diff --git a/src/server/responses/core-combo.ts b/src/server/responses/core-combo.ts index 3104433d4c..ce76e36637 100644 --- a/src/server/responses/core-combo.ts +++ b/src/server/responses/core-combo.ts @@ -144,7 +144,8 @@ function cooledPoolAccountLabel(config: OcxConfig, providerName: string, modelId * through `accountLogLabel`, but the per-request account budget can end the pool's own rotation * before the last refused account is recorded. Hand that account to the pool's rotator, which * cools it under the pool's own Retry-After policy, and spare the target only when the pool still - * has an eligible sibling. A lone account or an all-cooled pool returns null and cools the target. + * has an eligible sibling. The rotator only peeks round-robin here: nothing is sent to that sibling. + * A lone account or an all-cooled pool returns null and cools the target. */ function cooledGenericOAuthAccountLabel( config: OcxConfig, providerName: string, modelId: string, label: string | undefined, retryAfter: string | undefined, @@ -155,7 +156,7 @@ function cooledGenericOAuthAccountLabel( const matches = getAccountSet(providerName)?.accounts.filter(account => oauthAccountLogLabel(account.id, base) === label) ?? []; return matches.length === 1 - && rotateGenericOAuthAccountOn429(config, providerName, matches[0]!.id, retryAfter, Date.now(), modelId) + && rotateGenericOAuthAccountOn429(config, providerName, matches[0]!.id, retryAfter, Date.now(), modelId, false) ? label : undefined; } diff --git a/tests/oauth/generic-oauth-combo-account-cooldown.test.ts b/tests/oauth/generic-oauth-combo-account-cooldown.test.ts index 0ec326ed74..b8ec163c75 100644 --- a/tests/oauth/generic-oauth-combo-account-cooldown.test.ts +++ b/tests/oauth/generic-oauth-combo-account-cooldown.test.ts @@ -12,6 +12,7 @@ import { tmpdir } from "node:os"; import { join } from "node:path"; import { clearComboSelectionState, clearComboTargetCooldowns, isComboTargetInCooldown } from "../../src/combos"; import { clearGenericFailoverHealth, eligibleFailoverAccounts } from "../../src/oauth/generic-account-failover"; +import { genericPoolKey, peekRoundRobinAccount } from "../../src/oauth/pool-kernel"; import { getAccountSet, saveCredential, setActiveAccount } from "../../src/oauth/store"; import { clearAccountQuotaCache } from "../../src/providers/quota"; import { clearResponseStateForTests, flushResponseState } from "../../src/responses/state"; @@ -129,3 +130,20 @@ test("every generic OAuth account spent still cools the combo target", async () expect(sends).toEqual(["1", "2", "B"]); expect(isComboTargetInCooldown("pooled", TARGET)).toBe(true); }, 20_000); + +test("the combo's sibling check does not advance a round-robin pool onto an unsent account", async () => { + await seed(5); + upstream(4); + const roundRobin: OcxConfig = { ...config, pool: { kernel: true }, providers: { ...config.providers, + xai: { ...config.providers.xai!, oauthAccountFailover: { strategy: "round-robin", stickyLimit: 3 } } } }; + const ids = getAccountSet("xai")!.accounts.map(account => account.id); + const response = await handleResponses(new Request("http://localhost/v1/responses", { + method: "POST", headers: { "content-type": "application/json" }, + body: JSON.stringify({ model: "combo/pooled", input: "Answer briefly", stream: false }), + }), roundRobin, { model: "", provider: "" }); + expect(await response.text()).toContain("fallback answer"); + expect(sends).not.toContain("5"); + expect(isComboTargetInCooldown("pooled", TARGET)).toBe(false); + // Account 5 was only probed for eligibility, never sent; the ring must not stick to it. + expect(peekRoundRobinAccount(genericPoolKey("xai"), ids, 3)).not.toBe(ids[4]); +}, 20_000);