fix(resilience): scope the circuit breaker to one connection (#14530)

Merged. Thank you, @RaviTharuma — and thank you for crediting whose design this follows.

The principle is right and overdue: a circuit-breaker trip belongs to the **connection** that failed. One account returning errors should not open `getCircuitBreaker(provider)` for every other healthy account on that provider — that turns a single bad key into a provider-wide outage. Keeping the provider-wide breaker for the case that genuinely is provider-wide (a dead proxy) is the right line to draw. Pulling `MODEL_ACCESS_DENIED_PATTERNS` / `isModelScoped400` into `open-sse/services/modelAccessDenied.ts` so combo predicates and account fallback quote one module is a good consolidation too.

**One real defect had to be fixed before this could land — the PR did not compile.** In `evaluateExecuteTargetGates`, the connection breaker was looked up with `connectionId`:

```ts
const scopedConnectionId = target.connectionId ?? undefined;
const connectionBreaker = scopedConnectionId
  ? getCircuitBreaker(connectionCircuitBreakerName(provider, connectionId))   // ← here
  : null;
```

The function declares `connectionId` again ~245 lines further down, in the credential-gate block, so this reference sat in that `const`'s temporal dead zone (TS2448) and carried the unnarrowed `string | undefined` into a `string` parameter (TS2345). Now it passes `scopedConnectionId` — the same expression, already narrowed by the ternary guard, and the same id the OPEN log line six lines below already reports, so lookup and message now name the same connection. This is the value you meant, not just the one that compiles.

It survived because `tests/unit/connection-circuit-breaker.test.ts` exercises `recordProviderFailure` / `isProviderInCooldown` / `isModelScoped400` but never reaches `evaluateExecuteTargetGates`. Two tests were added to the existing `tests/unit/combo/execute-target-gates.test.ts` harness: an OPEN **connection** breaker skips only that connection (mutation-verified — swapping the lookup back to `getCircuitBreaker(provider)` makes it fail, so it pins the breaker *name*), and a sibling connection on the same provider still proceeds.

Validation before merge, on the real merge tree (current tip + this head): `typecheck:core` no longer reports either `executeTargetGates.ts` error; `connection-circuit-breaker` + `provider-breaker-halfopen-recovery` + `execute-target-gates` **25 pass / 0 fail**; `check:file-size` and `check:changelog-integrity` OK. Two files you left over Prettier's 100-char width were formatted in a separate style-only commit.

`check:open-sse-typecheck` still reports 8 errors in `open-sse/executors/auggie.ts`. Those are pre-existing on the tip (#14496) and this PR does not touch that file.

Co-authored-by: Bob.Hou <19586012+HouMinXi@users.noreply.github.com>
Co-authored-by: Koosha Paridehpour <42529354+KooshaPari@users.noreply.github.com>

changelog.d/features/14530-connection-breaker-model-scope.md

@@ -0,0 +1 @@
+- **fix(resilience):** follow HouMinXi's per-connection circuit breaker, so one account's failure no longer opens the breaker for every account on that provider. Proxy failures stay provider-wide. Model-scoped 400 patterns now live in one module, which is KooshaPari's #8251 plan, shared by combo dispatch and account fallback ([#14530](https://github.com/diegosouzapw/OmniRoute/pull/14530)) — thanks @HouMinXi @KooshaPari

open-sse/services/accountFallback.ts

@@ -36,6 +36,11 @@ import {
   getAllCircuitBreakerStatuses,
   getCircuitBreaker,
 } from "../../src/shared/utils/circuitBreaker";
+import { MODEL_ACCESS_DENIED_PATTERNS, isModelScoped400 } from "./modelAccessDenied.ts";
+import {
+  connectionCircuitBreakerName,
+  failureCircuitBreakerName,
+} from "./connectionCircuitBreaker.ts";
 import {
   classify429FromError,
   looksLikeQuotaExhausted,
@@ -371,30 +376,9 @@ const MODEL_ACCESS_AMBIGUOUS_TYPES = new Set([
   "permission_error", // Anthropic: could be model access OR key/org/feature scope
 ]);
 
-// Model access patterns — the account does not have access to the requested model
-// but a different account (e.g. PRO vs free tier) may support it.
-// Exported so combo.ts #2101 can exempt model-scoped 400s from the body-specific
-// stop guard (#5249): "model not supported" must advance to the next combo target
-// even when the message also contains wrapper words like "invalid" / "bad request".
-export const MODEL_ACCESS_DENIED_PATTERNS = [
-  /\binvalid model\b/i,
-  /\bmodel.*not.*(?:available|found|supported|accessible)\b/i,
-  /\bmodel.*(?:does not exist|doesn't exist)\b/i,
-  // "does not support" / "unsupported model" — GitHub Copilot / OpenAI-compatible
-  // often phrase model rejection this way without the "is not supported" word order.
-  /\bmodel\b[\s\S]{0,80}?\b(?:does\s+not\s+support|doesn't\s+support|unsupported)\b/i,
-  /\b(?:does\s+not\s+support|doesn't\s+support|unsupported)\b[\s\S]{0,80}?\bmodel\b/i,
-  /\bunsupported\s+model\b/i,
-  /\baccess.*denied.*model\b/i,
-  /\bmodel.*access.*denied\b/i,
-  /\bplease select a different model\b/i,
-  /\bunknown\s+provider\s+for\s+model\b/i,
-  // "...access to the requested model" / "model ... access" — bounded lookahead
-  // (no nested quantifiers) so it stays ReDoS-safe while requiring BOTH an
-  // access/permission word and "model" so a pure auth error never matches.
-  /\b(?:access|permission)[\s\S]{0,60}?\bmodel\b/i,
-  /\bmodel[\s\S]{0,60}?\b(?:access|permission)\b/i,
-];
+// KooshaPari (#8251): the pattern list lives in modelAccessDenied.ts.
+// Re-exported so existing accountFallback imports keep working.
+export { MODEL_ACCESS_DENIED_PATTERNS, isModelScoped400 };
 
 // Pure credential/authentication failures — the key or token itself is bad, which
 // is NOT a model-availability problem. Some providers phrase these as a 400 that
@@ -1156,7 +1140,8 @@ function getProviderBreaker(provider: string | null | undefined) {
 
 function configureProviderBreaker(
   provider: string | null | undefined,
-  profile?: ProviderBreakerProfile | null
+  profile?: ProviderBreakerProfile | null,
+  breakerName?: string
 ) {
   if (!provider) return null;
 
@@ -1166,7 +1151,7 @@ function configureProviderBreaker(
   // Stored value type is `boolean | undefined` — never `null` after PATCH.
   const userValue = resolvedProfile.useUpstream429BreakerHints;
   const useHints = resolveUseUpstream429BreakerHints(provider, userValue);
-  return getCircuitBreaker(provider, {
+  return getCircuitBreaker(breakerName || provider, {
     failureThreshold: resolvedProfile.failureThreshold ?? resolvedProfile.circuitBreakerThreshold,
     resetTimeout: resolvedProfile.resetTimeoutMs ?? resolvedProfile.circuitBreakerReset,
     ...(useHints
@@ -1187,9 +1172,15 @@ function configureProviderBreaker(
 /**
  * Check if a provider is currently blocked by the shared circuit breaker.
  */
-export function isProviderInCooldown(provider: string | null | undefined): boolean {
-  const breaker = getProviderBreaker(provider);
-  return breaker ? !breaker.canExecute() : false;
+export function isProviderInCooldown(
+  provider: string | null | undefined,
+  connectionId?: string | null
+): boolean {
+  if (!provider) return false;
+  const providerBreaker = getProviderBreaker(provider);
+  if (providerBreaker && !providerBreaker.canExecute()) return true;
+  if (!connectionId) return false;
+  return !getCircuitBreaker(connectionCircuitBreakerName(provider, connectionId)).canExecute();
 }
 
 /**
@@ -1259,7 +1250,11 @@ export function recordProviderFailure(
     pruneConnectionFailureDedupeEntries();
   }
 
-  const breaker = configureProviderBreaker(provider, profile);
+  const breaker = configureProviderBreaker(
+    provider,
+    profile,
+    failureCircuitBreakerName(provider, connectionId, opts?.isNetworkError)
+  );
   if (!breaker) return;
 
   if (!breaker.canExecute()) return;
@@ -1288,7 +1283,9 @@ export function recordProviderSuccess(
 ): void {
   if (!provider || provider === "unknown") return;
 
-  const breaker = getProviderBreaker(provider);
+  const breaker = connectionId
+    ? getCircuitBreaker(connectionCircuitBreakerName(provider, connectionId))
+    : getProviderBreaker(provider);
   if (!breaker) return;
   const breakerState = breaker.getStatus().state;
 

open-sse/services/combo/comboPredicates.ts

@@ -16,16 +16,14 @@ import {
   isLocalExecutionError,
   isModelCapacityOverloadError,
 } from "@/shared/utils/circuitBreaker";
-import {
-  CONTEXT_OVERFLOW_PATTERNS,
-  MODEL_ACCESS_DENIED_PATTERNS,
-  cooldownUntilMs,
-} from "../accountFallback.ts";
+import { CONTEXT_OVERFLOW_PATTERNS, cooldownUntilMs } from "../accountFallback.ts";
 import { isResourceNotFoundResponse } from "../errorClassifier.ts";
 import { getTrustedLocalRateLimitResponse } from "../rateLimitManager/errors.ts";
 import type { ResolvedComboTarget } from "./types.ts";
 import type { ComboErrorEntry } from "./comboErrorAggregation.ts";
 
+export { isModelScoped400 } from "../modelAccessDenied.ts";
+
 // Status codes that should mark round-robin target semaphores as cooling down.
 export const TRANSIENT_FOR_SEMAPHORE = [429, 502, 503, 504];
 // G1 (silent-stop fix): hard ceiling for the combo target loop when the operator
@@ -651,18 +649,3 @@ export function isParamValidation400(errorText: string | null | undefined): bool
     /\bis illegal.*range\b/i.test(text)
   );
 }
-
-/**
- * #5249 / #2101: model-scoped 400s must NEVER stop the combo.
- */
-export function isModelScoped400(errorText: string | null | undefined): boolean {
-  const text = String(errorText || "");
-  if (!text) return false;
-  if (MODEL_ACCESS_DENIED_PATTERNS.some((p) => p.test(text))) return true;
-  return (
-    /\bmodel\b[\s\S]{0,80}?\b(?:not\s+supported|unsupported|unknown|unavailable)\b/i.test(text) ||
-    /\b(?:not\s+supported|unsupported|unknown)\b[\s\S]{0,80}?\bmodel\b/i.test(text) ||
-    /\bunsupported_api_for_model\b/i.test(text) ||
-    /\bdoes\s+not\s+support\s+(?:the\s+)?responses\s+api\b/i.test(text)
-  );
-}

open-sse/services/combo/executeTargetGates.ts

@@ -15,6 +15,7 @@ import { isProviderInCooldown } from "../providerCooldownTracker.ts";
 import { checkCredentialGate, logCredentialSkip } from "../credentialGate.ts";
 import { errorResponse } from "../../utils/error.ts";
 import { getCircuitBreaker } from "../../../src/shared/utils/circuitBreaker";
+import { connectionCircuitBreakerName } from "../connectionCircuitBreaker.ts";
 import { parseModel } from "../model.ts";
 import { canAffordRequest } from "../../../src/lib/quota/quotaScheduler.ts";
 import { getCachedProviderConnectionById } from "../../../src/lib/db/readCache.ts";
@@ -82,9 +83,16 @@ export async function evaluateExecuteTargetGates(opts: {
     if (i > 0) state.fallbackCount++;
   };
 
-  const cb = getCircuitBreaker(provider);
-  const cbStatus = cb.getStatus();
-  if (cbStatus.state === "OPEN") {
+  const providerBreaker = getCircuitBreaker(provider);
+  const scopedConnectionId = target.connectionId ?? undefined;
+  const connectionBreaker = scopedConnectionId
+    ? getCircuitBreaker(connectionCircuitBreakerName(provider, scopedConnectionId))
+    : null;
+  const providerOpen = providerBreaker.getStatus().state === "OPEN";
+  const connectionOpen = connectionBreaker?.getStatus().state === "OPEN";
+  if (providerOpen || connectionOpen) {
+    const cb = providerOpen ? providerBreaker : connectionBreaker!;
+    const cbStatus = cb.getStatus();
     state.skippedForCircuitOpen = true;
     if (
       cbStatus.retryAfterMs > 0 &&
@@ -93,7 +101,12 @@ export async function evaluateExecuteTargetGates(opts: {
     ) {
       state.earliestCircuitOpenRetryMs = cbStatus.retryAfterMs;
     }
-    deps.log.info("COMBO", `Skipping ${modelStr} — circuit breaker OPEN for ${provider}`);
+    deps.log.info(
+      "COMBO",
+      providerOpen
+        ? `Skipping ${modelStr} — circuit breaker OPEN for ${provider}`
+        : `Skipping ${modelStr} — circuit breaker OPEN for connection ${scopedConnectionId}`
+    );
     recordComboDecision(deps.traceInvocationId, {
       step: target.executionKey,
       target: modelStr,

open-sse/services/connectionCircuitBreaker.ts

@@ -0,0 +1,17 @@
+/**
+ * HouMinXi: one connection's failures must not open the breaker for every
+ * account on that provider. A dead proxy still uses the provider key, because
+ * it fails every account the same way.
+ */
+export function connectionCircuitBreakerName(provider: string, connectionId: string): string {
+  return `${provider}::conn::${connectionId}`;
+}
+
+export function failureCircuitBreakerName(
+  provider: string,
+  connectionId?: string | null,
+  isNetworkError?: boolean
+): string {
+  if (connectionId && !isNetworkError) return connectionCircuitBreakerName(provider, connectionId);
+  return provider;
+}

open-sse/services/modelAccessDenied.ts

@@ -0,0 +1,34 @@
+/**
+ * KooshaPari's plan on #8251: one shared source for model-scoped 400s, so
+ * combo dispatch and account fallback quote the same patterns.
+ * HouMinXi: a wrapped "invalid" / "Bad Request" that is really "this model is
+ * not on this account" advances. A different account may still serve it.
+ */
+
+export const MODEL_ACCESS_DENIED_PATTERNS = [
+  /\binvalid model\b/i,
+  /\bmodel.*not.*(?:available|found|supported|accessible)\b/i,
+  /\bmodel.*(?:does not exist|doesn't exist)\b/i,
+  /\bmodel\b[\s\S]{0,80}?\b(?:does\s+not\s+support|doesn't\s+support|unsupported)\b/i,
+  /\b(?:does\s+not\s+support|doesn't\s+support|unsupported)\b[\s\S]{0,80}?\bmodel\b/i,
+  /\bunsupported\s+model\b/i,
+  /\baccess.*denied.*model\b/i,
+  /\bmodel.*access.*denied\b/i,
+  /\bplease select a different model\b/i,
+  /\bunknown\s+provider\s+for\s+model\b/i,
+  /\b(?:access|permission)[\s\S]{0,60}?\bmodel\b/i,
+  /\bmodel[\s\S]{0,60}?\b(?:access|permission)\b/i,
+];
+
+/** #5249 / #2101: model-scoped 400s must not stop the combo. */
+export function isModelScoped400(errorText: string | null | undefined): boolean {
+  const text = String(errorText || "");
+  if (!text) return false;
+  if (MODEL_ACCESS_DENIED_PATTERNS.some((pattern) => pattern.test(text))) return true;
+  return (
+    /\bmodel\b[\s\S]{0,80}?\b(?:not\s+supported|unsupported|unknown|unavailable)\b/i.test(text) ||
+    /\b(?:not\s+supported|unsupported|unknown)\b[\s\S]{0,80}?\bmodel\b/i.test(text) ||
+    /\bunsupported_api_for_model\b/i.test(text) ||
+    /\bdoes\s+not\s+support\s+(?:the\s+)?responses\s+api\b/i.test(text)
+  );
+}

tests/unit/combo/execute-target-gates.test.ts

@@ -5,6 +5,7 @@
 import test from "node:test";
 import assert from "node:assert/strict";
 import { getCircuitBreaker, STATE } from "../../../src/shared/utils/circuitBreaker.ts";
+import { connectionCircuitBreakerName } from "../../../open-sse/services/connectionCircuitBreaker.ts";
 import type {
   AttemptLoopDeps,
   AttemptLoopState,
@@ -106,6 +107,60 @@ test("breaker OPEN skips and does not call handleSingleModel", async () => {
   assert.equal(state.skippedForCircuitOpen, true);
 });
 
+/**
+ * #14530 — the pre-dispatch circuit gate must look the connection breaker up
+ * under `connectionCircuitBreakerName(provider, target.connectionId)`, not
+ * under the bare provider name. Pins the breaker NAME, not just the skip:
+ * the provider breaker stays CLOSED in both tests, so a lookup that fell back
+ * to `getCircuitBreaker(provider)` would let the dead connection proceed, and
+ * a lookup that widened to the provider would wrongly skip its healthy sibling.
+ */
+test("#14530: an OPEN connection breaker skips only that connection", async () => {
+  const { evaluateExecuteTargetGates } =
+    await import("../../../open-sse/services/combo/executeTargetGates.ts");
+  const provider = `openai-gates-14530-${Date.now()}`;
+  const deadConnectionId = `conn-dead-${Date.now()}`;
+  const connectionBreaker = getCircuitBreaker(
+    connectionCircuitBreakerName(provider, deadConnectionId),
+    { failureThreshold: 1, resetTimeout: 60_000 }
+  );
+  connectionBreaker._onFailure("transient");
+  assert.equal(connectionBreaker.getStatus().state, STATE.OPEN);
+  // The provider-wide breaker must be untouched — that is the whole point.
+  assert.notEqual(getCircuitBreaker(provider).getStatus().state, STATE.OPEN);
+
+  const target = modelTarget({
+    provider,
+    modelStr: `${provider}/gpt-4o-mini`,
+    connectionId: deadConnectionId,
+  });
+  const state = emptyState({ orderedTargets: [target] });
+  const decision = await evaluateExecuteTargetGates({ index: 0, state, deps: baseDeps() });
+  assert.equal(decision.kind, "skip");
+  assert.equal(state.skippedForCircuitOpen, true);
+});
+
+test("#14530: a sibling connection on the same provider still proceeds", async () => {
+  const { evaluateExecuteTargetGates } =
+    await import("../../../open-sse/services/combo/executeTargetGates.ts");
+  const provider = `openai-gates-14530-sibling-${Date.now()}`;
+  const deadConnectionId = `conn-dead-${Date.now()}`;
+  getCircuitBreaker(connectionCircuitBreakerName(provider, deadConnectionId), {
+    failureThreshold: 1,
+    resetTimeout: 60_000,
+  })._onFailure("transient");
+
+  const healthy = modelTarget({
+    provider,
+    modelStr: `${provider}/gpt-4o-mini`,
+    connectionId: `conn-healthy-${Date.now()}`,
+  });
+  const state = emptyState({ orderedTargets: [healthy] });
+  const decision = await evaluateExecuteTargetGates({ index: 0, state, deps: baseDeps() });
+  assert.equal(decision.kind, "proceed");
+  assert.equal(state.skippedForCircuitOpen, false);
+});
+
 test("exhausted connection skip uses getExhaustedTargetSkipReason", async () => {
   const { evaluateExecuteTargetGates } =
     await import("../../../open-sse/services/combo/executeTargetGates.ts");

tests/unit/connection-circuit-breaker.test.ts

@@ -0,0 +1,39 @@
+import assert from "node:assert/strict";
+import test from "node:test";
+import { getCircuitBreaker } from "../../src/shared/utils/circuitBreaker.ts";
+import {
+  isProviderInCooldown,
+  recordProviderFailure,
+} from "../../open-sse/services/accountFallback.ts";
+import { isModelScoped400 } from "../../open-sse/services/modelAccessDenied.ts";
+import { connectionCircuitBreakerName } from "../../open-sse/services/connectionCircuitBreaker.ts";
+
+const lowThreshold = { failureThreshold: 1, resetTimeoutMs: 60_000 };
+
+test("one connection's failure does not open the breaker for another connection", () => {
+  const provider = `hou-conn-${Date.now()}`;
+  recordProviderFailure(provider, undefined, "account-a", lowThreshold);
+  assert.equal(isProviderInCooldown(provider, "account-a"), true);
+  assert.equal(isProviderInCooldown(provider, "account-b"), false);
+  assert.equal(isProviderInCooldown(provider), false);
+  assert.equal(
+    getCircuitBreaker(connectionCircuitBreakerName(provider, "account-a")).getStatus().state,
+    "OPEN"
+  );
+});
+
+test("a proxy failure still opens the provider-wide breaker", () => {
+  const provider = `hou-proxy-${Date.now()}`;
+  recordProviderFailure(provider, undefined, "account-a", lowThreshold, { isNetworkError: true });
+  assert.equal(isProviderInCooldown(provider), true);
+  assert.equal(isProviderInCooldown(provider, "account-b"), true);
+});
+
+test("Koosha's shared model-scope source still advances wrapped model 400s", () => {
+  assert.equal(
+    isModelScoped400("invalid_request_error: model claude-fable-5 is not supported"),
+    true
+  );
+  assert.equal(isModelScoped400("Bad Request: The model is not supported"), true);
+  assert.equal(isModelScoped400("Invalid message format: the request body is malformed."), false);
+});

tests/unit/provider-breaker-halfopen-recovery.test.ts

@@ -23,6 +23,7 @@ import {
   isProviderInCooldown,
   clearCooldownState,
 } from "../../open-sse/services/providerCooldownTracker.ts";
+import { connectionCircuitBreakerName } from "../../open-sse/services/connectionCircuitBreaker.ts";
 
 const uniqueProvider = (suffix: string) =>
   `halfopen-test-${suffix}-${Date.now()}-${Math.floor(Math.random() * 1e6)}`;
@@ -87,7 +88,7 @@ test("recordProviderSuccess does not reset cooldown when breaker is OPEN", () =>
     failureThreshold: 1,
     resetTimeoutMs: 60_000,
   });
-  const breaker = getCircuitBreaker(provider);
+  const breaker = getCircuitBreaker(connectionCircuitBreakerName(provider, connectionId));
   assert.equal(breaker.state, "OPEN");
 
   // Success while OPEN should NOT reset cooldown (early-return guard)
@@ -242,7 +243,7 @@ test("recordProviderSuccess with connectionId transitions breaker and resets coo
     resetTimeoutMs: 100,
   });
 
-  const breaker = getCircuitBreaker(provider);
+  const breaker = getCircuitBreaker(connectionCircuitBreakerName(provider, connectionId));
   assert.equal(breaker.state, "OPEN");
 
   // Build up cooldown with connectionId