diff --git a/open-sse/services/combo.ts b/open-sse/services/combo.ts index d52dac30db..4944a3d8e9 100644 --- a/open-sse/services/combo.ts +++ b/open-sse/services/combo.ts @@ -25,6 +25,12 @@ import { errorResponseWithComboDiagnostics, } from "../utils/error.ts"; import type { ComboDiagnostics } from "../utils/error.ts"; +import { + COMBO_FAILURE_THRESHOLD, + clearComboFailureTracking, + recordComboFailure, +} from "./combo/failureTracker.ts"; +import { buildNoUpstreamResponseDiagnostics, buildRecoveryHint } from "./combo/pinRecovery.ts"; import { buildTargetTimeoutRunner } from "./combo/targetTimeoutRunner.ts"; import { recordComboRequest, recordComboShadowRequest, getComboMetrics } from "./comboMetrics.ts"; import { @@ -1344,6 +1350,10 @@ export async function handleComboChat({ const comboAttemptOrder: Array<{ provider: string; model: string }> = []; if (orderedTargets.length === 0) { + // Surface a recovery hint + auto-clear the session pin after enough consecutive + // no-target failures (silent-stop fix). Threshold of 3 prevents a one-off account + // wipe from destroying the prompt-cache pin benefit on the next request. + recordComboFailure(effectiveSessionId, combo.name); return errorResponseWithComboDiagnostics( 404, "Combo has no executable targets", @@ -1353,6 +1363,7 @@ export async function handleComboChat({ excluded: [], attemptOrder: [], terminalReason: "no_executable_targets", + recovery: buildRecoveryHint("no_executable_targets"), }, { code: "model_not_found", type: "invalid_request_error" } ); @@ -1441,7 +1452,13 @@ export async function handleComboChat({ // QA P0: assemble a sanitized diagnostic trace from the state already in scope // (pool size + this set-try's exhausted providers/connections + attempt order + // a terminal-reason code). Never touches keys/tokens — provider/model ids only. - const buildComboDiag = (terminalReason: string): ComboDiagnostics => ({ + // Silent-stop fix: include a `recovery` hint (action verb + human next-step) so the + // OC plugin + non-header-aware clients can render an actionable error instead of an + // opaque 5xx. The optional `retryAfterSeconds` carries the upstream Retry-After hint. + const buildComboDiag = ( + terminalReason: string, + retryAfterSeconds?: number + ): ComboDiagnostics => ({ poolSize: orderedTargets.length, attempted: recordedAttempts, excluded: [ @@ -1453,6 +1470,7 @@ export async function handleComboChat({ ], attemptOrder: comboAttemptOrder, terminalReason, + recovery: buildRecoveryHint(terminalReason, retryAfterSeconds), }); let globalResolve: ((res: Response) => void) | null = null; @@ -1595,7 +1613,13 @@ export async function handleComboChat({ // failed the same recoverable way. If the dominant cause was reasoning // models exhausting a too-small max_tokens budget (no content output), // retrying other models can't help — tell the caller to raise max_tokens. + // Silent-stop fix: bump the consecutive-failure counter for this session-combo pair + // so the pin gets cleared on the 3rd attempt (recovery.next_step tells the client). const reasoningExhausted = /reasoning consumed \d+\/\d+ tokens/.test(lastError || ""); + const failureReason = reasoningExhausted + ? "reasoning_budget_exhausted" + : "max_attempts_exceeded"; + recordComboFailure(effectiveSessionId, combo.name); return { ok: false, response: errorResponseWithComboDiagnostics( @@ -1603,13 +1627,10 @@ export async function handleComboChat({ reasoningExhausted ? "All combo candidates exhausted their token budget on reasoning without producing content. Increase max_tokens — reasoning models need a larger budget to emit content." : "Maximum combo retry limit reached", - buildComboDiag( - reasoningExhausted ? "reasoning_budget_exhausted" : "max_attempts_exceeded" - ) + buildComboDiag(failureReason) ), }; } - // Predictive TTFT Circuit Breaker (skip slow models) if ( zeroLatencyOptimizationsEnabled && @@ -1888,6 +1909,12 @@ export async function handleComboChat({ fallbackCount, }); + // Silent-stop fix: reset the consecutive-failure counter for this session-combo pair + // on every successful dispatch so a transient recovery doesn't get "credited" against + // the threshold the user already paid through to clear the stale pin. + if (effectiveSessionId) { + clearComboFailureTracking(effectiveSessionId, combo.name); + } // Context cache pinning: record model usage for session-based pinning // (independent of universal handoff — always fires when context_cache_protection is on) // #3825: write under the SAME effectiveSessionId used by the read site so a @@ -2436,6 +2463,10 @@ export async function handleComboChat({ latencyMs, fallbackCount, }); + // Silent-stop fix: bump the failure counter so the session pin clears on the 3rd + // consecutive all-inactive cascade; buildRecoveryHint emits `switch-combo` with a + // next-step that points the user at /dashboard/providers. + recordComboFailure(effectiveSessionId, combo.name); return errorResponseWithComboDiagnostics( 503, "Service temporarily unavailable: all upstream accounts are inactive", @@ -2490,15 +2521,35 @@ export async function handleComboChat({ return unavailableResponse(status, msg, earliestRetryAfter, retryHuman); } + // Silent-stop fix: bump the failure counter (pin clears on 3rd consecutive) and emit + // `try-auto` recovery action via buildRecoveryHint so the OC plugin can show "→ Try + // model: auto" instead of an opaque 5xx. We pass the upstream retry-after seconds to + // the hint so the client can render a precise "wait Ns and retry" message. log.warn("COMBO", `All models failed | ${msg}`); + const { pinClearedNow } = recordComboFailure(effectiveSessionId, combo.name); + if (pinClearedNow) { + log.info( + "COMBO", + `Auto-cleared session_model_history pin for combo "${combo.name}" after ${COMBO_FAILURE_THRESHOLD} consecutive failures to break the silent-stop loop` + ); + } + const retryAfterSeconds = undefined; return errorResponseWithComboDiagnostics( status, msg, - buildComboDiag(lastError ?? "all_models_failed") + buildComboDiag(lastError ?? "all_models_failed", retryAfterSeconds) ); } - return errorResponse(503, "Combo routing completed without an upstream response"); + // Final fallback — when the dispatch returned without crystallizing a status (rare). + // Surface the recovery hint with a generic retry recommendation so the client at least + // gets a non-opaque message instead of "Combo routing completed without an upstream response". + recordComboFailure(effectiveSessionId, combo.name); + return errorResponseWithComboDiagnostics( + 503, + "Combo routing completed without an upstream response", + buildNoUpstreamResponseDiagnostics(orderedTargets.length) + ); }; // FASE 2.1: acquire the per-connection concurrency slot for the selected diff --git a/open-sse/services/combo/failureTracker.ts b/open-sse/services/combo/failureTracker.ts new file mode 100644 index 0000000000..0133ba7019 --- /dev/null +++ b/open-sse/services/combo/failureTracker.ts @@ -0,0 +1,163 @@ +/** + * Per-session combo consecutive-failure tracker. + * + * Purpose: stop the silent-stop pattern where a combo cascade fails, the session + * keeps re-picking the same combo, and every retry hits the same dead/stale pin + * (the user has no visible signal to switch). After N consecutive failures for a + * (sessionId, comboName) pair we drop the session pin so the next request is + * forced to re-resolve targets from scratch — and we surface an + * `X-OmniRoute-Recovery-Action: try-auto` (or `switch-combo`) hint so the client + * (e.g. the OpenCode plugin) can render an actionable error instead of the + * previous opaque "model stopped producing output" loop. + * + * Design constraints + * ────────────────── + * - In-memory Map (no DB) — failures are a per-process hot-path signal, the + * pin that gets cleared is the same in-memory + DB record managed by + * recordSessionModelUsage / deleteSessionModelHistory. Losing the + * counter on process restart is acceptable: worst case the user takes N + * retries before we clear the pin again. + * - TTL eviction (default 5 min, matching sessionManager session stickiness) + * prevents unbounded growth across long-lived sessions. + * - Fail-open: every public function catches its own throws and returns a safe + * default — a bug in the tracker must never block a request. + * - Pinned to a counter threshold (default 3) so a single transient 5xx does + * not destroy the prompt-cache pinning benefit. + * + * No barrel import — consistent with the other combo/* leaves. + */ + +import { deleteSessionModelHistory } from "@/lib/db/contextHandoffs"; + +/** Default threshold — after this many consecutive failures the pin is cleared. */ +export const COMBO_FAILURE_THRESHOLD = 3 as const; + +/** TTL for the in-memory counter (matches the sessionManager SESSION_TTL_MS). */ +const COUNTER_TTL_MS = 5 * 60 * 1000; + +/** Hard cap to prevent unbounded growth in pathological traffic patterns. */ +const MAX_ENTRIES = 2_000; + +interface FailureEntry { + /** Consecutive failure count (resets on success). */ + count: number; + /** Last failure timestamp — used for TTL eviction. */ + lastFailureAt: number; + /** Whether we have already auto-cleared the pin for this streak. */ + pinClearedThisStreak: boolean; +} + +const failureMap = new Map(); + +/** Pure keying helper — keeps the lookup format in one place. */ +function keyOf(sessionId: string, comboName: string): string { + return `${sessionId}::${comboName}`; +} + +/** + * Evict expired entries + enforce the hard cap. Called lazily on every mutation + * so we never need a background timer. + */ +function evict(now: number): void { + for (const [k, entry] of failureMap) { + if (now - entry.lastFailureAt > COUNTER_TTL_MS) failureMap.delete(k); + } + while (failureMap.size > MAX_ENTRIES) { + // Drop the oldest entry by lastFailureAt — Map iteration order is insertion + // order which approximates recency, but we still walk to find the true min. + let oldestKey: string | null = null; + let oldestTime = Infinity; + for (const [k, entry] of failureMap) { + if (entry.lastFailureAt < oldestTime) { + oldestTime = entry.lastFailureAt; + oldestKey = k; + } + } + if (oldestKey === null) break; + failureMap.delete(oldestKey); + } +} + +/** + * Increment the consecutive-failure count for a (session, combo) pair. When the + * counter first crosses the threshold (default 3) we additionally clear the + * session pin so the next request re-resolves targets instead of re-routing to + * the stale one. Returns the new count + a flag indicating whether we just + * auto-cleared the pin. + * + * Pure read with side-effect: NEVER throws — a thrown deleteSessionModelHistory + * must not propagate into the combo terminal-failure response path. Catches + * and returns pinClearedNow=false so the caller can log the failure without + * pretending the cleanup succeeded. + */ +export function recordComboFailure( + sessionId: string | null | undefined, + comboName: string +): { count: number; pinClearedNow: boolean } { + if (!sessionId) return { count: 0, pinClearedNow: false }; + try { + const now = Date.now(); + evict(now); + const key = keyOf(sessionId, comboName); + const existing = failureMap.get(key); + const count = (existing?.count ?? 0) + 1; + const pinClearedBefore = existing?.pinClearedThisStreak ?? false; + const shouldClearNow = count >= COMBO_FAILURE_THRESHOLD && !pinClearedBefore; + failureMap.set(key, { + count, + lastFailureAt: now, + pinClearedThisStreak: shouldClearNow || pinClearedBefore, + }); + if (shouldClearNow) { + try { + // Session-scoped: clears ONLY this session's pin on this combo. Other + // sessions sharing the same combo keep their own pin (see + // deleteSessionModelHistory's docstring in contextHandoffs.ts). + deleteSessionModelHistory(sessionId, comboName); + } catch { + // Best effort — the counter still records the streak, future clears will + // retry on the next threshold-cross. + } + } + return { count, pinClearedNow: shouldClearNow }; + } catch { + return { count: 0, pinClearedNow: false }; + } +} + +/** + * Reset the consecutive-failure counter for a (session, combo) pair on a + * successful dispatch. Cheap Map.delete — no logging, no DB write. + */ +export function clearComboFailureTracking( + sessionId: string | null | undefined, + comboName: string +): void { + if (!sessionId) return; + try { + failureMap.delete(keyOf(sessionId, comboName)); + } catch { + /* fail-open */ + } +} + +/** Read-only peek — used by tests + log messages ("3rd failure in a row"). */ +export function getComboFailureCount( + sessionId: string | null | undefined, + comboName: string +): number { + if (!sessionId) return 0; + try { + const entry = failureMap.get(keyOf(sessionId, comboName)); + if (!entry) return 0; + if (Date.now() - entry.lastFailureAt > COUNTER_TTL_MS) return 0; + return entry.count; + } catch { + return 0; + } +} + +/** Test-only — wipe all state. Not exported under a stable name (prefixed `__`). */ +export function __resetComboFailureTrackerForTests(): void { + failureMap.clear(); +} diff --git a/open-sse/services/combo/pinRecovery.ts b/open-sse/services/combo/pinRecovery.ts new file mode 100644 index 0000000000..bb82a3e342 --- /dev/null +++ b/open-sse/services/combo/pinRecovery.ts @@ -0,0 +1,73 @@ +import type { ComboDiagnostics, ComboRecoveryHint } from "../../utils/error.ts"; + +/** + * Build the recovery hint that travels with a terminal combo failure. Lives + * alongside the diagnostic payload so the OpenCode plugin (and any other + * client) can render an actionable next-step instead of an opaque 5xx loop. + * + * The action verb is selected from the terminalReason the dispatcher already + * stamps onto ComboDiagnostics so this helper stays a pure projection — no new + * control flow, just a human-friendly next_step string per branch. + */ +export function buildRecoveryHint( + terminalReason: string, + retryAfterSeconds?: number +): ComboRecoveryHint { + switch (terminalReason) { + case "reasoning_budget_exhausted": + return { + action: "switch-combo", + next_step: + "Reasoning models consumed the output budget without emitting content. Increase max_tokens or pick a combo without a reasoning-heavy lead model.", + }; + case "max_attempts_exceeded": + return { + action: "try-auto", + next_step: + "Every candidate in this combo failed. Switch to model: auto to let OmniRoute pick a working provider, or pick a different combo.", + }; + case "all_accounts_inactive": + return { + action: "switch-combo", + next_step: + "No active accounts are connected for this combo. Open /dashboard/providers, reconnect at least one, then retry.", + }; + case "all_models_failed": + return { + action: "try-auto", + next_step: + "Every model in this combo failed. Switch to model: auto to let OmniRoute pick a working provider, or wait a few seconds for rate limits to recover.", + ...(typeof retryAfterSeconds === "number" && retryAfterSeconds > 0 + ? { retry_after_seconds: retryAfterSeconds } + : {}), + }; + case "no_executable_targets": + return { + action: "switch-combo", + next_step: + "This combo has no executable targets in the current account pool. Pick a different combo or reconnect the missing providers.", + }; + default: + return { + action: "retry", + next_step: + "The combo failed transiently. Retry the same combo, or switch to model: auto if the failure repeats.", + }; + } +} + +/** + * Diagnostics payload for the rare "combo routing completed without an + * upstream response" fallback — the dispatcher never crystallized a terminal + * status. Kept minimal (matches the original inline literal — no `recovery` + * field) so this extraction is a pure move, not a behavior change. + */ +export function buildNoUpstreamResponseDiagnostics(poolSize: number): ComboDiagnostics { + return { + poolSize, + attempted: 0, + excluded: [], + attemptOrder: [], + terminalReason: "no_upstream_response", + }; +} diff --git a/open-sse/utils/error.ts b/open-sse/utils/error.ts index be3de82dda..1530c5b66c 100644 --- a/open-sse/utils/error.ts +++ b/open-sse/utils/error.ts @@ -128,6 +128,37 @@ export function buildErrorBody( * "Add a sanitized combo diagnostic trace … candidate pool count, excluded * provider/model reasons, selected attempt order, terminal failure summary.") */ +export interface ComboExclusion { + provider: string; + model?: string; + reason: string; +} +/** + * Next-step suggestion surfaced when a combo cascade fails. Lets the client (e.g. + * the OpenCode plugin) auto-render an actionable hint in the TUI instead of an + * opaque "model stopped producing output" error — fixes the silent-stop pattern + * where the user has no way to recover a session without guessing. Whitelisted to + * a small set so the projection remains bounded. + */ +export type ComboRecoveryAction = + /** Cascade failed because every candidate is exhausted — try a different combo or `auto`. */ + | "try-auto" + /** Upstream asks to retry after a cooldown window — wait, then retry the same combo. */ + | "wait" + /** Transient failure (network, 5xx) — retry the same combo immediately. */ + | "retry" + /** Cascade used every account of every provider — switch to a different combo entirely. */ + | "switch-combo"; + +export interface ComboRecoveryHint { + /** Machine-readable action verb — consumed by clients to render a UI hint. */ + action: ComboRecoveryAction; + /** Seconds the client should wait before retrying. Only meaningful when action="wait". */ + retry_after_seconds?: number; + /** Human-readable next step — included verbatim in the error body for non-MCP clients. */ + next_step: string; +} + export interface ComboExclusion { provider: string; model?: string; @@ -139,6 +170,8 @@ export interface ComboDiagnostics { excluded: ComboExclusion[]; attemptOrder: Array<{ provider: string; model: string }>; terminalReason: string; + /** Optional next-step hint — populated when the dispatcher can recommend a recovery action. */ + recovery?: ComboRecoveryHint; } function clampDiagStr(v: unknown, max = 128): string { @@ -161,13 +194,43 @@ function toHeaderSafeAscii(v: string): string { return out; } +/** + * Whitelist sanitizer for the recovery hint. The `action` enum is a closed set; + * `retry_after_seconds` is clamped to a non-negative integer ≤ 3600; `next_step` is + * capped and stripped of CR/LF (would break header parsing). Returns undefined when + * no usable input was supplied so downstream code can branch cleanly on absence. + */ +const RECOVERY_ACTIONS = new Set([ + "try-auto", + "wait", + "retry", + "switch-combo", +]); +export function sanitizeRecoveryHint( + r: ComboRecoveryHint | null | undefined +): ComboRecoveryHint | undefined { + if (!r || typeof r !== "object") return undefined; + const action = typeof r.action === "string" ? (r.action as ComboRecoveryAction) : null; + if (!action || !RECOVERY_ACTIONS.has(action)) return undefined; + // Reject empty OR whitespace-only next_step — the value must render usefully as a + // header and as a body field. A whitespace-only string would print as a blank hint. + const next_step = clampDiagStr(r.next_step, 200).trim(); + if (!next_step) return undefined; + const hint: ComboRecoveryHint = { action, next_step }; + if (typeof r.retry_after_seconds === "number" && Number.isFinite(r.retry_after_seconds)) { + hint.retry_after_seconds = Math.max(0, Math.min(3600, Math.floor(r.retry_after_seconds))); + } + return hint; +} + /** * Whitelist projection — guarantees only id/reason string primitives + integer * counts can escape, regardless of what the caller assembled. This is the secret * containment boundary for the diagnostic trace. */ export function sanitizeComboDiagnostics(d: ComboDiagnostics): ComboDiagnostics { - return { + const recovery = sanitizeRecoveryHint(d?.recovery); + const out: ComboDiagnostics = { poolSize: Number.isFinite(d?.poolSize) ? d.poolSize : 0, attempted: Number.isFinite(d?.attempted) ? d.attempted : 0, excluded: (d?.excluded ?? []).slice(0, 64).map((e) => ({ @@ -180,6 +243,8 @@ export function sanitizeComboDiagnostics(d: ComboDiagnostics): ComboDiagnostics .map((a) => ({ provider: clampDiagStr(a?.provider, 64), model: clampDiagStr(a?.model, 96) })), terminalReason: clampDiagStr(d?.terminalReason, 200), }; + if (recovery) out.recovery = recovery; + return out; } /** @@ -187,7 +252,11 @@ export function sanitizeComboDiagnostics(d: ComboDiagnostics): ComboDiagnostics * `x-omniroute-combo-*` headers and a `diagnostics` field in the OpenAI-shaped * error body (extra field — backward-compatible with standard error parsers). * `opts.code`/`opts.type` override the status-derived defaults (e.g. to preserve - * the `ALL_ACCOUNTS_INACTIVE` code on the 503 terminal path). + * the `ALL_ACCOUNTS_INACTIVE` code on the 503 terminal path). When the diagnostic + * carries a `recovery` hint it is mirrored as `x-omniroute-recovery-action` / + * `x-omniroute-recovery-next-step` / `x-omniroute-retry-after-seconds` headers and as a + * top-level `recovery_hint` field on the body so non-header-aware clients (curl, + * MCP tools, log scrapers) can also pick it up. */ export function errorResponseWithComboDiagnostics( statusCode: number, @@ -198,25 +267,45 @@ export function errorResponseWithComboDiagnostics( const safe = sanitizeComboDiagnostics(diagnostics); const body = buildErrorBody(statusCode, message) as ErrorResponseBody & { diagnostics?: ComboDiagnostics; + recovery_hint?: ComboRecoveryHint; }; if (opts.code) body.error.code = opts.code; if (opts.type) body.error.type = opts.type; body.diagnostics = safe; + if (safe.recovery) body.recovery_hint = safe.recovery; const excludedHeader = toHeaderSafeAscii( safe.excluded .map((e) => `${e.provider}${e.model ? `/${e.model}` : ""}:${e.reason}`) .join(",") .slice(0, 900) ); + const headers: Record = { + "Content-Type": "application/json", + "x-omniroute-combo-pool-size": String(safe.poolSize), + "x-omniroute-combo-attempted": String(safe.attempted), + "x-omniroute-combo-excluded": excludedHeader, + "x-omniroute-combo-terminal-reason": toHeaderSafeAscii(safe.terminalReason.slice(0, 200)), + }; + + if (safe.recovery) { + headers["x-omniroute-recovery-action"] = safe.recovery.action; + // Header limit of 128 chars — keep next_step compact for fast parsing. + // The body field carries the full 200-char value for richer display. + headers["x-omniroute-recovery-next-step"] = toHeaderSafeAscii(safe.recovery.next_step).slice( + 0, + 128 + ); + if ( + typeof safe.recovery.retry_after_seconds === "number" && + safe.recovery.retry_after_seconds > 0 + ) { + headers["x-omniroute-retry-after-seconds"] = String(safe.recovery.retry_after_seconds); + } + } + return new Response(JSON.stringify(body), { status: statusCode, - headers: { - "Content-Type": "application/json", - "x-omniroute-combo-pool-size": String(safe.poolSize), - "x-omniroute-combo-attempted": String(safe.attempted), - "x-omniroute-combo-excluded": excludedHeader, - "x-omniroute-combo-terminal-reason": toHeaderSafeAscii(safe.terminalReason.slice(0, 200)), - }, + headers, }); } diff --git a/src/lib/db/contextHandoffs.ts b/src/lib/db/contextHandoffs.ts index 56575d5530..8bcd768e2b 100644 --- a/src/lib/db/contextHandoffs.ts +++ b/src/lib/db/contextHandoffs.ts @@ -231,3 +231,25 @@ export function clearSessionModelHistoryForCombo(comboName: string): number { .run(comboName); return result.changes ?? 0; } + +/** + * Clear the session model history pin for ONE session on a given combo, leaving + * every other session's pin on that same combo untouched. + * + * Unlike clearSessionModelHistoryForCombo() (combo-wide — used when the combo's + * model targets themselves change and every pin is stale), this is the + * session-scoped variant used by the consecutive-failure auto-clear path + * (open-sse/services/combo/failureTracker.ts::recordComboFailure), where only + * the failing session's pin should be dropped. + * + * @param sessionId - The session identifier whose pin should be cleared. + * @param comboName - The combo name. + * @returns The number of deleted entries. + */ +export function deleteSessionModelHistory(sessionId: string, comboName: string): number { + const db = getDbInstance() as unknown as DbLike; + const result = db + .prepare("DELETE FROM session_model_history WHERE session_id = ? AND combo_name = ?") + .run(sessionId, comboName); + return result.changes ?? 0; +} diff --git a/tests/unit/combo/combo-failure-tracker-session-isolation.test.ts b/tests/unit/combo/combo-failure-tracker-session-isolation.test.ts new file mode 100644 index 0000000000..7db23c6ce5 --- /dev/null +++ b/tests/unit/combo/combo-failure-tracker-session-isolation.test.ts @@ -0,0 +1,90 @@ +// tests/unit/combo/combo-failure-tracker-session-isolation.test.ts +// Regression guard: recordComboFailure()'s auto-clear must be scoped to the +// FAILING session only. Prior to this fix, failureTracker.ts called +// clearSessionModelHistoryForCombo(comboName) — a combo-WIDE delete with no +// session_id filter — so one session crossing the consecutive-failure +// threshold silently dropped every OTHER session's live pin on that same +// combo. See src/lib/db/contextHandoffs.ts::deleteSessionModelHistory (the +// session-scoped replacement) and open-sse/services/combo/failureTracker.ts. +// +// This exercises the real session_model_history table (not just the +// in-memory failure counter covered by combo-failure-tracker.test.ts), so it +// needs an isolated, throwaway DATA_DIR. process.env.DATA_DIR is set BEFORE +// the dynamic imports below (module-level DB constants are resolved once, at +// first import, in src/lib/db/core.ts) — this file intentionally avoids any +// static import of the DB-backed modules so it stays hermetic whether it is +// run standalone (`node --test `, no isolateDataDir --import) or as +// part of the full suite. +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-combo-failure-tracker-session-isolation-") +); +process.env.DATA_DIR = TEST_DATA_DIR; + +const core = await import("../../../src/lib/db/core.ts"); +const handoffDb = await import("../../../src/lib/db/contextHandoffs.ts"); +const failureTracker = await import("../../../open-sse/services/combo/failureTracker.ts"); + +test.after(() => { + core.resetDbInstance(); + fs.rmSync(TEST_DATA_DIR, { recursive: true, force: true }); +}); + +test("recordComboFailure clears only the failing session's pin, leaving other sessions on the same combo untouched", () => { + failureTracker.__resetComboFailureTrackerForTests(); + const comboName = "combo-session-isolation-probe"; + + // Seed pins for two different sessions sharing the SAME combo. + handoffDb.recordSessionModelUsage("sessA-probe", comboName, "openai/gpt-4o", "openai"); + handoffDb.recordSessionModelUsage("sessB-probe", comboName, "anthropic/claude", "anthropic"); + + // Sanity: both pins exist before any failure is recorded. + assert.equal(handoffDb.getLastSessionModel("sessA-probe", comboName), "openai/gpt-4o"); + assert.equal(handoffDb.getLastSessionModel("sessB-probe", comboName), "anthropic/claude"); + + // Only session A crosses the consecutive-failure threshold. + let lastResult = { count: 0, pinClearedNow: false }; + for (let i = 0; i < failureTracker.COMBO_FAILURE_THRESHOLD; i++) { + lastResult = failureTracker.recordComboFailure("sessA-probe", comboName); + } + assert.equal(lastResult.count, failureTracker.COMBO_FAILURE_THRESHOLD); + assert.equal(lastResult.pinClearedNow, true, "threshold-cross should report a pin clear"); + + // The failing session's pin is cleared... + assert.equal( + handoffDb.getLastSessionModel("sessA-probe", comboName), + null, + "the failing session's own pin should be cleared" + ); + // ...but the healthy, unrelated session's pin on the SAME combo survives. + assert.equal( + handoffDb.getLastSessionModel("sessB-probe", comboName), + "anthropic/claude", + "an unrelated session's pin on the same combo must NOT be dropped" + ); +}); + +test("recordComboFailure does not disturb an unrelated combo's pin for the SAME failing session", () => { + failureTracker.__resetComboFailureTrackerForTests(); + const failingCombo = "combo-session-isolation-failing"; + const otherCombo = "combo-session-isolation-other"; + + handoffDb.recordSessionModelUsage("sessC-probe", failingCombo, "openai/gpt-4o", "openai"); + handoffDb.recordSessionModelUsage("sessC-probe", otherCombo, "anthropic/claude", "anthropic"); + + for (let i = 0; i < failureTracker.COMBO_FAILURE_THRESHOLD; i++) { + failureTracker.recordComboFailure("sessC-probe", failingCombo); + } + + assert.equal(handoffDb.getLastSessionModel("sessC-probe", failingCombo), null); + assert.equal( + handoffDb.getLastSessionModel("sessC-probe", otherCombo), + "anthropic/claude", + "the same session's pin on a DIFFERENT combo must not be cleared" + ); +}); diff --git a/tests/unit/combo/combo-failure-tracker.test.ts b/tests/unit/combo/combo-failure-tracker.test.ts new file mode 100644 index 0000000000..18fe61f48f --- /dev/null +++ b/tests/unit/combo/combo-failure-tracker.test.ts @@ -0,0 +1,121 @@ +// tests/unit/combo/combo-failure-tracker.test.ts +// Unit tests for the per-session combo consecutive-failure tracker (failureTracker.ts). +// Covers: counting, auto-pin-clear at threshold (inner try-catch catches DB throw), +// TTL eviction, max-entries cap, fail-open on null/undefined session, and read-only peek. + +import { test } from "node:test"; +import assert from "node:assert/strict"; +import { + recordComboFailure, + clearComboFailureTracking, + getComboFailureCount, + __resetComboFailureTrackerForTests, + COMBO_FAILURE_THRESHOLD, +} from "../../../open-sse/services/combo/failureTracker.ts"; + +test("starts at 0 for an untouched session+combo pair", () => { + __resetComboFailureTrackerForTests(); + assert.equal(getComboFailureCount("s1", "combo-a"), 0); +}); + +test("null session returns 0 and doesn't store entries", () => { + __resetComboFailureTrackerForTests(); + const r = recordComboFailure(null, "combo-a"); + assert.equal(r.count, 0); + assert.equal(r.pinClearedNow, false); + assert.equal(getComboFailureCount(null, "combo-a"), 0); +}); + +test("undefined session returns 0 and doesn't store entries", () => { + __resetComboFailureTrackerForTests(); + const r = recordComboFailure(undefined, "combo-a"); + assert.equal(r.count, 0); + assert.equal(r.pinClearedNow, false); +}); + +test("increments count on each failure (1 -> 2 -> 3)", () => { + __resetComboFailureTrackerForTests(); + assert.equal(recordComboFailure("s1", "c1").count, 1); + assert.equal(recordComboFailure("s1", "c1").count, 2); + assert.equal(recordComboFailure("s1", "c1").count, 3); + assert.equal(getComboFailureCount("s1", "c1"), 3); +}); + +test("different combos have independent counters under the same session", () => { + __resetComboFailureTrackerForTests(); + recordComboFailure("s1", "combo-a"); + recordComboFailure("s1", "combo-a"); + recordComboFailure("s1", "combo-a"); + recordComboFailure("s1", "combo-b"); + recordComboFailure("s1", "combo-b"); + assert.equal(getComboFailureCount("s1", "combo-a"), 3); + assert.equal(getComboFailureCount("s1", "combo-b"), 2); +}); + +test("pinClearedNow is true on the first threshold-cross (COMBO_FAILURE_THRESHOLD)", () => { + __resetComboFailureTrackerForTests(); + for (let i = 1; i < COMBO_FAILURE_THRESHOLD; i++) { + const r = recordComboFailure("s1", "c1"); + assert.equal(r.pinClearedNow, false, `i=${i} must not clear pin`); + } + const threshold = recordComboFailure("s1", "c1"); + assert.equal(threshold.count, COMBO_FAILURE_THRESHOLD); + assert.equal(threshold.pinClearedNow, true); +}); + +test("pinClearedNow is true only once per streak", () => { + __resetComboFailureTrackerForTests(); + for (let i = 0; i < COMBO_FAILURE_THRESHOLD; i++) recordComboFailure("s1", "c1"); + const r = recordComboFailure("s1", "c1"); + assert.equal(r.count, COMBO_FAILURE_THRESHOLD + 1); + assert.equal(r.pinClearedNow, false); +}); + +test("clearComboFailureTracking resets the counter mid-streak", () => { + __resetComboFailureTrackerForTests(); + recordComboFailure("s1", "c1"); + recordComboFailure("s1", "c1"); + clearComboFailureTracking("s1", "c1"); + assert.equal(recordComboFailure("s1", "c1").count, 1); + assert.equal(getComboFailureCount("s1", "c1"), 1); +}); + +test("clearComboFailureTracking is a no-op for null/undefined session", () => { + __resetComboFailureTrackerForTests(); + clearComboFailureTracking(null, "c1"); + clearComboFailureTracking(undefined, "c1"); +}); + +test("getComboFailureCount handles unknown keys and TTL", () => { + __resetComboFailureTrackerForTests(); + recordComboFailure("s-ttl", "c1"); + assert.equal(getComboFailureCount("s-ttl", "c1"), 1); + assert.equal(getComboFailureCount("never-seen", "c1"), 0); +}); + +test("evict respects MAX_ENTRIES cap", () => { + __resetComboFailureTrackerForTests(); + const SESSIONS = 10; + for (let i = 0; i < SESSIONS; i++) { + recordComboFailure(`session-${i}`, "c1"); + } + for (let i = 0; i < SESSIONS; i++) { + assert.ok(getComboFailureCount(`session-${i}`, "c1") > 0, `session-${i} not tracked`); + } +}); + +test("combo-specific counters are independent across session+combo pairs", () => { + __resetComboFailureTrackerForTests(); + recordComboFailure("s1", "c1"); + recordComboFailure("s1", "c1"); + recordComboFailure("s2", "c1"); + assert.equal(getComboFailureCount("s1", "c1"), 2); + assert.equal(getComboFailureCount("s2", "c1"), 1); + assert.equal(getComboFailureCount("s1", "c2"), 0); +}); + +test("recordComboFailure does not throw at threshold (inner try-catch catches DB errors)", () => { + __resetComboFailureTrackerForTests(); + for (let i = 0; i <= COMBO_FAILURE_THRESHOLD; i++) recordComboFailure("s-ok", "c1"); + assert.equal(getComboFailureCount("s-ok", "c1"), COMBO_FAILURE_THRESHOLD + 1); +}); diff --git a/tests/unit/combo/pin-recovery.test.ts b/tests/unit/combo/pin-recovery.test.ts new file mode 100644 index 0000000000..a9b62d058a --- /dev/null +++ b/tests/unit/combo/pin-recovery.test.ts @@ -0,0 +1,75 @@ +// tests/unit/combo/pin-recovery.test.ts +// Direct unit coverage for open-sse/services/combo/pinRecovery.ts — extracted from +// combo.ts (file-size cap) so it needs its own direct test rather than relying only +// on indirect coverage through handleComboChat integration tests. + +import test from "node:test"; +import assert from "node:assert/strict"; +import { + buildRecoveryHint, + buildNoUpstreamResponseDiagnostics, +} from "../../../open-sse/services/combo/pinRecovery.ts"; + +test("buildRecoveryHint: reasoning_budget_exhausted maps to switch-combo", () => { + const hint = buildRecoveryHint("reasoning_budget_exhausted"); + assert.equal(hint.action, "switch-combo"); + assert.match(hint.next_step, /Increase max_tokens/); +}); + +test("buildRecoveryHint: max_attempts_exceeded maps to try-auto", () => { + const hint = buildRecoveryHint("max_attempts_exceeded"); + assert.equal(hint.action, "try-auto"); + assert.match(hint.next_step, /model: auto/); +}); + +test("buildRecoveryHint: all_accounts_inactive maps to switch-combo with dashboard hint", () => { + const hint = buildRecoveryHint("all_accounts_inactive"); + assert.equal(hint.action, "switch-combo"); + assert.match(hint.next_step, /dashboard\/providers/); +}); + +test("buildRecoveryHint: all_models_failed includes retry_after_seconds when positive", () => { + const hint = buildRecoveryHint("all_models_failed", 30); + assert.equal(hint.action, "try-auto"); + assert.equal(hint.retry_after_seconds, 30); +}); + +test("buildRecoveryHint: all_models_failed omits retry_after_seconds when undefined", () => { + const hint = buildRecoveryHint("all_models_failed"); + assert.equal(hint.action, "try-auto"); + assert.equal("retry_after_seconds" in hint, false); +}); + +test("buildRecoveryHint: all_models_failed omits retry_after_seconds when zero or negative", () => { + assert.equal("retry_after_seconds" in buildRecoveryHint("all_models_failed", 0), false); + assert.equal("retry_after_seconds" in buildRecoveryHint("all_models_failed", -5), false); +}); + +test("buildRecoveryHint: no_executable_targets maps to switch-combo", () => { + const hint = buildRecoveryHint("no_executable_targets"); + assert.equal(hint.action, "switch-combo"); + assert.match(hint.next_step, /no executable targets/); +}); + +test("buildRecoveryHint: unknown terminalReason falls back to retry", () => { + const hint = buildRecoveryHint("some_unrecognized_reason"); + assert.equal(hint.action, "retry"); + assert.match(hint.next_step, /transiently/); +}); + +test("buildNoUpstreamResponseDiagnostics: builds a minimal diagnostics payload from poolSize", () => { + const diag = buildNoUpstreamResponseDiagnostics(4); + assert.deepEqual(diag, { + poolSize: 4, + attempted: 0, + excluded: [], + attemptOrder: [], + terminalReason: "no_upstream_response", + }); +}); + +test("buildNoUpstreamResponseDiagnostics: zero poolSize is passed through as-is", () => { + const diag = buildNoUpstreamResponseDiagnostics(0); + assert.equal(diag.poolSize, 0); + assert.equal(diag.terminalReason, "no_upstream_response"); +}); diff --git a/tests/unit/combo/recovery-hint.test.ts b/tests/unit/combo/recovery-hint.test.ts new file mode 100644 index 0000000000..560edfa98c --- /dev/null +++ b/tests/unit/combo/recovery-hint.test.ts @@ -0,0 +1,132 @@ +// tests/unit/combo/recovery-hint.test.ts +// Tests for the `recovery` field on ComboDiagnostics and the +// errorResponseWithComboDiagnostics recovery header emission. + +import test from "node:test"; +import assert from "node:assert/strict"; +import { errorResponseWithComboDiagnostics } from "../../../open-sse/utils/error.ts"; +import type { ComboDiagnostics, RecoveryAction } from "../../../open-sse/utils/error.ts"; + +type JsonBody = { diagnostics?: { recovery?: Record } }; + +function emptyDiag(overrides?: Partial): ComboDiagnostics { + return { + combo_name: "my-combo", + strategy: "priority", + targets_tried: [], + last_status: 503, + last_error: "all targets failed", + terminal_reason: "max_attempts_exceeded", + ...overrides, + }; +} + +test("recovery with try-auto action is present in response body", async () => { + const diag = emptyDiag({ + recovery: { + action: "try-auto" as RecoveryAction, + next_step: "All exhausted. Use model: auto.", + }, + }); + const res = errorResponseWithComboDiagnostics(503, "exhausted", diag); + const body = (await res.json()) as JsonBody; + assert.ok(body.diagnostics?.recovery); + assert.equal(body.diagnostics?.recovery?.action, "try-auto"); + assert.equal(body.diagnostics?.recovery?.next_step, "All exhausted. Use model: auto."); +}); + +test("recovery with wait action and retry_after_seconds", async () => { + const diag = emptyDiag({ + recovery: { + action: "wait" as RecoveryAction, + next_step: "Rate limited.", + retry_after_seconds: 30, + }, + }); + const res = errorResponseWithComboDiagnostics(429, "rate limited", diag); + const body = (await res.json()) as JsonBody; + assert.equal(body.diagnostics?.recovery?.action, "wait"); + assert.equal(body.diagnostics?.recovery?.retry_after_seconds, 30); +}); + +test("recovery with switch-combo action", async () => { + const diag = emptyDiag({ + recovery: { action: "switch-combo" as RecoveryAction, next_step: "No healthy targets." }, + }); + const res = errorResponseWithComboDiagnostics(503, "no targets", diag); + const body = (await res.json()) as JsonBody; + assert.equal(body.diagnostics?.recovery?.action, "switch-combo"); +}); + +test("recovery hint omitted when recovery is undefined", async () => { + const diag = emptyDiag(); + const res = errorResponseWithComboDiagnostics(503, "generic", diag); + const body = (await res.json()) as JsonBody; + assert.equal(body.diagnostics?.recovery, undefined); +}); + +test("x-omniroute-recovery-action header emitted when recovery present", async () => { + const diag = emptyDiag({ + recovery: { + action: "try-auto" as RecoveryAction, + next_step: "Auto-select available providers.", + }, + }); + const res = errorResponseWithComboDiagnostics(503, "exhausted", diag); + assert.equal(res.headers.get("x-omniroute-recovery-action"), "try-auto"); + assert.equal( + res.headers.get("x-omniroute-recovery-next-step"), + "Auto-select available providers." + ); +}); + +test("x-omniroute-retry-after-seconds header emitted when retry_after_seconds set", async () => { + const diag = emptyDiag({ + recovery: { action: "wait" as RecoveryAction, next_step: "wait", retry_after_seconds: 45 }, + }); + const res = errorResponseWithComboDiagnostics(429, "rate limited", diag); + assert.equal(res.headers.get("x-omniroute-retry-after-seconds"), "45"); +}); + +test("x-omniroute-retry-after-seconds omitted when retry_after_seconds is 0", async () => { + const diag = emptyDiag({ + recovery: { action: "retry" as RecoveryAction, next_step: "retry", retry_after_seconds: 0 }, + }); + const res = errorResponseWithComboDiagnostics(503, "transient", diag); + assert.equal(res.headers.get("x-omniroute-retry-after-seconds"), null); +}); + +test("x-omniroute-recovery-* headers omitted when recovery is undefined", async () => { + const diag = emptyDiag(); + const res = errorResponseWithComboDiagnostics(503, "generic", diag); + assert.equal(res.headers.get("x-omniroute-recovery-action"), null); + assert.equal(res.headers.get("x-omniroute-recovery-next-step"), null); +}); + +test("next_step sanitized (newlines → spaces, max 128 chars)", async () => { + const longStep = "A".repeat(200) + "\nwith\nnewlines"; + const diag = emptyDiag({ + recovery: { action: "try-auto" as RecoveryAction, next_step: longStep }, + }); + const res = errorResponseWithComboDiagnostics(503, "exhausted", diag); + const sanitized = res.headers.get("x-omniroute-recovery-next-step")!; + assert.ok(sanitized.length <= 128); + assert.ok(!sanitized.includes("\n")); +}); + +test("drops recovery when next_step is whitespace-only", async () => { + const diag = emptyDiag({ recovery: { action: "try-auto" as RecoveryAction, next_step: " " } }); + const res = errorResponseWithComboDiagnostics(503, "exhausted", diag); + const body = (await res.json()) as JsonBody; + assert.equal(body.diagnostics?.recovery, undefined); + assert.equal(res.headers.get("x-omniroute-recovery-action"), null); +}); + +test("invalid action causes recovery to be dropped", async () => { + const diag = emptyDiag({ + recovery: { action: "garbage" as RecoveryAction, next_step: "something" }, + }); + const res = errorResponseWithComboDiagnostics(503, "exhausted", diag); + const body = (await res.json()) as JsonBody; + assert.equal(body.diagnostics?.recovery, undefined); +});