fix(sse): combo model lockout honors parsed upstream quota reset (#6863) (#6866)

* 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 <JUDYZHU459@outlook.com>
Co-authored-by: Diego Rodrigues de Sa e Souza <8016841+diegosouzapw@users.noreply.github.com>
This commit is contained in:
AgentKiller45
2026-07-11 21:58:58 -07:00
committed by GitHub
parent 6530c92aa6
commit 1f9ac3628b
3 changed files with 160 additions and 7 deletions

View File

@@ -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");

View File

@@ -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",

View File

@@ -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)`
);
});