[defer] feat(combo): universal cooldown-aware retry & auto-strategy combo-ref guard (#7301)

* feat(combo): universal cooldown-aware retry & auto-strategy combo-ref guard

Two changes:

1. Universal cooldown-aware retry (combo.ts):
   - Remove strategy==="quota-share" gate from comboCooldownWaitEnabled
   - Enables all 18 combo strategies (priority, weighted, round-robin, etc.)
     to wait out a short transient cooldown and retry the full set
   - Non-quota-share strategies use shouldWaitForComboCooldown directly
     with earliestRetryAfter and reason="rate_limit" (no per-model lockout)
   - quota-share retains its existing per-target model lockout logic

2. Auto-strategy combo-ref guard (autoStrategy.ts):
   - expandAutoComboCandidatePool now detects kind==="combo-ref" entries
   - When present, returns eligibleTargets without expanding to ALL providers
   - Fixes scenario where an "auto" combo with combo-ref delegates to a
     sub-combo but pulls in every model from every active provider

* feat(combo): global comboTimeoutMs + aggregated error diagnostics

Adds two features to improve combo resilience and debuggability:

1. Global combo timeout (comboTimeoutMs)
   - Configurable per-combo via DEFAULT_COMBO_CONFIG (default 0 = disabled)
   - After each target completes, checks if total elapsed time exceeds limit
   - When exceeded, stops trying further targets and returns 504 immediately
   - Backward-compatible: 0 preserves legacy unlimited-iteration behavior

2. Aggregated error diagnostics
   - comboErrors array accumulates per-model failure details (model, status, error)
   - On combo timeout or all-models-exhausted, returns a message listing the
     first (up to 5) model-level errors with their HTTP status codes
   - Enables operators to see WHICH models failed and WHY without digging
     through individual server logs

* test(combo): cover universal cooldown-aware retry, comboTimeoutMs, and combo-ref guard

Adds/updates automated coverage for this PR's production changes (PR Test
Policy requires tests alongside src/open-sse/electron/bin changes):

- Update the "preserves the first failure status" expectation in
  combo-routing-engine.test.ts: the aggregated per-model error-diagnostics
  suffix is new intended output, not a regression.
- Add two new tests for the global comboTimeoutMs feature: the combo stops
  dispatching further targets and returns 504/COMBO_TIMEOUT with aggregated
  diagnostics once the ceiling trips, and comboTimeoutMs=0 (default) never
  trips it.
- Rewrite the non-quota-share (priority) cooldown-wait scenario in
  combo-quota-share-cooldown-wait-timing.test.ts: comboCooldownWait is no
  longer gated on strategy === "quota-share", so a priority combo now waits
  out a short 429 and re-dispatches too (via shouldWaitForComboCooldown with
  reason "rate_limit"), instead of propagating immediately as before. Also
  adds the disabled-flag counterpart for parity with the quota-share suite.
- Add coverage for the #COMBO-REF guard in expandAutoComboCandidatePool:
  a combo whose models array contains a kind:"combo-ref" entry must not be
  expanded to every model of every active provider.

Co-authored-by: diegosouzapw <diegosouza.pw@gmail.com>

* test(combo): keep the new comboTimeoutMs tests free of no-explicit-any

The two new tests initially copied the neighbouring tests' `any`-typed
handleSingleModel params / json() casts. Those neighbours are pre-existing
violations frozen in config/quality/eslint-suppressions.json at a count of 261
for this file, so the 7 new occurrences pushed it to 268 and broke `npm run
lint` (no-explicit-any is an error in tests/ since #6218; new violations must
be fixed, not re-frozen).

Type the new tests properly instead: `unknown`/`string` params, a
ComboErrorPayload interface for the parsed body, and drop the unused
`relayOptions: null as any` (the sibling combo cooldown suites already omit
it). Back to exactly 261 — the suppression file is untouched.

Co-authored-by: diegosouzapw <diegosouza.pw@gmail.com>

* fix(combo): gate the universal cooldown retry on the REAL lock reason, not a hardcoded "rate_limit"

The universal cooldown-aware retry kept the quota-share path on
resolveComboCooldownWaitDecision but gave every OTHER strategy a shortcut that
hardcoded `reason: "rate_limit"` and fed shouldWaitForComboCooldown the
earliestRetryAfter directly.

comboCooldownRetry.ts documents TWO deliberate barriers ("SECURITY —
quota_exhausted must be excluded"): (1) the reason allow-list, and (2) the
maxWaitMs ceiling, explicitly called the SECOND barrier. Hardcoding the reason
removed barrier 1 for 17 of the 18 strategies and left only the ceiling — which
does NOT cover a quota_exhausted lock whose wait lands under maxWaitMs. In that
case the combo waits, redispatches against a model that is locked until the
quota resets, and burns the retry budget for nothing.

The shortcut's premise ("non-quota-share combos have no per-connection model
lockout tracking") is also false: recordModelLockoutFailure in the target loop
is not gated on quota-share, so every strategy records model lockouts and the
real reason is always available.

Fix: one decision path for every strategy, always through
resolveComboCooldownWaitDecision, so the reason always crosses the allow-list.
The lock lookup is now keyed on each TARGET's own model (via a new third
`target` arg on lookupLock) — quota-share combos are single-model/multi-account
so this is identical to the previous orderedTargets[0] behavior, but
heterogeneous combos (priority, weighted, round-robin, …) carry a different
model per target and would otherwise miss every lock but the first.

