fix(combo): stop retrying an unrecoverable request-shape error across the whole fallback chain (#13338)

Merged. Retrying an unrecoverable request-shape error across the whole fallback chain burns every target on an error that will never change — the caller waits longer for the same failure. Stopping at the first one is right.

Validated as a combined board first (this PR merged with the 21 siblings of the same wave on the release tip): eslint with the frozen suppressions, typecheck:core, check:open-sse-typecheck, complexity, cognitive-complexity, changelog-integrity, i18n new-key coverage, docs-counts, docs-sync, migration-numbering, provider-consistency and a duplicate-identifier audit all green, plus 176 passing / 0 failing focused node:test cases across the 25 test files the wave touches and the dashboard test under Vitest (2/0). Then re-validated alone on the fresh tip before this merge: conflicts re-resolved, file sizes rebaselined for this PR's own growth, eslint and this PR's focused tests re-run.

Thank you.

changelog.d/fixes/13338-combo-identical-error-circuit-breaker.md

@@ -0,0 +1 @@
+- **fix(combo):** stop retrying a malformed-request-shape error across the entire fallback chain — a 400/422 that fails one target for a `kind: "model"` reason (exact status + error message match) will fail identically on every other target too, so a bad payload previously burned the full `MAX_GLOBAL_ATTEMPTS` budget instead of failing fast. Observed live: 41-44 identical combo decisions over 13+ minutes for a single request. Trips only on 3 consecutive identical `kind: "model"` failures, leaving transient and provider-side errors untouched ([#13338](https://github.com/diegosouzapw/OmniRoute/pull/13338)).

open-sse/services/combo/comboAttemptLoop.ts

@@ -44,6 +44,8 @@ import { withQuotaExhaustionClassification } from "./quotaExhaustion.ts";
 import {
   COMBO_LOOP_SAFETY_TIMEOUT_MS,
   COMBO_SAFETY_DRAIN_MS,
+  IDENTICAL_MODEL_ERROR_STREAK,
+  hasIdenticalModelErrorStreak,
   resolveDelayMs,
 } from "./comboPredicates.ts";
 import { evaluateExecuteTargetGates } from "./executeTargetGates.ts";
@@ -190,6 +192,12 @@ export async function dispatchWithCooldownRetry(opts: {
       });
       const runningTasks = new Set<Promise<void>>();
       let anySuccess = false;
+      // Flipped once the last IDENTICAL_MODEL_ERROR_STREAK targets have all
+      // failed with the exact same request-shape error — see comboPredicates.ts.
+      // Stops this set-try's target loop early AND skips the whole-set retry
+      // below, since a malformed request fails identically no matter how many
+      // more times it's replayed against the remaining fallbacks.
+      let comboRequestMalformed = false;
       // #10681: steps already recorded as dispatched (so per-target retries do not
       // duplicate the decision).
       state.dispatchedTargets = new Set<string>();
@@ -223,7 +231,7 @@ export async function dispatchWithCooldownRetry(opts: {
       };
 
       for (let i = 0; i < state.orderedTargets.length; i++) {
-        if (anySuccess || state.comboExpired) break;
+        if (anySuccess || state.comboExpired || comboRequestMalformed) break;
 
         const abortController = new AbortController();
         state.abortControllers.set(i, abortController);
@@ -296,6 +304,18 @@ export async function dispatchWithCooldownRetry(opts: {
               `${i + 1}/${state.orderedTargets.length} targets (${state.recordedAttempts} attempted) — stopping`
           );
         }
+
+        if (!anySuccess && !state.comboExpired && hasIdenticalModelErrorStreak(state.comboErrors)) {
+          comboRequestMalformed = true;
+          const last = state.comboErrors[state.comboErrors.length - 1];
+          deps.log.warn(
+            "COMBO",
+            `The last ${IDENTICAL_MODEL_ERROR_STREAK} targets all failed with the identical ` +
+              `request-shape error (status ${last.status}) after ${i + 1}/${state.orderedTargets.length} ` +
+              `targets (${state.recordedAttempts} attempted) — stopping instead of retrying the same ` +
+              `malformed request against remaining fallbacks or set-retries`
+          );
+        }
       }
 
       if (!anySuccess && runningTasks.size > 0) {
@@ -386,8 +406,10 @@ export async function dispatchWithCooldownRetry(opts: {
         });
       }
 
-      // Retry the entire set if more attempts remain
-      if (setTry < extra.maxSetRetries) continue;
+      // Retry the entire set if more attempts remain -- unless the identical-
+      // error streak already proved the request itself is malformed, in which
+      // case a fresh set-try would just reproduce the same streak.
+      if (setTry < extra.maxSetRetries && !comboRequestMalformed) continue;
 
       if (!state.lastStatus && state.recordedAttempts === 0 && extra.comboCooldownWaitEnabled) {
         const circuitOpenWait = resolveCircuitOpenWaitDecision({

open-sse/services/combo/comboPredicates.ts

@@ -24,6 +24,7 @@ import {
 import { isResourceNotFoundResponse } from "../errorClassifier.ts";
 import { getTrustedLocalRateLimitResponse } from "../rateLimitManager/errors.ts";
 import type { ResolvedComboTarget } from "./types.ts";
+import type { ComboErrorEntry } from "./comboErrorAggregation.ts";
 
 // Status codes that should mark round-robin target semaphores as cooling down.
 export const TRANSIENT_FOR_SEMAPHORE = [429, 502, 503, 504];
@@ -111,6 +112,29 @@ export const MAX_GLOBAL_ATTEMPTS = 30;
 // background-request DoS risk that motivated MAX_COMBO_DEPTH_HARD_CAP.
 export const MAX_GLOBAL_ATTEMPTS_HARD_CAP = 200;
 
+// A malformed/unsupported request shape (e.g. an incompatible tool-call
+// history for a provider's translation layer) fails the SAME way against
+// every fallback target, since it's a property of the request, not of any
+// one provider. Once this many *consecutive* targets have failed with the
+// identical model-shape error (same kind, status, and message), retrying the
+// remaining fallbacks — or the whole set again — cannot succeed either; it
+// only burns MAX_GLOBAL_ATTEMPTS and wall-clock time. See combo.ts's
+// `comboRequestMalformed` handling.
+export const IDENTICAL_MODEL_ERROR_STREAK = 3;
+
+export function hasIdenticalModelErrorStreak(
+  comboErrors: ReadonlyArray<ComboErrorEntry>,
+  streak: number = IDENTICAL_MODEL_ERROR_STREAK
+): boolean {
+  if (comboErrors.length < streak) return false;
+  const tail = comboErrors.slice(-streak);
+  const [first, ...rest] = tail;
+  if (first.kind !== "model") return false;
+  return rest.every(
+    (e) => e.kind === first.kind && e.status === first.status && e.error === first.error
+  );
+}
+
 /**
  * Clamp an operator-configured combo nesting depth (config.maxComboDepth) to a
  * safe integer in [1, MAX_COMBO_DEPTH_HARD_CAP]. Anything non-numeric, < 1, or

tests/unit/combo-identical-error-streak.test.ts

@@ -0,0 +1,83 @@
+// Regression coverage for the identical-request-shape-error circuit breaker
+// (combo.ts / comboPredicates.ts): a corrupted conversation history that one
+// provider's protocol validation rejects with a 4xx "kind: model" error is
+// certain to be rejected identically by every remaining fallback target,
+// since the malformed payload itself — not the provider — is at fault.
+// Without this breaker, production observed a single malformed request
+// burning through all MAX_GLOBAL_ATTEMPTS (30) targets identically, taking
+// 13+ minutes before the combo happened to reach a lenient-enough fallback
+// or gave up.
+import { test } from "node:test";
+import assert from "node:assert/strict";
+
+const { hasIdenticalModelErrorStreak, IDENTICAL_MODEL_ERROR_STREAK } = await import(
+  "../../open-sse/services/combo/comboPredicates.ts"
+);
+
+function entry(overrides: Partial<{ status: number; error: string; kind: string }> = {}) {
+  return {
+    model: "gemini/gemini-3.1-flash-lite",
+    status: 400,
+    error: "[400]: Please ensure that function call turn comes immediately after a user turn or after a function response turn.",
+    kind: "model",
+    ...overrides,
+  } as never;
+}
+
+test("false when fewer than the streak threshold have been attempted", () => {
+  assert.equal(hasIdenticalModelErrorStreak([]), false);
+  assert.equal(hasIdenticalModelErrorStreak([entry()]), false);
+  assert.equal(
+    hasIdenticalModelErrorStreak(Array.from({ length: IDENTICAL_MODEL_ERROR_STREAK - 1 }, () => entry())),
+    false
+  );
+});
+
+test("true once the last IDENTICAL_MODEL_ERROR_STREAK targets share status+error+kind", () => {
+  const errors = Array.from({ length: IDENTICAL_MODEL_ERROR_STREAK }, () => entry());
+  assert.equal(hasIdenticalModelErrorStreak(errors), true);
+});
+
+test("stays true once the streak is reached even with more matching entries after it", () => {
+  const errors = Array.from({ length: IDENTICAL_MODEL_ERROR_STREAK + 2 }, () => entry());
+  assert.equal(hasIdenticalModelErrorStreak(errors), true);
+});
+
+test("false when the tail is not all the same kind (a transient provider failure mixed in)", () => {
+  const errors = [
+    entry(),
+    entry({ kind: "provider", status: 503, error: "[503]: upstream unavailable" }),
+    entry(),
+  ];
+  assert.equal(hasIdenticalModelErrorStreak(errors), false);
+});
+
+test("false when the tail is not kind 'model' at all (e.g. every target rate-limited)", () => {
+  const errors = Array.from({ length: IDENTICAL_MODEL_ERROR_STREAK }, () =>
+    entry({ kind: "rate_limit", status: 429, error: "[429]: Provider returned error" })
+  );
+  assert.equal(hasIdenticalModelErrorStreak(errors), false);
+});
+
+test("false when the same status/kind repeats but with a genuinely different error message", () => {
+  const errors = [
+    entry({ error: "[400]: missing required field 'model'" }),
+    entry({ error: "[400]: unknown tool 'foo'" }),
+    entry({ error: "[400]: Please ensure that function call turn comes immediately after a user turn or after a function response turn." }),
+  ];
+  assert.equal(hasIdenticalModelErrorStreak(errors), false);
+});
+
+test("true even when an earlier, unrelated failure precedes the matching streak", () => {
+  const errors = [
+    entry({ kind: "auth", status: 401, error: "[401]: invalid api key" }),
+    ...Array.from({ length: IDENTICAL_MODEL_ERROR_STREAK }, () => entry()),
+  ];
+  assert.equal(hasIdenticalModelErrorStreak(errors), true);
+});
+
+test("a custom streak length is honored", () => {
+  const errors = Array.from({ length: 2 }, () => entry());
+  assert.equal(hasIdenticalModelErrorStreak(errors, 2), true);
+  assert.equal(hasIdenticalModelErrorStreak(errors, 3), false);
+});