fix(combo): recover provider circuit breaker from HALF_OPEN on success

config/quality/file-size-baseline.json

@@ -1,4 +1,5 @@
 {
+  "_rebaseline_2026_08_09_9207_breaker_halfopen_recovery": "PR #9207 own growth during the 2026-08-09 rebase: open-sse/services/accountFallback.ts 1978->2020 (+42 = recordProviderSuccess now also transitions the provider circuit breaker from HALF_OPEN to CLOSED when a request succeeds, so the breaker is not stuck half-open after repeated failures; the transition and its reset wiring grow the existing provider-success path, not extractable). Covered by tests/unit/provider-breaker-halfopen-recovery.test.ts.",
   "_rebaseline_2026_08_09_9296_adobe_media_capabilities": "PR #9296 (artickc, fix/adobe-firefly-model-capabilities) own growth: src/app/api/v1/models/catalog.ts 1590->1597 (+7). The image and video catalog serializers now expose the already-normalized Adobe Firefly discovery capability data (media_capabilities, plus the existing video modality/size fields) at their only response-emission chokepoints. The discovery parser and capability normalization remain in open-sse/services/adobeFireflyModels.ts; extracting these seven serialization fields would obscure the catalog contract. Covered by tests/unit/adobe-firefly.test.ts and tests/unit/image-upscale.test.ts.",
   "_rebaseline_2026_08_08_v3850_base_drift_batch_9757": "Base drift on release/v3.8.50, not own growth: the 08-06..08-08 merge batches grew 12 already-frozen (or newly-landed) files without carrying their rebaselines \u2014 the dedicated rebaseline PR #9616 was closed as 'superseded' but its file-size entries never actually reached the base, and later merges (#8894 combos page, #9539 EditConnectionModal, #8895 models route, #9294/#9293 catalog, #9541 db/core, #8970 tokenHealthCheck, #8925 mcp schemas+server, #8890 accountFallback, #9467 chat.ts, #8931 openai-to-kiro, ProxyRegistryManager) kept growing them. All 12 values re-measured on THIS branch's tree (= pure tip + this PR's 1-line chat.ts fix, which adds zero lines). This PR's own source changes (chat.ts identifier restore, stream.ts format carve-out) do not grow any frozen file past these values.",
   "_rebaseline_2026_08_08_migration_135_collision": "fix(db): resolve migration version 135 numbering collision \u2014 #9449's 135_connection_runtime_state.sql and #8908's 135_migrate_model_capability_max_token.sql both claimed version 135 (#9449 branched before #8908 merged and never got renumbered before landing on release/v3.8.50), which threw 'Migration version collision detected' the moment ANY code touched the database \u2014 a fresh install/deploy from this tip cannot even boot. Renumbered the later-landing file to 140 (next free slot) and added the matching isSchemaAlreadyApplied('140') retroactive guard, matching the established pattern already used for the prior 135/136 -> 137/138 renumber in the same file. Own growth: src/lib/db/migrationRunner.ts 1084->1094 (+10, the new case block) \u2014 irreducible, matches the existing per-case guard pattern exactly. Covered by tests/unit/migration-135-numbering-collision.test.ts (2/2), confirmed failing (reproducing the exact live crash) against the pre-fix colliding filenames, passing after.",

open-sse/services/accountFallback.ts

@@ -31,6 +31,7 @@ import {
   looksLikeQuotaExhausted,
   type FailureKind,
 } from "../../src/shared/utils/classify429";
+import { recordProviderSuccess as resetCooldownFailureCount } from "./providerCooldownTracker.ts";
 import { resolveProviderId } from "../../src/shared/constants/providers";
 import { resolveUseUpstream429BreakerHints } from "../../src/shared/utils/providerHints";
 import { getCodexModelScope } from "../config/codexQuotaScopes.ts";
@@ -1001,6 +1002,47 @@ export function recordProviderFailure(
   }
 }
 
