fix(resilience): honor comboCooldownWait for every combo strategy (#8541) (#8559)

* fix(resilience): honor comboCooldownWait for every combo strategy

The cooldown-wait path claimed all strategies, but isComboCooldownWaitEligible still gated on quota-share/auto. Widen the gate, raise the per-target timeout floor with it, and consult the allow-list from earliestRetryAfter so a later 403 cannot skip the decision. Fixes #8541.

* test(combo): fold cooldown-wait strategy assertions into a loop

Keep combo-config.test.ts under the file-size ceiling while covering every
routing strategy (including internal quota-share) for the #8541 gate fix.

* chore(combo): trim cooldown-wait comment under file-size ceiling

After rebase onto tip the net +2 in combo.ts sat one line over the frozen
check:file-size count (split-based); fold the SECURITY note into the block.
This commit is contained in:
AmirHossein Rezaei
2026-07-26 10:21:38 +03:30
committed by GitHub
parent f60c9fe542
commit a51cd06322
11 changed files with 131 additions and 139 deletions

View File

@@ -209,14 +209,13 @@ on). Without a `max_concurrent` cap the behavior is unchanged.
### Combo cooldown-aware retry
For quota-share and `auto` combos, a request that would crystallize a 429 for a
SHORT transient cooldown waits it out and re-dispatches instead of returning
the 429 — this covers Gemini-class TPM/RPM windows (~60s retry-after) on a
multi-model `auto` combo, e.g. both targets of a 2-model combo hitting a
per-model rate limit. Bounded by `comboCooldownWait` (`enabled`, `maxWaitMs`
65s, `maxAttempts` 2, `budgetMs` 130s, hard ceiling 90s) in **Settings →
Resilience**. It never waits on `quota_exhausted` (locked until midnight) or
auth/not-found reasons.
For every combo strategy (when enabled), a request that would crystallize a 429
for a SHORT transient cooldown waits it out and re-dispatches instead of
returning the 429 — this covers Gemini-class TPM/RPM windows (~60s retry-after)
on multi-model combos, e.g. both targets of a 2-model combo hitting a per-model
rate limit. Bounded by `comboCooldownWait` (`enabled`, `maxWaitMs`, `maxAttempts`,
`budgetMs`) in **Settings → Resilience**. It never waits on `quota_exhausted`
(locked until midnight) or auth/not-found reasons.
---

View File