Regression guard (tests/unit/serial/combo-quota-share-cooldown-wait-timing.test.ts):
a priority combo where modelLockout.errorCodes=[403] leaves the 403's
quota_exhausted lock as the only one in play while a 429 crystallizes the
status, and the resulting wait is short enough that the ceiling lets it through
— so only the allow-list can stop it. Verified failing-then-passing: with the
hardcoded reason the combo makes 6 dispatches (wait+redispatch x2); with the
real reason it makes 2 and propagates the 429.

Co-authored-by: diegosouzapw <diegosouza.pw@gmail.com>

* chore(quality): rebaseline file-size for PR #7301 own growth (combo +91, combo-routing-engine test +68)

---------

Co-authored-by: diegosouzapw <diegosouza.pw@gmail.com>
This commit is contained in:
ViFigueiredo
2026-07-21 16:30:37 -03:00
committed by GitHub
parent 0844163966
commit 577bbf3e47
9 changed files with 404 additions and 38 deletions

View File

@@ -1,4 +1,5 @@
{
"_rebaseline_2026_07_21_7301_universal_cooldown_retry": "PR #7301 (ViFigueiredo, feat/universal-cooldown-retry) own growth, surfaced during rebase-onto-tip reconciliation (fast-gates PR->release do not run check:file-size): open-sse/services/combo.ts 3388->3479 (+91) generalizes the existing quota-share-only cooldown-aware retry (dispatchWithCooldownRetry) to ALL combo strategies (priority/weighted/round-robin/etc), gates it on the model lockout's REAL reason (not a hardcoded \"rate_limit\") via the existing getModelLockoutInfo/resolveComboCooldownWaitDecision chokepoint, and adds a global comboTimeoutMs guard + aggregated per-target error diagnostics on exhaustion. Companion leaves open-sse/services/combo/comboCooldownRetry.ts (+29), combo/autoStrategy.ts (+9, auto-strategy combo-ref guard so a combo cannot recursively reference itself as a candidate), combo/comboSetup.ts (+3), comboConfig.ts (+6) all stay under cap. Irreducible orchestration wiring at the existing dispatch chokepoint (mirrors the quota-share-only precedent this PR generalizes); not extractable without hiding the retry loop. Covered by tests/unit/combo-auto-candidate-expansion.test.ts (+61, combo-ref guard), tests/unit/combo-routing-engine.test.ts (+68, universal retry across strategies + comboTimeoutMs, no-explicit-any clean), tests/unit/serial/combo-quota-share-cooldown-wait-timing.test.ts (+136, quota_exhausted vs rate_limit reason gating, disabled-flag passthrough). Structural shrink of combo.ts tracked in #3501.",
"_rebaseline_2026_07_21_7935_vi_locale_residual_ui": "PR #7935 (nguyenha935, fix/vietnamese-locale-residual) own growth: 9 dashboard components gained `useTranslations()` wiring (import + hook call + a handful of `t(\"key\")` call-sites replacing hardcoded English strings) as part of restoring i18n coverage — ComboHealthTab.tsx 1028->1031 (+3), cloud-agents/page.tsx 922->931 (+9), PoolWizard.tsx 1007->1022 (+15), EndpointPageClient.tsx 2612->2615 (+3), health/page.tsx 1091->1095 (+4), ProviderOnboardingWizard.tsx 912->948 (+36, largest — several previously-hardcoded wizard step labels/descriptions), PricingTab.tsx 1012->1017 (+5), ProxyRegistryManager.tsx 1461->1464 (+3), BudgetTab.tsx 1016->1028 (+12). All additions are literal `t(...)`/`tc(...)` call-site swaps for existing UI text, verified byte-identical in intent against the corresponding new `src/i18n/messages/{en,vi}.json` keys (see tests/unit/dashboard-localization-contract.test.ts, tests/unit/i18n-vi-completeness.test.ts, tests/unit/gamification-display-contract.test.ts, tests/unit/cli-catalog-display-contract.test.ts added by the same PR). Fast-gates PR->release do not run check:file-size, so this surfaced only during rebase-onto-tip reconciliation.",
"_rebaseline_2026_07_21_7908_chathelpers_abort_guard": "PR #7908 (insoln, don't cool down accounts or trip the breaker on client-side stream aborts, #7907) own growth: src/sse/handlers/chatHelpers.ts 876->877 (+1 = the single `isLocalStreamLifecycleError(failure?.message ?? failure)` clause added to executeChatWithBreaker's onStreamFailure connection-disable check, verified working by the existing #4602 test + the PR's own circuit-breaker-client-abort.test.ts, no regressions). Irreducible call-site wiring at the existing failure-classification chokepoint. Fast-gates PR->release do not run check:file-size, so this surfaced only during the /green-prs pre-merge pass.",
"_rebaseline_2026_07_21_7908_combo_breaker_abort_guard": "PR #7908 pre-green fix (green-prs pipeline): shouldRecordProviderBreakerFailure() (open-sse/services/combo/comboPredicates.ts, not frozen) gained an `error` field so a client-side stream abort no longer trips the whole-provider circuit breaker in the combo path (mirrors the connection-cooldown fix shouldSkipConnDisable() already applies for the same #4602/#7907 policy). Own growth: open-sse/services/combo.ts 3387->3388 (+1, irreducible call-site wiring — the single new `error: errorText,` field passed at the existing shouldRecordProviderBreakerFailure() call site inside handleComboChat's executeTarget). Covered by tests/unit/circuit-breaker-abort-provider-trip-7907.test.ts.",
@@ -189,7 +190,7 @@
"_rebaseline_2026_06_24_headroom_strategy": "Headroom-aware connection selection (dario technique): combo.ts 3168->3180 (+12 = a new `else if (strategy === \"headroom\")` dispatch branch in handleComboChat that delegates to orderTargetsByHeadroom + its log line, plus the import). The actual logic lives OUT of the god-file: the pure ranker rankByHeadroom/computeHeadroom is the new leaf open-sse/services/combo/headroomRanking.ts (91 LOC, <cap) and the async orderer orderTargetsByHeadroom is appended to the existing open-sse/services/combo/quotaStrategies.ts (<cap) next to its sibling reset-aware/reset-window orderers (reuses their connection-expansion machinery). headroom = 1 - max(util_5h, util_7d) from getSaturation (src/lib/quota/saturationSignals.ts), prefers the connection with the most free capacity. Only the dispatch wiring is irreducible at the existing combo strategy chokepoint (mirrors the reset-aware/reset-window/context-optimized branches); not extractable without hiding the call site. fill-first stays default; all existing strategies untouched. Covered by tests/unit/combo-headroom-ranking.test.ts (pure helper) + tests/unit/combo-headroom-strategy.test.ts (orderer, saturation injected). Structural shrink of combo.ts tracked in #3501.",
"_rebaseline_2026_06_24_quota_share_strategy": "Dedicated quota-share strategy (Phase 3 #9): combo.ts 3180->3190 (+10 = one new `else if (strategy === \"quota-share\")` dispatch branch in handleComboChat that delegates 100% to selectQuotaShareTarget + its log line, plus the import). All the new logic lives OUT of the god-file in two new leaves under open-sse/services/combo/: quotaShareInflight.ts (in-flight counter with TTL/lease, ~150 LOC <cap) and quotaShareStrategy.ts (per-model bucket gating via isBucketSaturated + DRR proportional to weight + P2C over in-flight, ~240 LOC <cap). Only the dispatch wiring is irreducible at the existing combo strategy chokepoint (mirrors the headroom/reset-aware/reset-window/context-optimized branches); not extractable without hiding the call site. ZERO existing strategy cases were modified — only this branch was added, and the qtSd/ combos switched from fill-first to quota-share in src/lib/quota/quotaCombos.ts. Covered by tests/unit/quota-share-strategy.test.ts (gating, DRR fairness, P2C in-flight, fail-open, activation). Structural shrink of combo.ts tracked in #3501.",
"_rebaseline_2026_06_24_task_aware_routing": "Task-aware routing strategy (port PR #2045, OmniRoute #4945): combo.ts 3190->3225 (+35) = one new `else if (strategy === \"task-aware\")` dispatch branch delegating 100% to selectTaskAwareTarget + its imports/log lines. All scoring/classification logic lives OUT of the god-file in the new leaf open-sse/services/taskAwareRouting.ts (553 LOC <cap). Only the dispatch wiring is irreducible at the existing combo strategy chokepoint (mirrors quota-share/headroom/reset-aware branches). ZERO existing strategy cases modified. Covered by tests/unit/combo-task-aware.test.ts (35 tests). Structural shrink of combo.ts tracked in #3501.",
"open-sse/services/combo.ts": 3388,
"open-sse/services/combo.ts": 3479,
"_rebaseline_2026_06_26_fidelity_gate_extraction": "Milestone-B fidelity-gate wiring residual: bodyToText+gateAdvance extracted to fidelityGateStep.ts (889->854, -35), but the StackOptions.fidelityGate field, the `const fidelityGate` reads at the two stacked-loop dispatch chokepoints, and the import of FidelityGateConfig are irreducible wiring that cannot leave strategySelector without an architectural refactor of the pre-existing stacked pipeline. Net: 889->854 (+6 vs the pre-Milestone-B frozen 848). Covered by tests/unit/compression/*.test.ts (940 pass).",
"_rebaseline_2026_06_28_5243_risk_gate_prepass": "PR #5243 (compression risk-gate pre-pass) own growth: open-sse/services/compression/strategySelector.ts 854->899 (+45). The three exported entry points (applyCompression/applyStackedCompression/applyStackedCompressionAsync) become thin wrappers over pure-extracted private bodies (runCompression/runStackedCompression/runStackedCompressionAsync) so the risk-gate mask->run->restore wrapper sits strictly OUTSIDE the per-step loop — a single universal integration point. The wrapper logic itself (resolveRiskGate/withRiskGate) lives in the new riskGate/strategyWrap.ts (<cap); the residual growth is the duplicated thin-wrapper signatures + the extracted bodies' dispatch boundary, guarded by a byte-identical parity test (riskGateIntegration). Default off (DEFAULT_COMPRESSION_CONFIG unchanged). Not extractable without hiding the dispatch boundary, mirroring prior compression rebaselines. Structural shrink tracked in #3501.",
"_rebaseline_2026_06_29_5286_memoization": "PR #5286 own growth: strategySelector.ts 899->960 (+61 = the opt-in result-memoization branches in applyCompression/applyCompressionAsync — principal+determinism gate, makeMemoKey lookup/store with model+supportsVision folded into the key, recompute-with-memo-off). Default off (memoizeCompressionResults), so zero behavior change. The memo helpers live in the leaf resultMemo.ts (<cap); the chokepoint wiring here is not extractable. Structural shrink of this hot-path file tracked in #3501.",
@@ -321,7 +322,7 @@
"tests/unit/chatcore-translation-paths.test.ts": 2810,
"tests/unit/chatgpt-web.test.ts": 3170,
"tests/unit/combo-config.test.ts": 881,
"tests/unit/combo-routing-engine.test.ts": 3243,
"tests/unit/combo-routing-engine.test.ts": 3311,
"tests/unit/combo-strategy-fallbacks.test.ts": 880,
"tests/unit/db-core-init.test.ts": 877,
"tests/unit/db-migration-runner.test.ts": 1499,

View File

@@ -130,7 +130,10 @@ import {
releaseRejectedQualityResponse,
toRetryAfterDisplayValue,
} from "./combo/validateQuality.ts";
import { resolveComboCooldownWaitDecision } from "./combo/comboCooldownRetry.ts";
import {
resolveComboCooldownWaitDecision,
ResolveComboCooldownDecisionResult,
} from "./combo/comboCooldownRetry.ts";
import {
computeClosestRetryAfter,
waitForCooldownAwareRetry,
@@ -1418,11 +1421,21 @@ export async function handleComboChat({
// re-runs ONLY the set loop (selection / shadow routing / setup above stay
// untouched), preserving the pre-existing `continue`-to-top-of-set-loop
// semantics exactly.
const comboCooldownWaitEnabled =
strategy === "quota-share" && resilienceSettings.comboCooldownWait.enabled;
const comboCooldownWaitEnabled = resilienceSettings.comboCooldownWait.enabled;
let comboCooldownAttempt = 0;
let comboCooldownBudgetLeftMs = resilienceSettings.comboCooldownWait.budgetMs;
// Global combo timeout: when set (>0), limits total wall-clock time the combo
// spends iterating through targets. After each target completes, if elapsed time
// exceeds comboTimeoutMs, remaining targets are skipped and a 504 with aggregated
// error diagnostics is returned. 0 = disabled (backward-compatible, unlimited).
const comboTimeoutMs = config.comboTimeoutMs || 0;
const comboStartTime = Date.now();
let comboExpired = false;
// Accumulator for per-model error details across targets in the current set try.
// Reset at the start of each set retry (same lifecycle as lastError/recordedAttempts).
let comboErrors: Array<{ model: string; status: number; error: string }> = [];
// FASE 2.1: per-connection concurrency limit for quota-share. The gating in
// selectQuotaShareTarget is fail-open and cannot hard-limit a single-connection
// pool, so we serialize concurrent requests to the selected account through a
@@ -1463,6 +1476,7 @@ export async function handleComboChat({
const startTime = Date.now();
let fallbackCount = 0;
let recordedAttempts = 0;
comboErrors = [];
// QA P0: assemble a sanitized diagnostic trace from the state already in scope
// (pool size + this set-try's exhausted providers/connections + attempt order +
@@ -2237,6 +2251,7 @@ export async function handleComboChat({
});
recordedAttempts++;
lastError = errorText || String(result.status);
comboErrors.push({ model: modelStr, status: result.status, error: errorText || String(result.status) });
if (!lastStatus) lastStatus = result.status;
if (i > 0) fallbackCount++;
log.warn("COMBO", `Model ${modelStr} failed with body-specific error, stopping combo`);
@@ -2330,6 +2345,7 @@ export async function handleComboChat({
});
recordedAttempts++;
lastError = errorText || String(result.status);
comboErrors.push({ model: modelStr, status: result.status, error: errorText || String(result.status) });
if (!lastStatus) lastStatus = result.status;
if (i > 0) fallbackCount++;
// Wire combo failures into the resilience dashboard (model-level lockout)
@@ -2402,7 +2418,7 @@ export async function handleComboChat({
};
for (let i = 0; i < orderedTargets.length; i++) {
if (anySuccess) break;
if (anySuccess || comboExpired) break;
const abortController = new AbortController();
abortControllers.set(i, abortController);
@@ -2447,6 +2463,17 @@ export async function handleComboChat({
} else {
await Promise.race([task, globalPromise]);
}
// Global combo timeout check: after each target completes, stop trying
// further targets if the total elapsed time exceeds comboTimeoutMs.
if (!anySuccess && comboTimeoutMs > 0 && Date.now() - comboStartTime >= comboTimeoutMs) {
comboExpired = true;
log.info(
"COMBO",
`Combo global timeout (${comboTimeoutMs}ms) reached after ` +
`${i + 1}/${orderedTargets.length} targets (${recordedAttempts} attempted) — stopping`
);
}
}
if (!anySuccess && runningTasks.size > 0) {
@@ -2457,6 +2484,40 @@ export async function handleComboChat({
return await globalPromise;
}
// Global combo timeout: return aggregated error immediately, skipping set retries.
if (comboExpired) {
const summary = comboErrors
.slice(0, 5)
.map((e) => `${e.model} (${e.status})`)
.join(", ");
const msg =
`Combo global timeout (${comboTimeoutMs}ms) after ${recordedAttempts}/${orderedTargets.length} targets` +
(comboErrors.length > 0
? ` | tried: ${summary}${comboErrors.length > 5 ? `... (+${comboErrors.length - 5})` : ""}`
: "");
const latencyMs = Date.now() - startTime;
if (recordedAttempts === 0) {
recordComboRequest(combo.name, null, {
success: false,
latencyMs,
fallbackCount,
strategy,
});
}
notifyWebhookEvent("request.failed", {
combo: combo.name,
reason: "COMBO_TIMEOUT",
latencyMs,
fallbackCount,
});
return errorResponseWithComboDiagnostics(
504,
msg,
buildComboDiag("combo_timeout"),
{ code: "COMBO_TIMEOUT", type: "server_error" }
);
}
// All models failed in this set try
const latencyMs = Date.now() - startTime;
if (recordedAttempts === 0) {
@@ -2492,31 +2553,61 @@ export async function handleComboChat({
}
const status = lastStatus;
const msg = lastError || "All combo models unavailable";
// Build aggregated error message with per-model failure details for diagnostics.
const comboErrorSummary =
comboErrors.length > 0
? " [" +
comboErrors
.slice(0, 5)
.map((e) => `${e.model} (${e.status})`)
.join(", ") +
(comboErrors.length > 5 ? `... (+${comboErrors.length - 5})` : "") +
"]"
: "";
const msg = (lastError || "All combo models unavailable") + comboErrorSummary;
if (earliestRetryAfter) {
// Quota-share cooldown-aware retry: instead of crystallizing the 429,
// 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.
// 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) {
const decision = resolveComboCooldownWaitDecision({
// 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,
lookupLock: (provider, connectionId) => {
const rawModel = parseModel(orderedTargets[0]?.modelStr ?? "").model || "";
// 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",
`Quota-share cooldown wait: ${msg} — waiting ${Math.ceil(
`${strategy} cooldown wait: ${msg} — waiting ${Math.ceil(
decision.waitMs / 1000
)}s (reason=${decision.reason ?? "?"}) then retrying (attempt ${
comboCooldownAttempt + 1
@@ -2524,7 +2615,7 @@ export async function handleComboChat({
);
const completed = await waitForCooldownAwareRetry(decision.waitMs, signal);
if (!completed) {
log.info("COMBO", "Quota-share cooldown wait aborted by client disconnect");
log.info("COMBO", `${strategy} cooldown wait aborted by client disconnect`);
return errorResponse(499, "Request aborted");
}
comboCooldownAttempt += 1;

View File

@@ -419,6 +419,15 @@ export async function expandAutoComboCandidatePool(
if (Array.isArray(localAutoConfig?.candidatePool) && localAutoConfig.candidatePool.length > 0)
return eligibleTargets;
// #COMBO-REF: if the combo references other combos via kind:"combo-ref" entries,
// the resolved eligibleTargets already represent the operator's intended pool.
// Expanding to ALL providers would defeat the purpose of the combo-ref constraint
// (e.g. an "auto" combo delegating to a "priority" sub-combo should not pull in
// every model from every active provider).
const rawModels = (combo as Record<string, unknown> | null | undefined)?.models;
if (Array.isArray(rawModels) && rawModels.some((m) => isRecord(m) && m.kind === "combo-ref"))
return eligibleTargets;
try {
const allConnections = await getCachedProviderConnections({ isActive: true });
const providerIds = [

View File

@@ -139,16 +139,37 @@ export interface ComboCooldownLockInfo {
remainingMs: number;
}
/** Minimal combo-target shape this helper inspects. */
export interface ComboCooldownTarget {
provider?: string | null;
connectionId?: string | null;
/**
* The target's own model. Heterogeneous combos (priority, weighted,
* round-robin, …) hold a DIFFERENT model per target, so the lock lookup must
* be keyed on each target's own model — keying every lookup on the first
* target's model would miss every other target's lock and silently degrade
* the reason allow-list to "no lock found".
*/
modelStr?: string | null;
}
export interface ResolveComboCooldownDecisionInput {
/** Combo targets to inspect for an active short lock. */
targets: ReadonlyArray<{ provider?: string | null; connectionId?: string | null }>;
targets: ReadonlyArray<ComboCooldownTarget>;
/** Earliest retry-after the loop crystallized (string | number | Date | null). */
earliestRetryAfter: unknown;
attempt: number;
budgetLeftMs: number;
settings: ComboCooldownWaitSettings;
/** Per-target lock lookup (getModelLockoutInfo bound to the request model). */
lookupLock: (provider: string, connectionId: string) => ComboCooldownLockInfo | null;
/**
* Per-target lock lookup (getModelLockoutInfo). Receives the target itself so
* the caller can key the lookup on that target's own model.
*/
lookupLock: (
provider: string,
connectionId: string,
target: ComboCooldownTarget
) => ComboCooldownLockInfo | null;
/** Derives the wait (ms) from the retry-after hint (computeClosestRetryAfter). */
computeWaitMs: (retryAfter: unknown) => number | null;
}
@@ -199,7 +220,7 @@ export function resolveComboCooldownWaitDecision(
const provider = typeof target.provider === "string" ? target.provider : "";
if (!provider) continue;
const connectionId = typeof target.connectionId === "string" ? target.connectionId : "";
const info = lookupLock(provider, connectionId);
const info = lookupLock(provider, connectionId, target);
if (!info) continue;
const remainingMs =
typeof info.remainingMs === "number" && Number.isFinite(info.remainingMs)

View File

@@ -37,6 +37,7 @@ export interface ComboSetup {
clientRequestedStream: boolean;
config: ReturnType<typeof resolveComboSetupConfig>;
comboTargetTimeoutMs: number;
comboTimeoutMs: number;
reasoningTokenBufferEnabled: boolean;
}
@@ -116,6 +117,7 @@ export function phaseComboSetup(ctx: ComboContext): ComboSetup {
FETCH_TIMEOUT_MS,
DEFAULT_COMBO_TARGET_TIMEOUT_MS
);
const comboTimeoutMs = config.comboTimeoutMs || 0;
const reasoningTokenBufferEnabled = config.reasoningTokenBufferEnabled !== false;
return {
@@ -128,6 +130,7 @@ export function phaseComboSetup(ctx: ComboContext): ComboSetup {
clientRequestedStream,
config,
comboTargetTimeoutMs,
comboTimeoutMs,
reasoningTokenBufferEnabled,
};
}

View File

@@ -88,6 +88,12 @@ const DEFAULT_COMBO_CONFIG = {
pipeline_fallback: "single-provider",
resetAwareQuotaCacheTtlMs: 0,
resetAwareQuotaCacheMaxStaleMs: 0,
// Global combo timeout (0 = disabled). When set, limits the total wall-clock time
// the combo spends iterating through targets. After each target completes, if the
// elapsed time exceeds comboTimeoutMs, remaining targets are skipped and a 504 with
// aggregated error diagnostics is returned. Backward-compatible: 0 preserves the
// legacy unlimited-iteration behavior.
comboTimeoutMs: 0,
shadowRouting: {
enabled: false,
targets: [],

View File

@@ -115,6 +115,67 @@ test("expandAutoComboCandidatePool falls through to active connections when cand
assert.ok(openaiTargets.length > 0, "expected openai targets to be expanded");
});
test("expandAutoComboCandidatePool is a no-op when the combo references other combos via kind:\"combo-ref\" entries", async () => {
await providersDb.createProviderConnection({
provider: "openai",
authType: "apikey",
name: "OpenAI",
apiKey: "sk-test-openai",
defaultModel: "gpt-4o-mini",
});
const seed = [
{
kind: "model" as const,
stepId: "anthropic/claude-3-5-sonnet",
executionKey: "anthropic/claude-3-5-sonnet",
modelStr: "anthropic/claude-3-5-sonnet",
provider: "anthropic",
providerId: "anthropic",
connectionId: null,
weight: 1,
label: null,
},
];
// An "auto" combo delegating to a "priority" sub-combo via a combo-ref entry:
// expanding to every model of every active provider (openai included) would
// defeat the point of the combo-ref constraint, so the resolved
// eligibleTargets must be returned unchanged (#COMBO-REF).
const result = await combo.expandAutoComboCandidatePool(seed, {
config: {},
models: [{ kind: "combo-ref", ref: "priority-subcombo" }],
});
assert.equal(result.length, 1, "combo-ref guard must prevent provider-wide expansion");
assert.equal(result[0].modelStr, "anthropic/claude-3-5-sonnet");
assert.ok(
!result.some((t) => t.provider === "openai"),
"no openai targets should have been pulled in despite an active openai connection"
);
});
test("expandAutoComboCandidatePool still expands normally when models has no combo-ref entries", async () => {
await providersDb.createProviderConnection({
provider: "openai",
authType: "apikey",
name: "OpenAI",
apiKey: "sk-test-openai",
defaultModel: "gpt-4o-mini",
});
const result = await combo.expandAutoComboCandidatePool([], {
config: {},
models: ["openai/gpt-4o-mini"],
});
const openaiTargets = result.filter((t) => t.provider === "openai");
assert.ok(
openaiTargets.length > 0,
"plain model-string entries (no combo-ref) must not trip the guard"
);
});
test("expandAutoComboCandidatePool does not duplicate an already-present modelStr", async () => {
await providersDb.createProviderConnection({
provider: "openai",

View File

@@ -842,7 +842,7 @@ test("handleComboChat records per-target metrics separately when the same model
assert.equal(metrics.byTarget[secondStep.id].connectionId, "conn-openai-b");
});
test("handleComboChat preserves the first failure status but surfaces the last error message", async () => {
test("handleComboChat preserves the first failure status but surfaces the last error message plus per-model diagnostics", async () => {
const result = await handleComboChat({
body: {},
combo: {
@@ -864,7 +864,75 @@ test("handleComboChat preserves the first failure status but surfaces the last e
const payload = (await result.json()) as any;
assert.equal(result.status, 500);
assert.equal(payload.error.message, "fail:model-b");
// The last error message is preserved and now carries an aggregated
// per-model diagnostics suffix (status codes for every target attempted
// in this set try), added alongside the global comboTimeoutMs feature.
assert.equal(payload.error.message, "fail:model-b [model-a (500), model-b (429)]");
});
interface ComboErrorPayload {
error: { code?: string; message: string };
}
test("handleComboChat global comboTimeoutMs stops iterating remaining targets and returns 504 with aggregated diagnostics", async () => {
const calls: string[] = [];
const result = await handleComboChat({
body: {},
combo: {
name: "timeout-combo",
strategy: "priority",
models: ["model-a", "model-b", "model-c"],
// An effectively-zero ceiling: the FIRST target still dispatches (the
// check only runs after a target completes), but by the time it
// resolves any nonzero elapsed time trips the timeout, so the loop
// must stop instead of trying model-b/model-c.
config: { maxRetries: 0, comboTimeoutMs: 1 },
},
handleSingleModel: async (_body: unknown, modelStr: string) => {
calls.push(modelStr);
// A tiny real delay guarantees Date.now() advances past the 1ms ceiling
// before the post-target timeout check runs.
await new Promise((resolve) => setTimeout(resolve, 5));
return errorResponse(500, `fail:${modelStr}`);
},
isModelAvailable: async () => true,
log: createLog(),
settings: null,
allCombos: null,
});
const payload = (await result.json()) as ComboErrorPayload;
assert.equal(result.status, 504);
assert.equal(payload.error.code, "COMBO_TIMEOUT");
assert.match(payload.error.message, /Combo global timeout \(1ms\)/);
assert.match(payload.error.message, /model-a \(500\)/);
assert.deepEqual(calls, ["model-a"], "remaining targets must be skipped once the timeout trips");
});
test("handleComboChat comboTimeoutMs=0 (default) never trips the global timeout, all targets still tried", async () => {
const calls: string[] = [];
const result = await handleComboChat({
body: {},
combo: {
name: "no-timeout-combo",
strategy: "priority",
models: ["model-a", "model-b"],
config: { maxRetries: 0 }, // comboTimeoutMs defaults to 0 (disabled)
},
handleSingleModel: async (_body: unknown, modelStr: string) => {
calls.push(modelStr);
if (modelStr === "model-a") return errorResponse(500, "fail:model-a");
return okResponse();
},
isModelAvailable: async () => true,
log: createLog(),
settings: null,
allCombos: null,
});
assert.equal(result.status, 200);
assert.deepEqual(calls, ["model-a", "model-b"]);
});
test("handleComboChat round-robin rotates sequentially across requests", async () => {

View File

@@ -3,18 +3,29 @@
*
* Extracted from tests/unit/combo-quota-share-cooldown-wait.test.ts (#6803).
*
* These two scenarios assert a wall-clock ceiling (`elapsed < 1500`) around a
* handleComboChat() call that also performs real SQLite I/O (test.beforeEach
* does fs.rmSync+fs.mkdirSync + core.resetDbInstance()). Under CI-runner
* CPU/IO contention (multiple concurrent sibling shard jobs) this ceiling can
* be exceeded even though the functional behavior (no wait, single dispatch)
* is correct — this is a "did NOT wait out a cooldown" ceiling, not a
* behavior-under-test assertion, so it is timing-sensitive by nature.
* The quota_exhausted scenario below asserts a wall-clock ceiling
* (`elapsed < 10000`) around a handleComboChat() call that also performs real
* SQLite I/O (test.beforeEach does fs.rmSync+fs.mkdirSync +
* core.resetDbInstance()). Under CI-runner CPU/IO contention (multiple
* concurrent sibling shard jobs) this ceiling can be exceeded even though the
* functional behavior (no wait, single dispatch) is correct — this is a "did
* NOT wait out a cooldown" ceiling, not a behavior-under-test assertion, so it
* is timing-sensitive by nature.
*
* Running these in tests/unit/serial/ (--test-concurrency=1, see
* package.json's test:unit:serial) removes the intra-suite parallelism that
* was the dominant source of contention, matching the repo's established
* remedy pattern for this class of test.
*
* 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
* 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.
*/
import test from "node:test";
import assert from "node:assert/strict";
@@ -57,13 +68,17 @@ function jsonResponse(status: number, body: Record<string, unknown>) {
});
}
function rateLimitResponse(status: number) {
function rateLimitResponse(status: number, retryAfterMs: number = RETRY_AFTER_MS) {
return jsonResponse(status, {
error: { message: `rate limited (${status})` },
retryAfter: new Date(Date.now() + RETRY_AFTER_MS).toISOString(),
retryAfter: new Date(Date.now() + retryAfterMs).toISOString(),
});
}
function okResponse() {
return jsonResponse(200, { id: "ok", choices: [{ message: { content: "recovered" } }] });
}
function comboOf(strategy: string) {
return {
name: `qtSd/${strategy}-${Math.random().toString(16).slice(2, 8)}`,
@@ -127,11 +142,17 @@ test("quota-share: 403 quota_exhausted → NO wait, error propagated immediately
assert.ok(elapsed < 10000, `quota_exhausted must not wait out a cooldown, but ${elapsed}ms elapsed`);
});
test("non quota-share (priority): 429 propagated immediately, NO wait", async () => {
test("non quota-share (priority): short 429 cooldown → waits and re-dispatches (2nd pass 200)", async () => {
let calls = 0;
const handleSingleModel = async () => {
calls += 1;
return rateLimitResponse(429);
// 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).
return calls === 1 ? rateLimitResponse(429) : okResponse();
};
const startedAt = Date.now();
@@ -146,8 +167,93 @@ test("non quota-share (priority): 429 propagated immediately, NO wait", async ()
});
const elapsed = Date.now() - startedAt;
assert.equal(res.status, 429, "priority combo must propagate the 429 unchanged");
assert.equal(calls, 1, "priority combo must NOT wait+redispatch");
// Widened from 1500ms (#6803) — see the sibling test above for rationale.
assert.ok(elapsed < 10000, `priority combo must not wait out a cooldown, but ${elapsed}ms elapsed`);
assert.equal(res.status, 200, "expected the retried dispatch to succeed with 200");
assert.equal(calls, 2, "expected exactly one wait+redispatch (2 upstream calls)");
assert.ok(
elapsed >= RETRY_AFTER_MS - 50,
`expected to have waited out the cooldown, only ${elapsed}ms elapsed`
);
});
test("non quota-share (priority): a quota_exhausted lock drives the decision with a SHORT wait → NO wait (the reason allow-list is the PRIMARY barrier; the maxWaitMs ceiling does NOT cover this)", async () => {
// THE regression guard for the two-barrier policy documented in
// comboCooldownRetry.ts ("SECURITY — quota_exhausted must be excluded" /
// "The small maxWaitMs ceiling is the second barrier").
//
// 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.
// - 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.
const calls: string[] = [];
const handleSingleModel = async (_body: unknown, modelStr: string) => {
calls.push(modelStr);
return modelStr === "openai/gpt-4" ? rateLimitResponse(429) : rateLimitResponse(403);
};
const res = await handleComboChat({
body: { model: "openai/gpt-4" },
combo: {
name: "priority-quota-exhausted-short-wait",
strategy: "priority",
models: ["openai/gpt-4", "anthropic/claude-3-5-sonnet"],
config: { maxRetries: 0, retryDelayMs: 0, fallbackDelayMs: 0, maxSetRetries: 0 },
},
handleSingleModel,
isModelAvailable: async () => true,
log: createLog() as never,
settings: {
modelLockout: {
...shortModelLockoutSettings().modelLockout,
// 429 deliberately excluded: only the 403 records a lock, so the
// quota_exhausted reason is unambiguously the one under test.
errorCodes: [403],
},
},
allCombos: null,
});
assert.equal(res.status, 429, "the crystallized 429 must be propagated, not retried");
// 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
// produces 6 dispatches, not 2.
assert.deepEqual(
calls,
["openai/gpt-4", "anthropic/claude-3-5-sonnet"],
"a quota_exhausted lock must NOT trigger a wait+redispatch, even when the wait would be short enough to clear the maxWaitMs ceiling"
);
});
test("non quota-share (priority) with comboCooldownWait disabled → 429 propagated, NO wait", async () => {
let calls = 0;
const handleSingleModel = async () => {
calls += 1;
return rateLimitResponse(429);
};
const res = await handleComboChat({
body: { model: "openai/gpt-4" },
combo: { ...comboOf("priority"), name: "priority-combo-disabled" },
handleSingleModel,
isModelAvailable: async () => true,
log: createLog() as never,
settings: {
...shortModelLockoutSettings(),
resilienceSettings: { comboCooldownWait: { enabled: false } },
},
allCombos: null,
});
assert.equal(res.status, 429, "disabled feature must propagate the 429 unchanged");
assert.equal(calls, 1, "disabled feature must NOT wait+redispatch");
});