From 1f9ac3628ba087bd243aa41cecc756017e6e9c7f Mon Sep 17 00:00:00 2001 From: AgentKiller45 Date: Sat, 11 Jul 2026 21:58:58 -0700 Subject: [PATCH] fix(sse): combo model lockout honors parsed upstream quota reset (#6863) (#6866) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix(sse): combo model lockout honors parsed upstream quota reset (#6863) The combo failure path recorded model lockouts from checkFallbackError's cooldownMs only, discarding quotaResetHintMs — the ungated channel that carries a parsed upstream quota reset (e.g. Antigravity 429 "Resets in 92h27m28s"). With OAuth profiles defaulting useUpstreamRetryHints=false, the lockout fell back to the base cooldown (seconds), so quota-dead accounts were re-walked serially by every combo request for days (measured 122s per request, 494s worst case in #6863). Thread max(cooldownMs, quotaResetHintMs) into selectLockoutCooldownMs at both combo lockout sites, mirroring the single-model path pattern in src/sse/services/auth.ts (v3.8.43). All three resilience fences are preserved: useUpstreamRetryHints still gates connection cooldowns, the hint only affects model-scope lockouts, and combo 429s remain non-persistent. TDD: tests/unit/combo-lockout-quota-reset-6863.test.ts fails on base (lockout 5000ms) and passes with the fix (~92.5h). * test(sse): tighten #6863 lockout assertion to parsed-reset bounds; prettier pass Assert remainingMs falls within (parsedResetMs - 5s, parsedResetMs] so a hardcoded long cooldown cannot satisfy the regression test. Also apply Prettier to both changed files (includes one pre-existing formatting fix in handleRoundRobinCombo picked up by --write). * fix(sse): align combo lockout hint selection with single-model path; register test in mutation gate Adopt review feedback: replace Math.max(cooldownMs, quotaResetHintMs) with the auth.ts pattern (usedUpstreamRetryHint ? cooldownMs : quotaResetHintMs) so a parsed reset SHORTER than the fallback cooldown wins too — e.g. the subscription-quota branch returns a 1h fallback while the body says "resets in 45m"; max() would over-lock by 15 minutes. Add a regression test for the short-reset case (fails against the max() variant) and register the new test file in stryker.conf.json tap.testFiles to satisfy check:mutation-test-coverage --strict. * chore(quality): register cliproxyapi dispatch test in mutation gate tests/unit/cliproxyapi-model-mapping-dispatch.test.ts landed on release/v3.8.47 via #6903 without a tap.testFiles entry, so check:mutation-test-coverage --strict fails on the branch tip and on every PR merge ref. Register it so the gate is green again. --------- Co-authored-by: judy459 Co-authored-by: Diego Rodrigues de Sa e Souza <8016841+diegosouzapw@users.noreply.github.com> --- open-sse/services/combo.ts | 25 +++- stryker.conf.json | 2 + .../combo-lockout-quota-reset-6863.test.ts | 140 ++++++++++++++++++ 3 files changed, 160 insertions(+), 7 deletions(-) create mode 100644 tests/unit/combo-lockout-quota-reset-6863.test.ts diff --git a/open-sse/services/combo.ts b/open-sse/services/combo.ts index 69cec49793..7e4c719093 100644 --- a/open-sse/services/combo.ts +++ b/open-sse/services/combo.ts @@ -2036,6 +2036,19 @@ export async function handleComboChat({ structuredError ); const { cooldownMs } = fallbackResult; + // #6863: a parsed upstream quota reset (e.g. Antigravity "Resets in 92h27m28s") + // arrives in `quotaResetHintMs` — it bypasses the operator-gated + // `useUpstreamRetryHints` connection-cooldown setting. Mirror the + // single-model path (src/sse/services/auth.ts): when the retry hint was + // already honored, `cooldownMs` IS the upstream value; otherwise prefer the + // parsed quota reset — even when it is SHORTER than the fallback cooldown + // (e.g. subscription-quota 1h default vs a real "resets in 10m"). + // `selectLockoutCooldownMs` still ignores hints at/below the base cooldown, + // so absent/tiny hints keep the #1308 exponential-backoff behavior. + const lockoutHintMs = + fallbackResult.usedUpstreamRetryHint === true + ? cooldownMs + : (fallbackResult.quotaResetHintMs ?? 0); const selectedConnectionId = result.headers?.get("X-OmniRoute-Selected-Connection-Id") || result.headers?.get("x-omniroute-selected-connection-id") || @@ -2161,9 +2174,9 @@ export async function handleComboChat({ mlSettings.baseCooldownMs, profile, { - // #1308: honor a long upstream reset (e.g. "Resets in 160h") over + // #1308/#6863: honor a long upstream reset (e.g. "Resets in 160h") over // the short base cooldown / exponential backoff when present. - exactCooldownMs: selectLockoutCooldownMs(cooldownMs, mlSettings), + exactCooldownMs: selectLockoutCooldownMs(lockoutHintMs, mlSettings), maxCooldownMs: mlSettings.maxCooldownMs, } ); @@ -2204,8 +2217,8 @@ export async function handleComboChat({ mlSettings.baseCooldownMs, profile, { - // #1308: honor a long upstream reset over base/exponential cooldown. - exactCooldownMs: selectLockoutCooldownMs(cooldownMs, mlSettings), + // #1308/#6863: honor a long upstream reset over base/exponential cooldown. + exactCooldownMs: selectLockoutCooldownMs(lockoutHintMs, mlSettings), maxCooldownMs: mlSettings.maxCooldownMs, } ); @@ -2505,9 +2518,7 @@ async function handleRoundRobinCombo({ // runtime-unavailable, we must reconsider these before returning 503, instead of // permanently dropping a compat-rejected-but-healthy provider. const compatKeptSet = new Set(filteredTargets); - const compatRejectedTargets = evalRankedTargets.filter( - (target) => !compatKeptSet.has(target) - ); + const compatRejectedTargets = evalRankedTargets.filter((target) => !compatKeptSet.has(target)); const modelCount = filteredTargets.length; if (modelCount === 0) { return comboModelNotFoundResponse("Round-robin combo has no executable targets"); diff --git a/stryker.conf.json b/stryker.conf.json index 0c03de6500..13142050be 100644 --- a/stryker.conf.json +++ b/stryker.conf.json @@ -107,6 +107,7 @@ "tests/unit/claude-passthrough-thinking-2454.test.ts", "tests/unit/cli-simulate.test.ts", "tests/unit/cline-response-envelope.test.ts", + "tests/unit/cliproxyapi-model-mapping-dispatch.test.ts", "tests/unit/clinepass-provider.test.ts", "tests/unit/cliproxyapi-model-mapping-dispatch.test.ts", "tests/unit/codex-failover.test.ts", @@ -125,6 +126,7 @@ "tests/unit/combo-health-dashboard.test.ts", "tests/unit/combo-health-route.test.ts", "tests/unit/combo-hedging.test.ts", + "tests/unit/combo-lockout-quota-reset-6863.test.ts", "tests/unit/combo-max-depth-config.test.ts", "tests/unit/combo-model-lockout-honors-reset-1308.test.ts", "tests/unit/combo-omnimodel-tag-stripping.test.ts", diff --git a/tests/unit/combo-lockout-quota-reset-6863.test.ts b/tests/unit/combo-lockout-quota-reset-6863.test.ts new file mode 100644 index 0000000000..84eb7ed70a --- /dev/null +++ b/tests/unit/combo-lockout-quota-reset-6863.test.ts @@ -0,0 +1,140 @@ +// #6863: combo path model lockout must honor a parsed upstream quota reset +// ("Resets in 92h27m28s") instead of the base cooldown ladder, mirroring the +// single-model path (src/sse/services/auth.ts usedUpstreamRetryHint/quotaResetHintMs). +import test from "node:test"; +import assert from "node:assert/strict"; +import fs from "node:fs"; +import os from "node:os"; +import path from "node:path"; + +const TEST_DATA_DIR = fs.mkdtempSync(path.join(os.tmpdir(), "omr-combo-quota-reset-6863-")); +process.env.DATA_DIR = TEST_DATA_DIR; +process.env.API_KEY_SECRET = "test-combo-quota-reset-6863"; + +const core = await import("../../src/lib/db/core.ts"); +const { handleComboChat } = await import("../../open-sse/services/combo.ts"); +const { getModelLockoutInfo, clearAllModelLockouts, parseRetryFromErrorText } = + await import("../../open-sse/services/accountFallback.ts"); + +const UPSTREAM_429_MESSAGE = + "429: Individual quota reached. Please upgrade your subscription to increase your limits. Resets in 92h27m28s."; + +function createLog() { + return { info: () => {}, warn: () => {}, error: () => {}, debug: () => {} }; +} + +test.beforeEach(() => { + clearAllModelLockouts(); +}); + +test.after(() => { + clearAllModelLockouts(); + try { + core.resetDbInstance(); + fs.rmSync(TEST_DATA_DIR, { recursive: true, force: true }); + } catch {} +}); + +test("combo 429 lockout honors parsed upstream quota reset over base cooldown (#6863)", async () => { + const provider = "antigravity"; // OAuth category → quota signals preserved on 429 + const model = "claude-sonnet-4.6"; + + const settings = { + modelLockout: { + enabled: true, + errorCodes: [429], + baseCooldownMs: 3000, + maxCooldownMs: 1_800_000, + maxBackoffSteps: 10, + useExponentialBackoff: true, + }, + }; + + await handleComboChat({ + body: {}, + combo: { + name: "quota-reset-combo", + strategy: "priority", + models: [`${provider}/${model}`], + config: { maxRetries: 0, retryDelayMs: 0, fallbackDelayMs: 0 }, + }, + handleSingleModel: async () => + new Response(JSON.stringify({ error: { message: UPSTREAM_429_MESSAGE } }), { + status: 429, + headers: { "content-type": "application/json" }, + }), + isModelAvailable: async () => true, + log: createLog(), + settings, + allCombos: null, + }); + + const parsedResetMs = parseRetryFromErrorText(UPSTREAM_429_MESSAGE); + assert.ok( + parsedResetMs && parsedResetMs > 90 * 3600 * 1000, + `sanity: reset text must parse to ~92.5h, got ${parsedResetMs}` + ); + + const info = getModelLockoutInfo(provider, "", model); + assert.ok(info, "combo 429 must record a model lockout"); + // Bug #6863: lockout was baseCooldownMs (~seconds) while upstream said 92.5h. + // The lockout must equal the parsed reset minus elapsed test runtime (bounded slack), + // so a hardcoded long cooldown (e.g. a fixed 1h) cannot pass. + assert.ok( + info!.remainingMs > parsedResetMs! - 5_000 && info!.remainingMs <= parsedResetMs!, + `lockout must equal the parsed upstream reset (~${parsedResetMs}ms); got ${info!.remainingMs}ms (~${Math.round(info!.remainingMs / 1000)}s)` + ); +}); + +test("combo 429 lockout prefers a SHORT parsed reset over the subscription fallback cooldown", async () => { + // Review follow-up on #6863: the subscription-quota branch returns + // cooldownMs = 1h fallback when useUpstreamRetryHints is off (OAuth default), + // while quotaResetHintMs carries the real parsed reset. A max() of the two + // would over-lock (1h) — the lockout must follow the parsed value (~45m), + // matching the single-model path in src/sse/services/auth.ts. + const provider = "claude"; // OAuth category → subscription-quota branch applies + const model = "claude-sonnet-4-6"; + const shortResetMessage = + "429: Usage limit reached. Your Claude Pro usage limit resets in 45m0s."; + + const settings = { + modelLockout: { + enabled: true, + errorCodes: [429], + baseCooldownMs: 3000, + maxCooldownMs: 7_200_000, + maxBackoffSteps: 10, + useExponentialBackoff: true, + }, + }; + + await handleComboChat({ + body: {}, + combo: { + name: "short-reset-combo", + strategy: "priority", + models: [`${provider}/${model}`], + config: { maxRetries: 0, retryDelayMs: 0, fallbackDelayMs: 0 }, + }, + handleSingleModel: async () => + new Response(JSON.stringify({ error: { message: shortResetMessage } }), { + status: 429, + headers: { "content-type": "application/json" }, + }), + isModelAvailable: async () => true, + log: createLog(), + settings, + allCombos: null, + }); + + const parsedResetMs = parseRetryFromErrorText(shortResetMessage); + assert.equal(parsedResetMs, 45 * 60 * 1000, "sanity: reset text must parse to 45m"); + + const info = getModelLockoutInfo(provider, "", model); + assert.ok(info, "combo 429 must record a model lockout"); + // Must be the parsed 45m — NOT the 1h subscription fallback (over-lock). + assert.ok( + info!.remainingMs > parsedResetMs! - 5_000 && info!.remainingMs <= parsedResetMs!, + `lockout must follow the parsed 45m reset, not the 1h fallback; got ${info!.remainingMs}ms (~${Math.round(info!.remainingMs / 1000)}s)` + ); +});