From 38e2baa87913dfb345d911f99fb54159df38ebdd Mon Sep 17 00:00:00 2001 From: "Bob.Hou" Date: Sat, 29 Aug 2026 18:52:06 -0400 Subject: [PATCH] fix(resilience): clear persisted LKGP pin on target exhaustion and skip (#11911) (#12013) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- .../fixes/11911-lkgp-stale-pin-exhaustion.md | 1 + open-sse/services/combo.ts | 78 ++++++--- .../lkgp-stale-pin-exhaustion-11911.test.ts | 165 ++++++++++++++++++ 3 files changed, 218 insertions(+), 26 deletions(-) create mode 100644 changelog.d/fixes/11911-lkgp-stale-pin-exhaustion.md create mode 100644 tests/unit/lkgp-stale-pin-exhaustion-11911.test.ts diff --git a/changelog.d/fixes/11911-lkgp-stale-pin-exhaustion.md b/changelog.d/fixes/11911-lkgp-stale-pin-exhaustion.md new file mode 100644 index 0000000000..1c2a9fe724 --- /dev/null +++ b/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)). diff --git a/open-sse/services/combo.ts b/open-sse/services/combo.ts index ffc5f3e73b..efdbd7c152 100644 --- a/open-sse/services/combo.ts +++ b/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[] = [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 = { "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 () => { - 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, - }); - } - })(); + clearStaleLKGP(combo.name, target.executionKey, combo.id, log, "COMBO"); 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 () => { - 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, - }); - } - })(); + clearStaleLKGP(combo.name, target.executionKey, combo.id, log, "COMBO-RR"); recordedAttempts++; lastError = errorText || String(result.status); lastStatus = result.status; diff --git a/tests/unit/lkgp-stale-pin-exhaustion-11911.test.ts b/tests/unit/lkgp-stale-pin-exhaustion-11911.test.ts new file mode 100644 index 0000000000..25a2e1fcb5 --- /dev/null +++ b/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"); +});