fix(combo): clear LKGP pin when its target fails, not only set it on success (#10034)

setLKGP() was only ever called on success — nothing invalidated a "last
known good provider" pin once that provider started failing, so a
*separate* subsequent request kept re-selecting the same just-failed
target via applyStrategyOrdering.ts's LKGP reordering.

Live incident: an OpenClaw request to combo "default" (routerStrategy:
lkgp) got a real reasoning + apply_patch tool call from
opencode-zen/big-pickle, then 3 separate follow-up requests over the
next ~2 minutes each independently re-selected the same big-pickle
target and each timed out with "504 Stream produced no non-ping SSE
event within 95000ms" before the client gave up — instead of failing
over to any of the combo's other 12 models.

Root cause confirmed via code read: circuit breaker and model lockout
deliberately don't react to this failure class (isStreamReadinessFailureErrorBody
exempts STREAM_READINESS_TIMEOUT/combo_target_timeout 504s from tripping
the provider breaker, and REQUEST_SCOPED_UPSTREAM_ERROR_CODES suppresses
model-lockout recording for the same class — both intentional, to avoid
poisoning a healthy provider on request-specific timing). Nothing else
in the system was clearing the stale LKGP pin, so it kept winning
target-selection ordering for every new top-level request.

Fix: add clearLKGP(comboName, modelId) to src/lib/db/settings/lkgp.ts,
export it through settings.ts/localDb.ts, and call it (mirroring the
existing setLKGP-on-success call pattern exactly, same two keys) in both
combo.ts's per-target failure paths -- handleComboChat's "Done retrying
this model" block and handleRoundRobinCombo's structurally identical
twin -- right where a target is finally given up on and the loop moves
to the next one.