+/**
+ * Record a successful request for a provider.
+ * Symmetric counterpart of recordProviderFailure:
+ * - Resets cooldown failureCount (exponential backoff) for all non-OPEN states.
+ * - HALF_OPEN -> CLOSED (probe success), CLOSED/DEGRADED -> decay failureCount.
+ *
+ * When the breaker is OPEN (provider is failing), this is a no-op -- the
+ * cooldown stays intact and the breaker keeps its cooldown period.
+ *
+ * Matches execute()'s behavior: _onSuccess() is called for all non-OPEN states.
+ */
+export function recordProviderSuccess(
+  provider: string | null | undefined,
+  connectionId?: string | null
+): void {
+  if (!provider || provider === "unknown") return;
+
+  const breaker = getProviderBreaker(provider);
+  if (!breaker) return;
+  const breakerState = breaker.getStatus().state;
+
+  // When breaker is OPEN, the provider is failing -- do not reset cooldown
+  // even if one request slipped through (dispatched before the open).
+  // The cooldown resets when the breaker reaches HALF_OPEN and the probe
+  // succeeds below.
+  if (breakerState === "OPEN") return;
+
+  // Reset cooldown failureCount (exponential backoff) -- symmetric with
+  // recordProviderCooldown which increments it on each failure.
+  resetCooldownFailureCount(provider, connectionId ?? undefined);
+
+  // Clear failure-dedup window so the next genuine failure is not suppressed.
+  if (connectionId) {
+    lastConnectionFailure.delete(`${provider}:${connectionId}`);
+  }
+
+  // Transition breaker on success, matching execute()'s behavior:
+  // HALF_OPEN -> CLOSED (probe success), CLOSED/DEGRADED -> decay failureCount.
+  breaker._onSuccess();
+}
+
 /**
  * Reset the shared provider breaker.
  */

open-sse/services/combo.ts

