fix(resilience): clear persisted LKGP pin on target exhaustion and skip (#11911) (#12013)

When an auto/*/lkgp combo target failed into exhaustion (e.g. an unauthenticated free-tier 401) or was skipped pre-dispatch (cooldown, model lockout, unavailability), the Last Known Good Provider pin was never cleared — so subsequent requests kept re-selecting the same dead provider, causing repeated failures and mass-skipping instead of falling through to a healthy target. Centralizes invalidation into clearStaleLKGP(), invoked from both handleComboChat and handleRoundRobinCombo on exhaustion, pre-dispatch skip, and body-specific 400 termination.

changelog.d/fixes/11911-lkgp-stale-pin-exhaustion.md

@@ -0,0 +1 @@
+- **fix(resilience):** clear persisted LKGP pins when a target suffers connection/provider exhaustion or is skipped before dispatch due to cooldown/exhaustion/unavailability, preventing subsequent requests from repeatedly prioritizing known-dead providers ([#11911](https://github.com/diegosouzapw/OmniRoute/issues/11911)).

open-sse/services/combo.ts

@@ -357,6 +357,33 @@ export function releaseStickyPinOnFailure(
   clearStickyBinding(messageHash);
 }
 
+/**
+ * Clear persisted LKGP pins when a target fails or is skipped due to
+ * exhaustion, cooldown, or unavailability (#11911 #919).
+ */
+export function clearStaleLKGP(
+  comboName: string,
+  executionKey?: string | null,
+  comboId?: string | null,
+  log?: { warn?: (tag: string, msg: string, data?: unknown) => void } | null,
+  tag: string = "COMBO"
+): void {
+  void (async () => {
+    try {
+      const { clearLKGP } = await import("@/lib/localDb");
+      const promises: Promise<void>[] = [clearLKGP(comboName, comboId || comboName)];
+      if (executionKey) {
+        promises.push(clearLKGP(comboName, executionKey));
+      }
+      await Promise.all(promises);
+    } catch (err) {
+      log?.warn?.(tag, "Failed to clear Last Known Good Provider. This is non-fatal.", {
+        err,
+      });
+    }
+  })();
+}
+
 const DEFAULT_MODEL_P95_MS: Record<string, number> = {
   "grok-4-fast-non-reasoning": 1143,
   "grok-4-1-fast-non-reasoning": 1244,
@@ -1204,6 +1231,7 @@ async function handleComboChatInner({
           strategy === "priority" && target.fallbackOnlyOnQuotaExhaustion === true;
         const stopProtectedPriorityTarget = (message: string) => {
           observeFailure(false, target.executionKey);
+          clearStaleLKGP(combo.name, target.executionKey, combo.id, log, "COMBO");
           return protectedPriorityTarget
             ? { ok: false, response: errorResponse(503, message) }
             : null;
@@ -1265,6 +1293,7 @@ async function handleComboChatInner({
           );
           if (persistedSkip) {
             log.info("COMBO", persistedSkip);
+            clearStaleLKGP(combo.name, target.executionKey, combo.id, log, "COMBO");
             if (i > 0) fallbackCount++;
             return null;
           }
@@ -1323,6 +1352,7 @@ async function handleComboChatInner({
               "COMBO",
               `Skipping ${modelStr} — quota exhaustion cutoff (${quotaCutoff.reason || "quota_exhausted"})`
             );
+            clearStaleLKGP(combo.name, target.executionKey, combo.id, log, "COMBO");
             recordComboDecision(traceInvocationId, {
               step: target.executionKey,
               target: modelStr,
@@ -1360,6 +1390,7 @@ async function handleComboChatInner({
               "COMBO",
               `Skipping ${modelStr} — quota budget ${quotaDecision.reason} (remaining ${quotaDecision.tokensRemaining ?? 0}, cost ${quotaDecision.estimatedCost ?? 0})`
             );
+            clearStaleLKGP(combo.name, target.executionKey, combo.id, log, "COMBO");
             if (i > 0) fallbackCount++;
             return null;
           }
@@ -1377,6 +1408,7 @@ async function handleComboChatInner({
               "COMBO",
               `Skipping ${modelStr} — no credentials available or model excluded`
             );
+            clearStaleLKGP(combo.name, target.executionKey, combo.id, log, "COMBO");
             recordComboDecision(traceInvocationId, {
               step: target.executionKey,
               target: modelStr,
@@ -2182,6 +2214,13 @@ async function handleComboChatInner({
           // exhausted — if it's the currently sticky-bound one, release the pin now
           // rather than waiting for the next turn's lazy headroom/status recheck.
           releaseStickyPinOnFailure(_sticky.messageHash, targetWithConnection.connectionId);
+          if (
+            providerExhausted ||
+            exhaustedConnections.has(`${provider}:${targetWithConnection.connectionId}`) ||
+            (provider && exhaustedProviders.has(provider))
+          ) {
+            clearStaleLKGP(combo.name, target.executionKey, combo.id, log, "COMBO");
+          }
 
           // #2101: Prevent infinite fallback loops with 400 Bad Request errors that are genuinely
           // body-specific (malformed JSON, bad format, missing required fields).
@@ -2228,6 +2267,7 @@ async function handleComboChatInner({
             lastStatus = result.status;
             if (i > 0) fallbackCount++;
             log.warn("COMBO", `Model ${modelStr} failed with body-specific error, stopping combo`);
+            clearStaleLKGP(combo.name, target.executionKey, combo.id, log, "COMBO");
             // #4279: surface the 400 via the {ok,response} contract so the OUTER
             // target loop resolves the combo and stops. A bare `break` here only
             // exits the inner retry loop; executeTarget then returns null, which
@@ -2416,19 +2456,7 @@ async function handleComboChatInner({
           // *next* separate request. Circuit breaker / model lockout deliberately
           // don't react to request-scoped failure classes (see scopedFailure below),
           // so nothing else clears this stale pin.
-          void (async () => {
+          clearStaleLKGP(combo.name, target.executionKey, combo.id, log, "COMBO");
-            try {
-              const { clearLKGP } = await import("../../src/lib/localDb");
-              await Promise.all([
-                clearLKGP(combo.name, target.executionKey),
-                clearLKGP(combo.name, combo.id || combo.name),
-              ]);
-            } catch (err) {
-              log.warn("COMBO", "Failed to clear Last Known Good Provider. This is non-fatal.", {
-                err,
-              });
-            }
-          })();
           recordedAttempts++;
           lastError = errorText || String(result.status);
           comboErrors.push({
@@ -3249,6 +3277,7 @@ async function handleRoundRobinCombo({
             "COMBO-RR",
             `Skipping ${modelStr} — no credentials available or model excluded`
           );
+          clearStaleLKGP(combo.name, target.executionKey, combo.id, log, "COMBO-RR");
           if (offset > 0) fallbackCount++;
           continue;
         }
@@ -3264,6 +3293,7 @@ async function handleRoundRobinCombo({
         )
       ) {
         log.info("COMBO-RR", `Skipping ${modelStr} — provider ${provider} in global cooldown`);
+        clearStaleLKGP(combo.name, target.executionKey, combo.id, log, "COMBO-RR");
         if (offset > 0) fallbackCount++;
         continue;
       }
@@ -3276,6 +3306,7 @@ async function handleRoundRobinCombo({
       );
       if (exhaustedSkip) {
         log.info("COMBO-RR", exhaustedSkip);
+        clearStaleLKGP(combo.name, target.executionKey, combo.id, log, "COMBO-RR");
         if (offset > 0) fallbackCount++;
         continue;
       }
@@ -3711,6 +3742,13 @@ async function handleRoundRobinCombo({
             _rrSessionSticky.messageHash,
             targetWithConnection.connectionId
           );
+          if (
+            providerExhausted ||
+            exhaustedConnections.has(`${provider}:${targetWithConnection.connectionId}`) ||
+            (provider && exhaustedProviders.has(provider))
+          ) {
+            clearStaleLKGP(combo.name, target.executionKey, combo.id, log, "COMBO-RR");
+          }
 
           // Transient errors → mark in semaphore so round-robin stops stampeding this target.
           if (
@@ -3766,19 +3804,7 @@ async function handleRoundRobinCombo({
           // LKGP (#919) mirror of handleComboChat's failure-path clear above — see
           // that comment for why this must happen (nothing else clears a pin left
           // by a request-scoped failure class like a stream-readiness timeout).
-          void (async () => {
+          clearStaleLKGP(combo.name, target.executionKey, combo.id, log, "COMBO-RR");
-            try {
-              const { clearLKGP } = await import("../../src/lib/localDb");
-              await Promise.all([
-                clearLKGP(combo.name, target.executionKey),
-                clearLKGP(combo.name, combo.id || combo.name),
-              ]);
-            } catch (err) {
-              log.warn("COMBO-RR", "Failed to clear Last Known Good Provider. This is non-fatal.", {
-                err,
-              });
-            }
-          })();
           recordedAttempts++;
           lastError = errorText || String(result.status);
           lastStatus = result.status;

tests/unit/lkgp-stale-pin-exhaustion-11911.test.ts

@@ -0,0 +1,165 @@
+/**
+ * Regression test for #11911:
+ * When a provider/connection fails and enters exhaustion sets (e.g. auth-level 401 or
+ * connection-level error on an unauthenticated free tier connection), or when a target
+ * is skipped before dispatch because it is unavailable, locked, in cooldown, or already
+ * exhausted for this request, any persisted LKGP pin for that target/combo must be cleared
+ * so subsequent requests do not keep re-pinning the dead provider.
+ */
+import test, { after, beforeEach } from "node:test";
+import assert from "node:assert/strict";
+import fs from "node:fs";
+import os from "node:os";
+import path from "node:path";
+
+const TEST_DATA_DIR = fs.mkdtempSync(path.join(os.tmpdir(), "omniroute-lkgp-stale-11911-"));
+process.env.DATA_DIR = TEST_DATA_DIR;
+
+const { handleComboChat } = await import("../../open-sse/services/combo.ts");
+const settingsDb = await import("../../src/lib/db/settings.ts");
+const core = await import("../../src/lib/db/core.ts");
+const { resetAllComboMetrics } = await import("../../open-sse/services/comboMetrics.ts");
+const { resetAllCircuitBreakers } = await import("../../src/shared/utils/circuitBreaker.ts");
+const { resetAll: resetAllSemaphores } = await import("../../open-sse/services/rateLimitSemaphore.ts");
+
+after(() => {
+  core.resetDbInstance();
+  try {
+    fs.rmSync(TEST_DATA_DIR, { recursive: true, force: true });
+  } catch {}
+});
+
+beforeEach(() => {
+  resetAllComboMetrics();
+  resetAllCircuitBreakers();
+  resetAllSemaphores();
+  settingsDb.clearAllLKGP();
+});
+
+function createLog() {
+  return {
+    info: () => {},
+    warn: () => {},
+    error: () => {},
+    debug: () => {},
+  };
+}
+
+function jsonResponse(status: number, body: unknown) {
+  return new Response(JSON.stringify(body), {
+    status,
+    headers: { "content-type": "application/json" },
+  });
+}
+
+test("#11911: handleComboChat clears LKGP pin when target is skipped before dispatch (unavailable)", async () => {
+  const comboName = "auto-coding-skip-unavail";
+  const modelStr1 = "opencode/deepseek-free";
+  const modelStr2 = "felo/felo-flash";
+
+  await settingsDb.setLKGP(comboName, comboName, "opencode", "noauth");
+  await settingsDb.setLKGP(comboName, `opencode>${modelStr1}`, "opencode", "noauth");
+
+  const result = await handleComboChat({
+    body: { messages: [{ role: "user", content: "hi" }] },
+    combo: {
+      name: comboName,
+      strategy: "lkgp",
+      models: [modelStr1, modelStr2],
+      config: { maxRetries: 0 },
+    },
+    handleSingleModel: async (_body, targetModel) => {
+      if (targetModel.includes("opencode")) {
+        throw new Error("opencode should not be called if unavailable");
+      }
+      return jsonResponse(502, { error: { message: "felo upstream error" } });
+    },
+    isModelAvailable: async (modelStr) => {
+      if (modelStr.includes("opencode")) return false;
+      return true;
+    },
+    log: createLog(),
+    settings: null,
+    relayOptions: null,
+    allCombos: null,
+  });
+
+  assert.equal(result.status, 502);
+  const pinAfter = await settingsDb.getLKGP(comboName, comboName);
+  assert.equal(pinAfter, null, "stale LKGP pin for unavailable target must be cleared");
+});
+
+test("#11911: handleComboChat clears LKGP pin when target is skipped before dispatch due to request exhaustion", async () => {
+  const comboName = "auto-coding-skip-exhausted";
+  const modelStr1 = "opencode/deepseek-free";
+  const modelStr2 = "opencode/north-mini-free";
+  const modelStr3 = "felo/felo-flash";
+
+  await settingsDb.setLKGP(comboName, comboName, "opencode", "noauth");
+
+  const result = await handleComboChat({
+    body: { messages: [{ role: "user", content: "hi" }] },
+    combo: {
+      name: comboName,
+      strategy: "lkgp",
+      models: [modelStr1, modelStr2, modelStr3],
+      config: { maxRetries: 0 },
+    },
+    handleSingleModel: async (_body, targetModel) => {
+      if (targetModel === modelStr1) {
+        return jsonResponse(401, {
+          error: { message: "Auth failed on connection noauth", type: "authentication_error" },
+        });
+      }
+      if (targetModel === modelStr2) {
+        throw new Error("modelStr2 should be skipped by request exhaustion");
+      }
+      return jsonResponse(502, { error: { message: "felo failed" } });
+    },
+    log: createLog(),
+    settings: null,
+    relayOptions: null,
+    allCombos: null,
+  });
+
+  assert.equal(result.status, 502);
+  const pinAfter = await settingsDb.getLKGP(comboName, comboName);
+  assert.equal(pinAfter, null, "LKGP pin must be cleared when provider connection is exhausted");
+});
+
+test("#11911: handleComboChat (round-robin) clears LKGP pin when target is skipped before dispatch", async () => {
+  const comboName = "rr-skip-unavail";
+  const modelStr1 = "opencode/deepseek-free";
+  const modelStr2 = "felo/felo-flash";
+
+  await settingsDb.setLKGP(comboName, comboName, "opencode", "noauth");
+  await settingsDb.setLKGP(comboName, `opencode>${modelStr1}`, "opencode", "noauth");
+
+  const result = await handleComboChat({
+    body: { messages: [{ role: "user", content: "hi" }] },
+    combo: {
+      name: comboName,
+      strategy: "round-robin",
+      models: [modelStr1, modelStr2],
+      config: { maxRetries: 0 },
+    },
+    handleSingleModel: async (_body, targetModel) => {
+      if (targetModel.includes("opencode")) {
+        throw new Error("opencode should not be called if unavailable");
+      }
+      return jsonResponse(502, { error: { message: "felo upstream error" } });
+    },
+    isModelAvailable: async (modelStr) => {
+      if (modelStr.includes("opencode")) return false;
+      return true;
+    },
+    log: createLog(),
+    settings: null,
+    relayOptions: null,
+    allCombos: null,
+  });
+
+  assert.equal(result.status, 502);
+  const pinAfter = await settingsDb.getLKGP(comboName, comboName);
+  assert.equal(pinAfter, null, "stale LKGP pin in round-robin must be cleared on unavailable skip");
+});