TDD: new regression test in tests/unit/combo-routing-engine.test.ts
("clears LKGP after the last-known-good target fails") reproduces the
exact live scenario -- confirmed failing against the pre-fix code,
passing after. Added direct unit coverage for clearLKGP itself in
tests/unit/db-settings-crud.test.ts (deletes only the targeted key,
sibling keys survive; no-op on an unset key doesn't throw) and
registered the new export in db-settings-split.test.ts's public API
surface characterization test.

Test plan:
- Full combo/LKGP-related suite (combo-routing-engine, db-settings-crud,
  db-settings-split, combo-strategy-fallbacks,
  combo-selected-connection-success,
  delete-provider-connection-invalidates-lkgp-8887, db-read-cache) --
  183/183 passing.
- npx tsc --noEmit -- clean for all changed files (pre-existing unrelated
  errors elsewhere in the same test files confirmed identical against a
  pristine upstream/release/v3.8.50 checkout, zero diff at those lines).
- npm run lint -- clean (new test's any usage properly typed, not left
  to inflate the file's frozen any-budget suppression).

⚠️ base-red inherited: #9985

open-sse/services/combo.ts

@@ -1996,6 +1996,24 @@ export async function handleComboChat({
             strategy,
             target: toRecordedTarget(target),
           });
+          // LKGP (#919) mirror of the success-path set below: a just-failed target
+          // must not keep re-pinning itself as the "last known good" choice for the
+          // *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 () => {
+            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({
@@ -3135,6 +3153,22 @@ async function handleRoundRobinCombo({
           strategy: "round-robin",
           target: toRecordedTarget(target),
         });
+        // 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 () => {
+          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;

src/lib/db/settings.ts

@@ -815,7 +815,7 @@ export {
   resetAllPricing,
 } from "./settings/pricing";
 
-export { type LKGPRecord, getLKGP, setLKGP, clearAllLKGP } from "./settings/lkgp";
+export { type LKGPRecord, getLKGP, setLKGP, clearAllLKGP, clearLKGP } from "./settings/lkgp";
 
 export {
   type CacheTrendPoint,

src/lib/db/settings/lkgp.ts

@@ -48,6 +48,25 @@ export function clearAllLKGP(): void {
   db.prepare("DELETE FROM key_value WHERE namespace = 'lkgp'").run();
 }
 
+/**
+ * Delete one persisted LKGP pin after its target fails. `setLKGP` is only ever
+ * called on success — nothing previously invalidated a pin once its provider
+ * started failing, so a *separate* subsequent request kept re-selecting the
+ * same just-failed provider via `applyStrategyOrdering.ts`'s LKGP reordering
+ * (live incident: 3 consecutive requests all picked the same timed-out
+ * opencode-zen/big-pickle target instead of failing over to another combo
+ * model). Circuit breaker / model lockout deliberately don't react to this
+ * failure class (request-scoped timeouts, see comboPredicates.ts), so nothing
+ * else clears the stale pin.
+ */
+export async function clearLKGP(comboName: string, modelId: string): Promise<void> {
+  const db = getDbInstance();
+  const key = `${comboName}:${modelId}`;
+  db.prepare("DELETE FROM key_value WHERE namespace = 'lkgp' AND key = ?").run(key);
+  const { invalidateCachedLKGP } = await import("../readCache");
+  invalidateCachedLKGP(key);
+}
+
 /**
  * Delete persisted LKGP pins whose connectionId references a removed provider
  * connection. Provider-level pins and legacy/unparseable values are preserved.

src/lib/localDb.ts

@@ -144,6 +144,7 @@ export {
   // LKGP (Last Known Good Provider) (#919)
   getLKGP,
   setLKGP,
+  clearLKGP,
 
   // Pricing
   getPricing,

tests/unit/combo-routing-engine.test.ts

@@ -2540,10 +2540,59 @@ test("handleComboChat standalone lkgp strategy updates LKGP after a successful c
   }
 
   assert.equal(result.ok, true);
-  // getLKGP now returns LKGPRecord | null — source: src/lib/db/settings.ts getLKGP()
   assert.equal(persistedProvider?.provider, "openai");
 });
 
+test("handleComboChat standalone lkgp strategy clears LKGP after the last-known-good target fails", async () => {
+  // A prior successful request pinned "openai" as the last known good provider —
+  // exactly the state left behind by the previous (success) test's own scenario.
+  await settingsDb.setLKGP("standalone-lkgp-clear", "standalone-lkgp-clear", "openai");
+
+  const calls: string[] = [];
+  const result = await handleComboChat({
+    body: {},
+    combo: {
+      id: "standalone-lkgp-clear",
+      name: "standalone-lkgp-clear",
+      strategy: "lkgp",
+      // maxRetries: 0 below means this single target is tried exactly once,
+      // then the combo loop gives up on it (and on the whole combo, since it's
+      // the only model) — the exact "Done retrying this model" failure path.
+      models: ["openai/gpt-4o-mini"],
+      config: { maxRetries: 0 },
+    },
+    handleSingleModel: async (_body: Record<string, unknown>, modelStr: string) => {
+      calls.push(modelStr);
+      return errorResponse(504, "Stream produced no non-ping SSE event within 95000ms");
+    },
+    isModelAvailable: async () => true,
+    log: createLog(),
+    settings: null,
+    relayOptions: null,
+    allCombos: null,
+  });
+
+  // Give the async fire-and-forget LKGP clear a chance to execute
+  let persistedProvider: Awaited<ReturnType<typeof settingsDb.getLKGP>> = null;
+  for (let i = 0; i < 20; i++) {
+    persistedProvider = await settingsDb.getLKGP("standalone-lkgp-clear", "standalone-lkgp-clear");
+    if (persistedProvider === null) {
+      break;
+    }
+    await new Promise((resolve) => setTimeout(resolve, 10));
+  }
+
+  assert.equal(result.ok, false, "the only target failed, so the whole combo call fails");
+  assert.deepEqual(calls, ["openai/gpt-4o-mini"]);
+  // The bug this guards: without clearing, a *separate* subsequent request would
+  // keep re-selecting "openai" via LKGP reordering even though it just failed.
+  assert.equal(
+    persistedProvider,
+    null,
+    "LKGP must be cleared after its target fails, not left pointing at a just-failed provider"
+  );
+});
+
 test("handleComboChat auto strategy falls back to the full pool when tool filtering empties candidates", async () => {
   await settingsDb.updatePricing({
     openai: {

tests/unit/db-settings-crud.test.ts

@@ -234,6 +234,22 @@ test("LKGP overwrites connectionId when updated without one", async () => {
   assert.deepEqual(record, { provider: "openai" });
 });
 
+test("clearLKGP deletes only the targeted combo/model key", async () => {
+  await settingsDb.setLKGP("combo-f", "model-f", "openai");
+  await settingsDb.setLKGP("combo-f", "model-g", "anthropic");
+
+  await settingsDb.clearLKGP("combo-f", "model-f");
+
+  assert.equal(await settingsDb.getLKGP("combo-f", "model-f"), null);
+  // A sibling key under the same combo must survive.
+  assert.deepEqual(await settingsDb.getLKGP("combo-f", "model-g"), { provider: "anthropic" });
+});
+
+test("clearLKGP on a key with no existing pin does not throw", async () => {
+  await assert.doesNotReject(() => settingsDb.clearLKGP("combo-never-set", "model-never-set"));
+  assert.equal(await settingsDb.getLKGP("combo-never-set", "model-never-set"), null);
+});
+
 test("pricing helpers ignore malformed synced data and LKGP falls back to raw values", async () => {
   const db = core.getDbInstance();
 

tests/unit/db-settings-split.test.ts

@@ -71,6 +71,7 @@ describe("settings.ts public API surface", () => {
     "getLKGP",
     "setLKGP",
     "clearAllLKGP",
+    "clearLKGP",
     // Cache metrics (re-exported from ./settings/cacheMetrics)
     "getCacheMetrics",
     "updateCacheMetrics",