From d98a31587ea97d18b351fb3694bd341be7e30867 Mon Sep 17 00:00:00 2001 From: Diego Rodrigues de Sa e Souza <8016841+diegosouzapw@users.noreply.github.com> Date: Mon, 6 Jul 2026 18:16:10 -0300 Subject: [PATCH] fix(resilience): combo falls back to compat-rejected healthy targets before 503 (#6238) (#6450) --- CHANGELOG.md | 2 + open-sse/services/combo.ts | 41 +++++ .../services/combo/comboCompatFallback.ts | 82 ++++++++++ ...bo-roundrobin-compat-fallback-6238.test.ts | 151 ++++++++++++++++++ 4 files changed, 276 insertions(+) create mode 100644 open-sse/services/combo/comboCompatFallback.ts create mode 100644 tests/unit/combo-roundrobin-compat-fallback-6238.test.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index 2088779ed5..c558d4ee90 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -21,6 +21,8 @@ ### 🐛 Bug Fixes +- **fix(resilience):** a round-robin combo no longer returns `503 all upstream accounts are unavailable` when a compatibility-rejected target is actually healthy ([#6238](https://github.com/diegosouzapw/OmniRoute/issues/6238)). `filterTargetsByRequestCompatibility` drops request-incompatible targets (tool/vision/structured-output unsupported, or below the required context window) **before** any availability check runs, and its `compatible.length === 0` safety net only fired when *all* targets were filtered — not when the kept targets later all turned out runtime-unavailable (circuit-open / cooldown / no credentials). So a combo could 503 while a compat-rejected-but-healthy provider sat unused. `handleRoundRobinCombo` now keeps the compat-rejected set and, when every compat-kept target was skipped without a single real attempt, probes those rejected targets as a **last-resort fallback tier** (via the new pure `open-sse/services/combo/comboCompatFallback.ts`) before crystallizing the 503. Regression guard: `tests/unit/combo-roundrobin-compat-fallback-6238.test.ts`. (thanks @ThongAccount) + - **fix(startup):** best-effort self-heal for a corrupted Turbopack dev cache on Windows ([#6289](https://github.com/diegosouzapw/OmniRoute/issues/6289)). On Windows, `pnpm dev` can fail at startup when Turbopack `mmap`s a persistent-cache SST file and the OS refuses the mapping (`os error 1455` — "paging file too small"), which Turbopack surfaces as a misleading `Module not found: Can't resolve '@/shared/utils/machine'`. This is a **known upstream Turbopack cache-corruption bug — not our code**. The dev launcher (`scripts/dev/run-next.mjs`) now wraps `nextApp.prepare()` and, when it rejects with that signature (`isTurbopackCacheCorruption` in the new `scripts/dev/turbopackCacheHeal.mjs`), purges `.build/next/**/cache/turbopack` and retries **once** with a clear log. **Caveat — best-effort only:** the corruption often surfaces as a runtime overlay rather than a `prepare()` rejection, so this cannot always intercept it; the reliable remedy remains manually deleting the Turbopack cache dir. Regression guard: `tests/unit/turbopack-cache-heal-6289.test.ts`. (thanks @chirag127) - **fix(providers):** qodercli PAT auth no longer fails with `spawn qodercli ENOENT` on Windows ([#6263](https://github.com/diegosouzapw/OmniRoute/issues/6263)) — `spawnQoderCli` spawned the bare `qodercli` name with `shell:false` and an unenriched env, so the npm `.cmd` wrapper under `%APPDATA%\npm` (a user-PATH directory) was never resolved. It now resolves the absolute `.cmd`/`.exe` path through the existing `getCliRuntimeStatus("qoder")` resolver in `src/shared/services/cliRuntime.ts` (memoized), spawns with `shell` when the target is a `.cmd`/`.bat`, and uses the cliRuntime-enriched env (PATH + PATHEXT + APPDATA); the ENOENT error now lists the searched paths plus the `CLI_QODER_BIN` override. End-to-end spawn on a real Windows host is host-only (Hard Rule #18); the path-resolution logic is unit-tested. Regression guard: `tests/unit/qodercli-windows-resolve-6263.test.ts`. (thanks @chirag127) diff --git a/open-sse/services/combo.ts b/open-sse/services/combo.ts index c78970360d..78ee27b958 100644 --- a/open-sse/services/combo.ts +++ b/open-sse/services/combo.ts @@ -142,6 +142,7 @@ import { expandProviderWildcardsInCollection, } from "./combo/providerWildcard.ts"; import { resolveShadowTargets, scheduleShadowRouting } from "./combo/shadowRouting.ts"; +import { attemptCompatRejectedFallback } from "./combo/comboCompatFallback.ts"; import { filterTargetsByRequestCompatibility, resolveComboRuntimeUnits, @@ -2371,6 +2372,15 @@ async function handleRoundRobinCombo({ log, "Context-aware round-robin fallback" ); + // #6238: keep the targets the compat pre-filter rejected so they can serve as a + // last-resort fallback tier. The pre-filter drops request-incompatible targets + // BEFORE availability is known; if every compat-kept target then turns out to be + // runtime-unavailable, we must reconsider these before returning 503, instead of + // permanently dropping a compat-rejected-but-healthy provider. + const compatKeptSet = new Set(filteredTargets); + const compatRejectedTargets = evalRankedTargets.filter( + (target) => !compatKeptSet.has(target) + ); const modelCount = filteredTargets.length; if (modelCount === 0) { return comboModelNotFoundResponse("Round-robin combo has no executable targets"); @@ -2964,6 +2974,37 @@ async function handleRoundRobinCombo({ // All models exhausted const latencyMs = Date.now() - startTime; + + // #6238: every compat-kept target was skipped as unavailable and NONE was ever + // attempted (recordedAttempts === 0). Before crystallizing 503, probe the targets + // the compat pre-filter rejected — a compat-rejected-but-healthy provider is a + // valid last-resort fallback tier, not a permanently dropped target. + if (recordedAttempts === 0 && compatRejectedTargets.length > 0) { + const compatFallbackResult = await attemptCompatRejectedFallback(compatRejectedTargets, body, { + handleSingleModel, + isModelAvailable, + isProviderInCooldown: (target) => + resilienceSettings.providerCooldown.enabled && + Boolean(target.provider && target.provider !== "unknown") && + isProviderInCooldown( + target.provider as string, + target.connectionId as string | undefined, + resilienceSettings + ), + log, + strategy: "round-robin", + }); + if (compatFallbackResult) { + recordComboRequest(combo.name, null, { + success: true, + latencyMs: Date.now() - startTime, + fallbackCount, + strategy: "round-robin", + }); + return compatFallbackResult; + } + } + if (recordedAttempts === 0) { recordComboRequest(combo.name, null, { success: false, diff --git a/open-sse/services/combo/comboCompatFallback.ts b/open-sse/services/combo/comboCompatFallback.ts new file mode 100644 index 0000000000..127f2c354c --- /dev/null +++ b/open-sse/services/combo/comboCompatFallback.ts @@ -0,0 +1,82 @@ +import type { ComboLogger, HandleSingleModel, IsModelAvailable, ResolvedComboTarget } from "./types"; + +/** + * Last-resort fallback tier for combo routing (#6238). + * + * `filterTargetsByRequestCompatibility` drops targets that look request-incompatible + * (tool/vision/structured-output unsupported, or below the required context window) + * BEFORE any runtime availability check runs. Its only safety net triggers when ALL + * targets are filtered — not when the kept targets are later all runtime-unavailable + * (circuit-open / cooldown / no credentials). In that case a combo would return + * `503 ALL_ACCOUNTS_INACTIVE` without ever reconsidering a compat-rejected-but-healthy + * target. + * + * This helper makes those compat-rejected targets a genuine fallback tier: only after + * the primary (compat-kept) targets were all skipped without a single real attempt do + * we probe the rejected set. A compat-rejected target is used only if it is actually + * available and returns a successful upstream response; otherwise the caller keeps its + * original error surface unchanged. + */ +export interface CompatFallbackContext { + handleSingleModel: HandleSingleModel; + isModelAvailable?: IsModelAvailable; + /** Returns true when the target's provider connection is in resilience cooldown. */ + isProviderInCooldown?: (target: ResolvedComboTarget) => boolean; + log: ComboLogger; + /** Effective combo strategy, threaded into the single-model dispatch for telemetry. */ + strategy: string; +} + +/** + * Attempt each compatibility-rejected target in order, returning the first successful + * upstream `Response`. Targets that are unavailable, in cooldown, or return a non-ok + * response are skipped. Returns `null` when no rejected target yields a success, so the + * caller can fall through to its existing error/503 path with an unchanged error surface. + */ +export async function attemptCompatRejectedFallback( + rejectedTargets: ResolvedComboTarget[], + body: Record, + ctx: CompatFallbackContext +): Promise { + if (rejectedTargets.length === 0) return null; + + for (const target of rejectedTargets) { + if (ctx.isModelAvailable) { + const available = await ctx.isModelAvailable(target.modelStr, target); + if (!available) { + ctx.log.debug( + "COMBO", + `Last-resort compat fallback: ${target.modelStr} still unavailable — skipping` + ); + continue; + } + } + + if (ctx.isProviderInCooldown?.(target)) { + ctx.log.debug( + "COMBO", + `Last-resort compat fallback: ${target.modelStr} provider in cooldown — skipping` + ); + continue; + } + + ctx.log.info( + "COMBO", + `Last-resort compat fallback → ${target.modelStr} (all compat-kept targets were unavailable)` + ); + const result = await ctx.handleSingleModel(body, target.modelStr, { + ...target, + effectiveComboStrategy: ctx.strategy, + }); + if (result.ok) { + ctx.log.info("COMBO", `Last-resort compat fallback succeeded via ${target.modelStr}`); + return result; + } + ctx.log.debug( + "COMBO", + `Last-resort compat fallback: ${target.modelStr} failed (${result.status}) — trying next` + ); + } + + return null; +} diff --git a/tests/unit/combo-roundrobin-compat-fallback-6238.test.ts b/tests/unit/combo-roundrobin-compat-fallback-6238.test.ts new file mode 100644 index 0000000000..8ab28e1085 --- /dev/null +++ b/tests/unit/combo-roundrobin-compat-fallback-6238.test.ts @@ -0,0 +1,151 @@ +// Regression guard for #6238: a round-robin combo returned +// `503 all upstream accounts are unavailable` immediately when every +// compatibility-KEPT target was runtime-unavailable, without ever +// reconsidering a compatibility-REJECTED-but-healthy target. The compat +// pre-filter (filterTargetsByRequestCompatibility) drops request-incompatible +// targets BEFORE availability is known, and its `compatible.length === 0` +// safety net only fires when ALL targets are filtered — not when the kept +// targets later all turn out unavailable. The fix makes the rejected targets a +// genuine last-resort fallback tier. +import test 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-rr-compat-6238-")); +const ORIGINAL_DATA_DIR = process.env.DATA_DIR; +process.env.DATA_DIR = TEST_DATA_DIR; + +const { handleComboChat } = await import("../../open-sse/services/combo.ts"); +const core = await import("../../src/lib/db/core.ts"); +const settingsDb = await import("../../src/lib/db/settings.ts"); +const { saveModelsDevCapabilities, clearModelsDevCapabilities } = + await import("../../src/lib/modelsDevSync.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"); + +function createLog() { + return { + info: () => {}, + warn: () => {}, + error: () => {}, + debug: () => {}, + }; +} + +function okResponse(body: unknown = { choices: [{ message: { content: "ok" } }] }) { + return new Response(JSON.stringify(body), { + status: 200, + headers: { "content-type": "application/json" }, + }); +} + +function capabilityEntry(limitContext: unknown, overrides: Record = {}) { + return { + tool_call: true, + reasoning: false, + attachment: false, + structured_output: true, + temperature: true, + modalities_input: JSON.stringify(["text"]), + modalities_output: JSON.stringify(["text"]), + knowledge_cutoff: null, + release_date: null, + last_updated: null, + status: null, + family: null, + open_weights: false, + limit_context: limitContext, + limit_input: limitContext, + limit_output: 4096, + interleaved_field: null, + ...overrides, + }; +} + +test.beforeEach(() => { + resetAllComboMetrics(); + resetAllCircuitBreakers(); + resetAllSemaphores(); + clearModelsDevCapabilities(); +}); + +test.after(() => { + resetAllComboMetrics(); + resetAllCircuitBreakers(); + resetAllSemaphores(); + clearModelsDevCapabilities(); + settingsDb.clearAllLKGP(); + core.resetDbInstance(); + fs.rmSync(TEST_DATA_DIR, { recursive: true, force: true }); + if (ORIGINAL_DATA_DIR === undefined) { + delete process.env.DATA_DIR; + } else { + process.env.DATA_DIR = ORIGINAL_DATA_DIR; + } +}); + +test( + "round-robin falls back to a compat-rejected healthy target instead of 503 " + + "when every compat-kept target is unavailable (#6238)", + async () => { + // rr-a is tool-INCAPABLE → the tools-requiring request makes the compat + // pre-filter reject it. rr-b/rr-c are tool-capable → kept, but both are + // runtime-unavailable. Only the rejected rr-a is actually healthy. + saveModelsDevCapabilities({ + openai: { + "rr-a": capabilityEntry(128000, { tool_call: false }), + "rr-b": capabilityEntry(128000), + "rr-c": capabilityEntry(128000), + }, + }); + + const attempted: string[] = []; + const availabilityChecks: string[] = []; + + const result = await handleComboChat({ + body: { + messages: [{ role: "user", content: "Use a tool to look something up" }], + tools: [{ type: "function", function: { name: "lookup_weather" } }], + }, + combo: { + name: "rr-compat-fallback-6238", + strategy: "round-robin", + models: ["openai/rr-a", "openai/rr-b", "openai/rr-c"], + config: { maxRetries: 0, concurrencyPerModel: 1, queueTimeoutMs: 1000 }, + }, + handleSingleModel: async (_body, modelStr) => { + attempted.push(modelStr); + return okResponse({ choices: [{ message: { content: `served by ${modelStr}` } }] }); + }, + // Only the compat-rejected rr-a is healthy; the compat-kept rr-b/rr-c + // are all runtime-unavailable. + isModelAvailable: async (modelStr) => { + availabilityChecks.push(modelStr); + return modelStr === "openai/rr-a"; + }, + log: createLog(), + settings: null, + relayOptions: null, + allCombos: null, + }); + + // Before the fix this returned 503 ALL_ACCOUNTS_INACTIVE without ever + // attempting rr-a. After the fix the compat-rejected-but-healthy rr-a + // serves the request. + assert.equal(result.status, 200, "expected a 200 from the compat-rejected fallback, not 503"); + assert.equal(result.ok, true); + assert.deepEqual(attempted, ["openai/rr-a"], "only the healthy compat-rejected target is used"); + + const payload = await result.json(); + assert.equal(payload.choices[0].message.content, "served by openai/rr-a"); + // Sanity: the compat-kept rr-b/rr-c were probed for availability first. + assert.ok( + availabilityChecks.includes("openai/rr-b") || availabilityChecks.includes("openai/rr-c"), + "compat-kept targets should be probed before the last-resort fallback" + ); + } +);