@@ -2709,63 +2709,63 @@ export async function handleComboChat({
: "";
const msg = (lastError || "All combo models unavailable") + comboErrorSummary;
if (earliestRetryAfter && isRetryAfterEligibleStatus(status)) {
// Cooldown-aware retry: instead of crystallizing the 429/503, wait out
// a SHORT transient cooldown and re-run the whole set loop. Guarded by
// the helper (quota_exhausted/auth/not-found excluded, ceiling,
// attempts, budget). MAX_GLOBAL_ATTEMPTS still bounds total dispatches.
// Available to ALL combo strategies (not just quota-share).
if (comboCooldownWaitEnabled && status === 429) {
// ONE decision path for EVERY strategy. The reason that drives the
// wait is always the target's REAL model-lockout reason, resolved
// through the helper's allow-list — never a hardcoded literal.
//
// SECURITY (see comboCooldownRetry.ts header): the allow-list is the
// PRIMARY barrier and `maxWaitMs` only the SECOND one. Hardcoding
// reason:"rate_limit" for non-quota-share strategies would drop the
// primary barrier and leave only the ceiling — which does NOT cover a
// quota_exhausted lock carrying a SHORT upstream retry-after (e.g.
// 3s < maxWaitMs): the combo would wait, redispatch against a model
// locked until midnight, and burn the attempt. Model lockouts are
// recorded for all strategies (recordModelLockoutFailure above is not
// gated on quota-share), so the real reason is always available.
const decision: ResolveComboCooldownDecisionResult = resolveComboCooldownWaitDecision({
targets: orderedTargets,
earliestRetryAfter,
attempt: comboCooldownAttempt,
budgetLeftMs: comboCooldownBudgetLeftMs,
settings: resilienceSettings.comboCooldownWait,
// Key each lookup on the TARGET's own model: quota-share combos are
// single-model/multi-account (so this is identical to the previous
// orderedTargets[0] behavior), but heterogeneous combos carry a
// different model per target.
lookupLock: (provider, connectionId, target) => {
const rawModel = parseModel(target?.modelStr ?? "").model || "";
if (!rawModel) return null;
return getModelLockoutInfo(provider, connectionId, rawModel);
},
computeWaitMs: (retryAfter) => computeClosestRetryAfter(retryAfter).waitMs,
});
// Cooldown-aware retry: instead of crystallizing a transient failure, wait
// out a SHORT cooldown and re-run the whole set loop. Guarded by the helper
// (quota_exhausted/auth/not-found excluded, ceiling, attempts, budget).
// MAX_GLOBAL_ATTEMPTS still bounds total dispatches. Available to ALL combo
// strategies when enabled — entry is driven by earliestRetryAfter + the
// real model-lockout reason, NOT by whichever target last overwrote
// `status` (a later 403 must not skip the allow-list check for an earlier
// 429's retry-after hint). SECURITY (see comboCooldownRetry.ts header): the
// allow-list is the PRIMARY barrier and `maxWaitMs` only the SECOND one.
// Hardcoding reason:"rate_limit" would drop the primary barrier and leave
// only the ceiling — which does NOT cover a quota_exhausted lock carrying a
// SHORT upstream retry-after. Model lockouts are recorded for all strategies,
// so the real reason is always available.
if (comboCooldownWaitEnabled && earliestRetryAfter) {
const decision: ResolveComboCooldownDecisionResult = resolveComboCooldownWaitDecision({
targets: orderedTargets,
earliestRetryAfter,
attempt: comboCooldownAttempt,
budgetLeftMs: comboCooldownBudgetLeftMs,
settings: resilienceSettings.comboCooldownWait,
// Key each lookup on the TARGET's own model: quota-share combos are
// single-model/multi-account (so this is identical to the previous
// orderedTargets[0] behavior), but heterogeneous combos carry a
// different model per target.
lookupLock: (provider, connectionId, target) => {
const rawModel = parseModel(target?.modelStr ?? "").model || "";
if (!rawModel) return null;
return getModelLockoutInfo(provider, connectionId, rawModel);
},
computeWaitMs: (retryAfter) => computeClosestRetryAfter(retryAfter).waitMs,
});
if (decision.wait) {
log.info(
"COMBO",
`${strategy} cooldown wait: ${msg} — waiting ${Math.ceil(
decision.waitMs / 1000
)}s (reason=${decision.reason ?? "?"}) then retrying (attempt ${
comboCooldownAttempt + 1
}/${resilienceSettings.comboCooldownWait.maxAttempts})`
);
const completed = await waitForCooldownAwareRetry(decision.waitMs, signal);
if (!completed) {
log.info("COMBO", `${strategy} cooldown wait aborted by client disconnect`);
return errorResponse(499, "Request aborted");
}
comboCooldownAttempt += 1;
comboCooldownBudgetLeftMs = Math.max(0, comboCooldownBudgetLeftMs - decision.waitMs);
return dispatchWithCooldownRetry();
if (decision.wait) {
log.info(
"COMBO",
`${strategy} cooldown wait: ${msg} — waiting ${Math.ceil(
decision.waitMs / 1000
)}s (reason=${decision.reason ?? "?"}) then retrying (attempt ${
comboCooldownAttempt + 1
}/${resilienceSettings.comboCooldownWait.maxAttempts})`
);
const completed = await waitForCooldownAwareRetry(decision.waitMs, signal);
if (!completed) {
log.info("COMBO", `${strategy} cooldown wait aborted by client disconnect`);
return errorResponse(499, "Request aborted");
}
comboCooldownAttempt += 1;
comboCooldownBudgetLeftMs = Math.max(0, comboCooldownBudgetLeftMs - decision.waitMs);
return dispatchWithCooldownRetry();
}
}
// Retry-after decoration is separate from the wait decision above: only
// rate-limit-class final statuses may carry a `(reset after ...)` suffix
// (see unavailableRetryGate.ts — do not stitch a peer target's window onto
// a config-class status like 403/422).
if (earliestRetryAfter && isRetryAfterEligibleStatus(status)) {
const retryHuman = formatRetryAfter(toRetryAfterDisplayValue(earliestRetryAfter));
log.warn("COMBO", `All models failed | ${msg} (${retryHuman})`);
return unavailableResponse(status, msg, earliestRetryAfter, retryHuman);

View File

@@ -38,18 +38,22 @@ export const DEFAULT_COMBO_TARGET_TIMEOUT_MS = 120_000;
export const COMBO_TARGET_TIMEOUT_WAIT_BUFFER_MS = 10_000;
/**
* Whether a combo's cooldown-aware wait+retry (#7360) engages for this request: only
* "quota-share" and "auto" strategies wait out a short transient cooldown instead of
* crystallizing a 429 into a combo-level failure, and only when the operator has the
* feature enabled. Shared by combo.ts (to decide whether to wait) and comboSetup.ts (to
* size the per-target timeout floor so it doesn't cut the wait off early — see
* Whether a combo's cooldown-aware wait+retry (#7360 / #7301) engages for this request.
* When the operator has the feature enabled, EVERY combo strategy waits out a short
* transient cooldown instead of crystallizing a 429 into a combo-level failure.
* Shared by combo.ts (to decide whether to wait) and comboSetup.ts (to size the
* per-target timeout floor so it doesn't cut the wait off early — see
* resolveComboTargetTimeoutMsForCombo below).
*
* `strategy` is retained in the signature for call-site clarity; eligibility is
* strategy-agnostic (the wait path in combo.ts already uses the real model-lockout
* reason for every strategy).
*/
export function isComboCooldownWaitEligible(
strategy: string,
_strategy: string,
comboCooldownWait: Pick<ComboCooldownWaitSettings, "enabled">
): boolean {
return (strategy === "quota-share" || strategy === "auto") && comboCooldownWait.enabled;
return comboCooldownWait.enabled;
}
/**

View File

@@ -702,10 +702,10 @@ function ComboCooldownWaitCard({
setDraft(value);
}, [value]);
const title = t("resilienceComboCooldownWaitTitle") || "Quota-share combo cooldown wait";
const title = t("resilienceComboCooldownWaitTitle") || "Combo cooldown wait";
const desc =
t("resilienceComboCooldownWaitDesc") ||
"For quota-share combos only: wait out a short transient cooldown and re-dispatch instead of returning a 429 immediately. Never waits on quota_exhausted.";
"For all combo strategies: wait out a short transient cooldown and re-dispatch instead of returning a 429 immediately. Never waits on quota_exhausted.";
return (
<Card className="p-6">
@@ -738,7 +738,7 @@ function ComboCooldownWaitCard({
label={t("resilienceEnableServerWait") || "Enabled"}
description={
t("resilienceComboCooldownWaitToggleDesc") ||
"Quota-share combos only; never waits on quota_exhausted."
"All combo strategies; never waits on quota_exhausted."
}
checked={draft.enabled}
onChange={(enabled) => setDraft((prev) => ({ ...prev, enabled }))}

View File

@@ -7458,9 +7458,9 @@
"resilienceEnableServerWaitDesc": "When enabled, OmniRoute waits for the first cooldown to expire and retries automatically.",
"resilienceMaxAttempts": "Maximum attempts",
"resilienceMaxWaitPerAttempt": "Maximum wait per attempt",
"resilienceComboCooldownWaitTitle": "Quota-share combo cooldown wait",
"resilienceComboCooldownWaitDesc": "For quota-share combos only: wait out a short transient cooldown and re-dispatch instead of returning a 429 immediately. Never waits on quota_exhausted.",
"resilienceComboCooldownWaitToggleDesc": "Quota-share combos only; never waits on quota_exhausted.",
"resilienceComboCooldownWaitTitle": "Combo cooldown wait",
"resilienceComboCooldownWaitDesc": "For all combo strategies: wait out a short transient cooldown and re-dispatch instead of returning a 429 immediately. Never waits on quota_exhausted.",
"resilienceComboCooldownWaitToggleDesc": "All combo strategies; never waits on quota_exhausted.",
"resilienceComboCooldownMaxWaitMs": "Maximum wait per attempt",
"resilienceComboCooldownBudgetMs": "Total wait budget",
"resilienceQuotaShareConcurrencyTitle": "Quota-share per-connection concurrency",

View File

@@ -95,8 +95,8 @@ export const DEFAULT_RESILIENCE_SETTINGS: ResilienceSettings = {
},
// Wait at most 90s for a single short transient cooldown (covers Gemini-class
// TPM/RPM windows, which report ~60s retry-after live — #7360), at most 5
// redispatch cycles, never more than 300s (5 min) total. Active for
// quota-share and auto combos, and only for transient (non quota_exhausted)
// redispatch cycles, never more than 300s (5 min) total. Active for every
// combo strategy when enabled, and only for transient (non quota_exhausted)
// reasons.
comboCooldownWait: {
enabled: true,

View File

@@ -65,12 +65,12 @@ export interface WaitForCooldownSettings {
}
/**
* Quota-share combo cooldown-aware retry (Variante A). A quota-share (`qtSd/…`)
* combo that would crystallize a 429 `model_cooldown` for a SHORT transient
* cooldown waits it out and re-dispatches instead. Guards (gating + the
* `quota_exhausted`/auth/not-found exclusions) live in
* open-sse/services/combo/comboCooldownRetry.ts; `maxWaitMs`/`maxAttempts`/
* `budgetMs` bound a single wait, the retry cycles, and the total wait time.
* Combo cooldown-aware retry. When enabled, any combo strategy that would
* crystallize a 429 `model_cooldown` for a SHORT transient cooldown waits it
* out and re-dispatches instead. Guards (gating + the `quota_exhausted`/auth/
* not-found exclusions) live in open-sse/services/combo/comboCooldownRetry.ts;
* `maxWaitMs`/`maxAttempts`/`budgetMs` bound a single wait, the retry cycles,
* and the total wait time.
*/
export interface ComboCooldownWaitSettings {
enabled: boolean;

View File

@@ -14,6 +14,9 @@ const {
const { createComboSchema, updateComboDefaultsSchema } =
await import("../../src/shared/validation/schemas.ts");
const { MAX_TIMER_TIMEOUT_MS } = await import("../../src/shared/utils/runtimeTimeouts.ts");
const { ROUTING_STRATEGY_VALUES, INTERNAL_ROUTING_STRATEGY_VALUES } =
await import("../../src/shared/constants/routingStrategies.ts");
const ALL_COMBO_STRATEGIES = [...ROUTING_STRATEGY_VALUES, ...INTERNAL_ROUTING_STRATEGY_VALUES];
test("getDefaultComboConfig returns a fresh copy of the defaults", () => {
const first = getDefaultComboConfig();
@@ -321,40 +324,32 @@ test("resolveComboTargetTimeoutMs falls back to the saner combo default when uns
assert.equal(resolveComboTargetTimeoutMs({}, 0, 120000), 0);
});
// #7360 follow-up: a "default" auto-strategy combo hitting Gemini TPM/RPM on both
// targets waits out cooldowns for up to comboCooldownWait.budgetMs (default 130s), but
// DEFAULT_COMBO_TARGET_TIMEOUT_MS (120s) is shorter — the per-target timeout was cutting
// the wait off early and returning a synthetic 524 instead of letting the wait finish.
test("isComboCooldownWaitEligible only engages for quota-share/auto with the feature enabled", () => {
assert.equal(isComboCooldownWaitEligible("auto", { enabled: true }), true);
assert.equal(isComboCooldownWaitEligible("quota-share", { enabled: true }), true);
assert.equal(isComboCooldownWaitEligible("auto", { enabled: false }), false);
assert.equal(isComboCooldownWaitEligible("fill-first", { enabled: true }), false);
assert.equal(isComboCooldownWaitEligible("priority", { enabled: true }), false);
// #7360 / #7301: any strategy with comboCooldownWait enabled waits out cooldowns for up
// to comboCooldownWait.budgetMs, so the per-target timeout floor must cover that budget
// (DEFAULT_COMBO_TARGET_TIMEOUT_MS alone would cut a long wait short into a 524).
test("isComboCooldownWaitEligible engages for every strategy when the feature is enabled", () => {
for (const strategy of ALL_COMBO_STRATEGIES) {
assert.equal(isComboCooldownWaitEligible(strategy, { enabled: true }), true);
assert.equal(isComboCooldownWaitEligible(strategy, { enabled: false }), false);
}
});
test("resolveComboTargetTimeoutMsForCombo raises the floor to cover the cooldown-wait budget for eligible strategies", () => {
test("resolveComboTargetTimeoutMsForCombo raises the floor to cover the cooldown-wait budget when enabled", () => {
const comboCooldownWait = { enabled: true, budgetMs: 130000 };
const floor = 130000 + COMBO_TARGET_TIMEOUT_WAIT_BUFFER_MS;
const disabled = { enabled: false, budgetMs: 130000 };
// Wait-eligible strategy: floor is budget + buffer (150s), not the 120s default.
assert.equal(
resolveComboTargetTimeoutMsForCombo({}, 600000, "auto", comboCooldownWait),
130000 + COMBO_TARGET_TIMEOUT_WAIT_BUFFER_MS
);
assert.equal(
resolveComboTargetTimeoutMsForCombo({}, 600000, "quota-share", comboCooldownWait),
130000 + COMBO_TARGET_TIMEOUT_WAIT_BUFFER_MS
);
// Not wait-eligible (wrong strategy, or feature disabled): unchanged 120s default.
assert.equal(
resolveComboTargetTimeoutMsForCombo({}, 600000, "fill-first", comboCooldownWait),
DEFAULT_COMBO_TARGET_TIMEOUT_MS
);
assert.equal(
resolveComboTargetTimeoutMsForCombo({}, 600000, "auto", { enabled: false, budgetMs: 130000 }),
DEFAULT_COMBO_TARGET_TIMEOUT_MS
);
// Feature on: floor is budget + buffer for every strategy; off: 120s default.
for (const strategy of ALL_COMBO_STRATEGIES) {
assert.equal(
resolveComboTargetTimeoutMsForCombo({}, 600000, strategy, comboCooldownWait),
floor
);
assert.equal(
resolveComboTargetTimeoutMsForCombo({}, 600000, strategy, disabled),
DEFAULT_COMBO_TARGET_TIMEOUT_MS
);
}
// A small budget below the 120s default never lowers the floor.
assert.equal(
@@ -364,12 +359,7 @@ test("resolveComboTargetTimeoutMsForCombo raises the floor to cover the cooldown
// Explicit per-combo targetTimeoutMs still wins over the derived floor.
assert.equal(
resolveComboTargetTimeoutMsForCombo(
{ targetTimeoutMs: 45000 },
600000,
"auto",
comboCooldownWait
),
resolveComboTargetTimeoutMsForCombo({ targetTimeoutMs: 45000 }, 600000, "auto", comboCooldownWait),
45000
);

View File

@@ -9,8 +9,8 @@
* 2. A 403 (quota_exhausted, locked until midnight) → NO wait, the 403/429 is
* propagated immediately (the helper's critical exclusion).
* 3. Client abort DURING the wait → 499 "Request aborted".
* 4. strategy="priority" (non quota-share) → unchanged: the 429 is propagated
* immediately with NO wait.
* 4. strategy="priority" (and every other strategy) also waits out a SHORT
* transient 429 when comboCooldownWait is enabled — same decision helper.
* 5. comboCooldownWait disabled in settings → unchanged: 429 propagated, no wait.
*
* The waits use a real (short) cooldown so the real setTimeout in

View File

@@ -20,9 +20,8 @@
* The non-quota-share (priority) scenario was UPDATED for the "universal
* cooldown-aware retry" change: comboCooldownWait is no longer gated on
* `strategy === "quota-share"` — every combo strategy now waits out a SHORT
* transient 429 and re-dispatches (using a "rate_limit" reason and the
* earliest retry-after hint directly, since non quota-share strategies have no
* per-connection model-lockout tracking to consult). It used to assert the
* transient 429 and re-dispatches via the same resolveComboCooldownWaitDecision
* path (real model-lockout reason + allow-list). It used to assert the
* OPPOSITE (immediate propagation, no wait) — that assertion is now testing
* dead behavior, so it was rewritten to assert the new intended behavior
* instead of being deleted or weakened.
@@ -147,11 +146,8 @@ test("non quota-share (priority): short 429 cooldown → waits and re-dispatches
const handleSingleModel = async () => {
calls += 1;
// 1st dispatch: transient 429 with a short retry-after hint. 2nd dispatch
// (after the universal cooldown wait): success. Priority combos have no
// per-connection model-lockout tracking, so this exercises the
// `shouldWaitForComboCooldown({ reason: "rate_limit", ... })` path fed
// directly by the earliest retry-after hint (not resolveComboCooldownWaitDecision's
// per-target lock lookup, which stays quota-share-only).
// (after the universal cooldown wait): success. Exercises the shared
// resolveComboCooldownWaitDecision path (real model-lockout reason).
return calls === 1 ? rateLimitResponse(429) : okResponse();
};
@@ -182,18 +178,18 @@ test("non quota-share (priority): a quota_exhausted lock drives the decision wit
//
// Barrier 1 = the reason allow-list. Barrier 2 = the maxWaitMs ceiling.
// This scenario is engineered so ONLY barrier 1 can stop the wait:
// - modelLockout.errorCodes is [403] ONLY, so model-a's 429 crystallizes
// status 429 (the sole status that opens the cooldown-wait branch) WITHOUT
// recording a competing `rate_limit` lock.
// - modelLockout.errorCodes is [403] ONLY, so model-a's 429 contributes a
// retry-after hint (opens the cooldown-wait decision) WITHOUT recording a
// competing `rate_limit` lock.
// - model-b's 403 records the only lock in play: `quota_exhausted`. It is
// therefore the lock resolveComboCooldownWaitDecision picks, so its reason
// is what drives the decision.
// - The resulting wait is SHORT (well under maxWaitMs=5000), so barrier 2
// lets it through. Only the allow-list can reject it.
//
// With the reason hardcoded to "rate_limit" (as the non-quota-share path did
// before), barrier 1 is gone and this exact input waits + redispatches against
// a quota-exhausted model — verified: the same test yields 6 dispatches.
// With the reason hardcoded to "rate_limit", barrier 1 is gone and this exact
// input waits + redispatches against a quota-exhausted model — verified: the
// same test yields 6 dispatches.
const calls: string[] = [];
const handleSingleModel = async (_body: unknown, modelStr: string) => {
calls.push(modelStr);
@@ -222,7 +218,10 @@ test("non quota-share (priority): a quota_exhausted lock drives the decision wit
allCombos: null,
});
assert.equal(res.status, 429, "the crystallized 429 must be propagated, not retried");
// Final status is the last target's 403 (aggregation last-wins). The security
// invariant is that the allow-list rejected the wait — not that a peer 429 is
// re-surfaced as the HTTP status.
assert.equal(res.status, 403, "quota_exhausted target status crystallizes; wait must not redispatch");
// Deterministic proof (no wall-clock dependency, so it cannot flake under
// CI-runner contention): each target is dispatched EXACTLY ONCE. Had the wait
// fired, the whole set loop would re-run — maxAttempts=2 within the 8s budget

View File

@@ -95,10 +95,10 @@ const resilienceTabSettingsKeys = [
];
const quotaShareResilienceSettingsMessages = {
resilienceComboCooldownWaitTitle: "Quota-share combo cooldown wait",
resilienceComboCooldownWaitTitle: "Combo cooldown wait",
resilienceComboCooldownWaitDesc:
"For quota-share combos only: wait out a short transient cooldown and re-dispatch instead of returning a 429 immediately. Never waits on quota_exhausted.",
resilienceComboCooldownWaitToggleDesc: "Quota-share combos only; never waits on quota_exhausted.",
"For all combo strategies: wait out a short transient cooldown and re-dispatch instead of returning a 429 immediately. Never waits on quota_exhausted.",
resilienceComboCooldownWaitToggleDesc: "All combo strategies; never waits on quota_exhausted.",
resilienceComboCooldownMaxWaitMs: "Maximum wait per attempt",
resilienceComboCooldownBudgetMs: "Total wait budget",
resilienceQuotaShareConcurrencyTitle: "Quota-share per-connection concurrency",