@@ -19,6 +19,7 @@ import {
   MODEL_ACCESS_DENIED_PATTERNS,
   recordModelLockoutFailure,
   recordProviderFailure,
+  recordProviderSuccess,
   selectLockoutCooldownMs,
 } from "./accountFallback.ts";
 import {
@@ -89,11 +90,7 @@ import {
 } from "./combo/promptCacheAffinity.ts";
 import type { CompressionMode } from "./compression/types.ts";
 import { getCachedProviderConnections } from "../../src/lib/db/readCache";
-import {
-  isProviderInCooldown,
-  recordProviderCooldown,
-  recordProviderSuccess,
-} from "./providerCooldownTracker.ts";
+import { isProviderInCooldown, recordProviderCooldown } from "./providerCooldownTracker.ts";
 import {
   resolveResilienceSettings,
   type ResilienceSettings,

open-sse/services/providerCooldownTracker.ts

@@ -171,6 +171,11 @@ export function getRemainingCooldownMs(
 /**
  * Record a successful request for a provider/connection.
  * Resets the failure count (but keeps the entry for reference).
+ *
+ * @deprecated Use accountFallback.recordProviderSuccess instead -- it also
+ * transitions the circuit breaker from HALF_OPEN to CLOSED. This function
+ * only resets the cooldown failureCount without touching the breaker, which
+ * leaves the breaker stuck in HALF_OPEN after repeated failures.
  */
 export function recordProviderSuccess(provider: string, connectionId: string | undefined): void {
   if (!provider || provider === "unknown") return;

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

@@ -0,0 +1,275 @@
+/**
+ * Tests for circuit breaker HALF_OPEN -> CLOSED recovery via recordProviderSuccess.
+ *
+ * Production scenario: provider breaker stuck in HALF_OPEN with 699 requests
+ * at 98% success rate, never recovering. Root cause: combo.ts success path
+ * calls recordProviderSuccess (cooldown-only) but never calls breaker._onSuccess().
+ *
+ * This test verifies that recordProviderSuccess in accountFallback.ts
+ * transitions the breaker from HALF_OPEN to CLOSED and resets failure count.
+ */
+import { test } from "node:test";
+import assert from "node:assert/strict";
+import {
+  getCircuitBreaker,
+  resetAllCircuitBreakers,
+} from "../../src/shared/utils/circuitBreaker.ts";
+import {
+  recordProviderFailure,
+  recordProviderSuccess,
+} from "../../open-sse/services/accountFallback.ts";
+import {
+  recordProviderCooldown,
+  isProviderInCooldown,
+  clearCooldownState,
+} from "../../open-sse/services/providerCooldownTracker.ts";
+
+const uniqueProvider = (suffix: string) =>
+  `halfopen-test-${suffix}-${Date.now()}-${Math.floor(Math.random() * 1e6)}`;
+
+test("recordProviderSuccess transitions breaker from HALF_OPEN to CLOSED and resets failureCount", async () => {
+  const provider = uniqueProvider("recovery");
+
+  // Step 1: Open the breaker (failureThreshold: 1 -> one failure opens it)
+  recordProviderFailure(provider, undefined, undefined, {
+    failureThreshold: 1,
+    resetTimeoutMs: 150,
+  });
+
+  const breaker = getCircuitBreaker(provider);
+  assert.equal(breaker.state, "OPEN", "breaker should be OPEN after failure");
+  assert.equal(breaker.failureCount, 1, "failureCount should be 1");
+
+  // Step 2: Wait past resetTimeout so _refreshOpenState transitions to HALF_OPEN
+  await new Promise((r) => setTimeout(r, 200));
+  breaker.canExecute(); // triggers _refreshOpenState -> HALF_OPEN
+  assert.equal(breaker.state, "HALF_OPEN", "breaker should be HALF_OPEN after resetTimeout");
+
+  // Step 3: recordProviderSuccess should close the breaker
+  recordProviderSuccess(provider, undefined);
+
+  assert.equal(breaker.state, "CLOSED", "breaker should be CLOSED after success");
+  assert.equal(breaker.failureCount, 0, "failureCount should be reset to 0");
+});
+
+test("recordProviderSuccess does not prematurely close an OPEN breaker before resetTimeout", () => {
+  const provider = uniqueProvider("no-premature-close");
+
+  // Open the breaker
+  recordProviderFailure(provider, undefined, undefined, {
+    failureThreshold: 1,
+    resetTimeoutMs: 60_000, // long timeout -- won't elapse during test
+  });
+
+  const breaker = getCircuitBreaker(provider);
+  assert.equal(breaker.state, "OPEN");
+
+  // Call success while still in OPEN (timeout hasn't elapsed)
+  recordProviderSuccess(provider, undefined);
+
+  // Breaker should stay OPEN -- we don't want to bypass the cooldown
+  assert.equal(breaker.state, "OPEN", "breaker must stay OPEN when resetTimeout has not elapsed");
+  assert.equal(breaker.failureCount, 1, "failureCount must not change while OPEN");
+});
+
+test("recordProviderSuccess does not reset cooldown when breaker is OPEN", () => {
+  const provider = uniqueProvider("open-cooldown-guard");
+  const connectionId = "conn-open";
+
+  // Build up cooldown first
+  for (let i = 0; i < 3; i++) {
+    recordProviderCooldown(provider, connectionId);
+  }
+  assert.equal(isProviderInCooldown(provider, connectionId), true);
+
+  // Open the breaker
+  recordProviderFailure(provider, undefined, connectionId, {
+    failureThreshold: 1,
+    resetTimeoutMs: 60_000,
+  });
+  const breaker = getCircuitBreaker(provider);
+  assert.equal(breaker.state, "OPEN");
+
+  // Success while OPEN should NOT reset cooldown (early-return guard)
+  recordProviderSuccess(provider, connectionId);
+
+  assert.equal(breaker.state, "OPEN", "breaker stays OPEN");
+  assert.equal(
+    isProviderInCooldown(provider, connectionId),
+    true,
+    "cooldown must NOT be cleared while breaker is OPEN"
+  );
+});
+
+test("recordProviderSuccess on CLOSED breaker decays failureCount (gradual recovery)", () => {
+  const provider = uniqueProvider("closed-gradual");
+
+  // Get a breaker in CLOSED state (no failures)
+  const breaker = getCircuitBreaker(provider, {
+    failureThreshold: 3,
+    resetTimeoutMs: 30_000,
+  });
+  assert.equal(breaker.state, "CLOSED");
+
+  // Add a failure via public API (but not enough to open: threshold=3)
+  recordProviderFailure(provider, undefined, undefined, {
+    failureThreshold: 3,
+    resetTimeoutMs: 30_000,
+  });
+  assert.equal(breaker.failureCount, 1);
+  assert.equal(breaker.state, "CLOSED"); // still closed (threshold=3)
+
+  // recordProviderSuccess on CLOSED decays failureCount by 1 (matching execute())
+  recordProviderSuccess(provider, undefined);
+
+  assert.equal(breaker.failureCount, 0, "failureCount decremented by 1 (gradual recovery)");
+  assert.equal(breaker.state, "CLOSED", "should stay CLOSED");
+});
+
+test("recordProviderSuccess with null/undefined provider is a safe no-op", () => {
+  // Should not throw
+  recordProviderSuccess(null, undefined);
+  recordProviderSuccess(undefined, undefined);
+  recordProviderSuccess("", undefined);
+});
+
+test("recordProviderSuccess is idempotent: multiple calls on HALF_OPEN are safe", async () => {
+  const provider = uniqueProvider("idempotent");
+
+  // Open the breaker
+  recordProviderFailure(provider, undefined, undefined, {
+    failureThreshold: 1,
+    resetTimeoutMs: 100,
+  });
+
+  const breaker = getCircuitBreaker(provider);
+  assert.equal(breaker.state, "OPEN");
+
+  // Wait for HALF_OPEN
+  await new Promise((r) => setTimeout(r, 150));
+  breaker.canExecute();
+  assert.equal(breaker.state, "HALF_OPEN");
+
+  // First success closes the breaker
+  recordProviderSuccess(provider, undefined);
+  assert.equal(breaker.state, "CLOSED");
+  assert.equal(breaker.failureCount, 0);
+
+  // Second success is a no-op (state is CLOSED, not HALF_OPEN)
+  recordProviderSuccess(provider, undefined);
+  assert.equal(breaker.state, "CLOSED");
+  assert.equal(breaker.failureCount, 0);
+});
+
+test("full lifecycle: CLOSED -> OPEN -> HALF_OPEN -> CLOSED via success", async () => {
+  const provider = uniqueProvider("lifecycle");
+
+  // Start CLOSED
+  const breaker = getCircuitBreaker(provider, {
+    failureThreshold: 1,
+    resetTimeoutMs: 100,
+  });
+  assert.equal(breaker.state, "CLOSED");
+
+  // Failure opens the breaker
+  recordProviderFailure(provider, undefined, undefined, {
+    failureThreshold: 1,
+    resetTimeoutMs: 100,
+  });
+  assert.equal(breaker.state, "OPEN");
+  assert.equal(breaker.failureCount, 1);
+
+  // Still OPEN before timeout
+  recordProviderSuccess(provider, undefined);
+  assert.equal(breaker.state, "OPEN", "must not close before resetTimeout");
+
+  // Wait for HALF_OPEN transition
+  await new Promise((r) => setTimeout(r, 150));
+  breaker.canExecute();
+  assert.equal(breaker.state, "HALF_OPEN");
+
+  // Success recovers to CLOSED
+  recordProviderSuccess(provider, undefined);
+  assert.equal(breaker.state, "CLOSED");
+  assert.equal(breaker.failureCount, 0);
+
+  // Subsequent failure re-opens (proves breaker is functional after recovery)
+  recordProviderFailure(provider, undefined, undefined, {
+    failureThreshold: 1,
+    resetTimeoutMs: 100,
+  });
+  assert.equal(breaker.state, "OPEN");
+  assert.equal(breaker.failureCount, 1);
+});
+
+test("recordProviderSuccess resets cooldown failureCount (exponential backoff)", () => {
+  const provider = uniqueProvider("cooldown-reset");
+  const connectionId = "conn-1";
+
+  // Build up 4 failures -> cooldown with exponential backoff
+  for (let i = 0; i < 4; i++) {
+    recordProviderCooldown(provider, connectionId);
+  }
+
+  // Cooldown should be active (failureCount > 0)
+  assert.equal(
+    isProviderInCooldown(provider, connectionId),
+    true,
+    "provider should be in cooldown after 4 failures"
+  );
+
+  // Success should reset the cooldown
+  recordProviderSuccess(provider, connectionId);
+
+  assert.equal(
+    isProviderInCooldown(provider, connectionId),
+    false,
+    "cooldown should be cleared after success (failureCount reset to 0)"
+  );
+
+  // Breaker should remain CLOSED (cooldown test does not open the breaker)
+  const breaker = getCircuitBreaker(provider);
+  assert.equal(breaker.state, "CLOSED", "breaker stays CLOSED (cooldown-only scenario)");
+});
+
+test("recordProviderSuccess with connectionId transitions breaker and resets cooldown", async () => {
+  const provider = uniqueProvider("connid-breaker");
+  const connectionId = "conn-abc";
+
+  // Open the breaker
+  recordProviderFailure(provider, undefined, connectionId, {
+    failureThreshold: 1,
+    resetTimeoutMs: 100,
+  });
+
+  const breaker = getCircuitBreaker(provider);
+  assert.equal(breaker.state, "OPEN");
+
+  // Build up cooldown with connectionId
+  for (let i = 0; i < 3; i++) {
+    recordProviderCooldown(provider, connectionId);
+  }
+  assert.equal(isProviderInCooldown(provider, connectionId), true);
+
+  // Wait for HALF_OPEN
+  await new Promise((r) => setTimeout(r, 150));
+  breaker.canExecute();
+  assert.equal(breaker.state, "HALF_OPEN");
+
+  // Success with connectionId should close breaker AND reset cooldown
+  recordProviderSuccess(provider, connectionId);
+
+  assert.equal(breaker.state, "CLOSED", "breaker CLOSED after success with connectionId");
+  assert.equal(breaker.failureCount, 0, "failureCount reset");
+  assert.equal(isProviderInCooldown(provider, connectionId), false, "cooldown cleared");
+});
+
+test("resetAllCircuitBreakers cleans up test state", () => {
+  // Verify cleanup works (prevents state leaking between tests)
+  resetAllCircuitBreakers();
+  clearCooldownState();
+  const provider = uniqueProvider("cleanup");
+  const breaker = getCircuitBreaker(provider);
+  assert.equal(breaker.state, "CLOSED");
+  assert.equal(breaker.failureCount, 0);
+});