From bc7f68fb9179f75ec027b2a89f29569c19fd49c3 Mon Sep 17 00:00:00 2001 From: Innokentiy Solntsev Date: Thu, 17 Sep 2026 21:25:32 +0200 Subject: [PATCH] fix(resilience): lock the exact model, not the quota family, on 5xx model-lockout failures (#12957) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix(resilience): lock the exact model, not the quota family, on 5xx model-lockout failures A 5xx model-lockout failure — a transport error (terminated, EHOSTUNREACH, connect timeout), an upstream server error, or OmniRoute's own synthesized 502 from quality validation — is evidence about one model endpoint at that moment, not about the account's quota family. recordModelLockoutFailure() wrote it under the quota-family key regardless, so for codex (whose family key is the whole `codex` scope, i.e. every gpt-5* model) one empty stream on gpt-5.6-luna removed gpt-5.6-sol and gpt-5.6-terra from routing too, for 2–30 min with exponential escalation, while the quota was untouched. - exactModelLock.ts: resolveLockoutScope(status, explicit) — 429/403/402 (and 404, already narrowed by getModelLockKey) keep the family key; any other status uses the exact provider/connection/model key. An explicit `scope` option still wins. - recordModelLockoutFailure() resolves the scope once for key + lock fn. - decayModelFailureCount() now walks every key shape (family, not_found, exact) so success-decay reaches exact-scope locks; null model stays a no-op. - getAllModelLockouts() parses the `exact:` marker out of the key so the Model Cooldowns card lists the bare model and can clear it by that name. - docs: RESILIENCE_GUIDE §3 key-scope-by-status; changelog fragment. * chore(changelog): name the fragment after PR #12957 and link issue #12955 --------- Co-authored-by: insoln --- .../12957-model-lockout-5xx-exact-scope.md | 1 + docs/architecture/RESILIENCE_GUIDE.md | 18 +- open-sse/services/accountFallback.ts | 31 ++- .../accountFallback/exactModelLock.ts | 68 +++++++ stryker.conf.json | 1 + .../model-lockout-5xx-exact-scope.test.ts | 189 ++++++++++++++++++ 6 files changed, 288 insertions(+), 20 deletions(-) create mode 100644 changelog.d/fixes/12957-model-lockout-5xx-exact-scope.md create mode 100644 tests/unit/model-lockout-5xx-exact-scope.test.ts diff --git a/changelog.d/fixes/12957-model-lockout-5xx-exact-scope.md b/changelog.d/fixes/12957-model-lockout-5xx-exact-scope.md new file mode 100644 index 0000000000..bc704ab359 --- /dev/null +++ b/changelog.d/fixes/12957-model-lockout-5xx-exact-scope.md @@ -0,0 +1 @@ +- **fix(resilience):** A `5xx` model-lockout failure — a transport error (`terminated`, `EHOSTUNREACH`, connect timeout), an upstream server error, or OmniRoute's own synthesized `502` from quality validation — now locks only the exact provider/connection/model tuple instead of the quota family; previously one empty response on a single `gpt-5.6-*` model removed every `gpt-5*` model of the codex connection from routing for 2–30 min (escalating) while its quota was untouched. Quota statuses (`429`/`403`/`402`) keep the family scope; success-decay and the Model Cooldowns card now handle exact-scope locks too ([#12955](https://github.com/diegosouzapw/OmniRoute/issues/12955), [#12957](https://github.com/diegosouzapw/OmniRoute/pull/12957) — thanks @insoln) diff --git a/docs/architecture/RESILIENCE_GUIDE.md b/docs/architecture/RESILIENCE_GUIDE.md index a6d06c8309..b5196fedab 100644 --- a/docs/architecture/RESILIENCE_GUIDE.md +++ b/docs/architecture/RESILIENCE_GUIDE.md @@ -166,6 +166,21 @@ Related mechanisms remain separate: **Scope:** provider + connection + model triple. +**Key scope by status:** the failing status decides which key a lockout writes +to (`resolveLockoutScope()` in `open-sse/services/accountFallback/exactModelLock.ts`): + +- `429` / `403` / `402` — a quota or entitlement signal — lock the **quota family**: + for codex the whole `codex` / `spark` scope (every `gpt-5*` model of the + connection), for other providers `getQuotaScopedModelForProvider()`. +- `404` locks the bare model (`getModelLockKey()` narrows `not_found`). +- Any other status — `5xx` transport/server failures and OmniRoute's own + synthesized `502` from quality validation — locks the **exact** + provider/connection/model tuple only. A bad stream on one model is not evidence + about the account's quota; before this rule one empty response on + `codex/gpt-5.6-luna` removed every `gpt-5*` model of that connection from + routing for 2–30 min (escalating) while its quota was untouched. +- A caller's explicit `scope` option always wins (Antigravity passes `"exact"`). + **Purpose:** avoid disabling a whole connection when only one model is unavailable or quota-limited. **Examples:** @@ -224,7 +239,8 @@ escalation window. This success-decay is in addition to plain timer expiry — either path can re-enable a model. **State:** lockouts are held **in-memory** (per-process `Map`s of -`ModelLockoutEntry` keyed by `provider:connectionId:model`), not persisted to +`ModelLockoutEntry` keyed by `provider:connectionId:model`, exact-scope locks by +`provider:connectionId:exact:model`), not persisted to the DB — they are lost on restart. The _settings_ are persisted; the active lockout _state_ is ephemeral. diff --git a/open-sse/services/accountFallback.ts b/open-sse/services/accountFallback.ts index aa5a62f03f..6918f16733 100644 --- a/open-sse/services/accountFallback.ts +++ b/open-sse/services/accountFallback.ts @@ -851,6 +851,7 @@ export function recordModelLockoutFailure( options: { exactCooldownMs?: number | null; maxCooldownMs?: number; + /** Explicit override; otherwise resolveLockoutScope(status) — 5xx lock the exact tuple. */ scope?: "exact" | "quota_family"; /** * #6863 vs #7940: set true only when `exactCooldownMs` came from an actual @@ -864,8 +865,9 @@ export function recordModelLockoutFailure( } = {} ) { ensureCleanupTimer(); + const scope = exactModelLock.resolveLockoutScope(status, options.scope); const key = - options.scope === "exact" + scope === "exact" ? buildExactKey(getCanonicalLockProvider(provider), connectionId, model) : getModelLockKey(provider, connectionId, model, reason, status); const now = Date.now(); @@ -917,7 +919,7 @@ export function recordModelLockoutFailure( lastCooldownMs: cooldownMs, }); - const lockFn = options.scope === "exact" ? lockExactModel : lockModel; + const lockFn = scope === "exact" ? lockExactModel : lockModel; lockFn(provider, connectionId, model, reason, cooldownMs, { failureCount, lastFailureAt: now, @@ -1033,21 +1035,13 @@ export function decayModelFailureCount( connectionId: string, model: string ): DecayResult { - const key = getModelLockKey(provider, connectionId, model); - const failure = modelFailureState.get(key); - if (!failure) return { cleared: false, newFailureCount: 0 }; - - const newFailureCount = Math.floor(failure.failureCount / 2); - if (newFailureCount === 0) { - modelFailureState.delete(key); - return { cleared: true, newFailureCount: 0 }; - } else { - modelFailureState.set(key, { - ...failure, - failureCount: newFailureCount, - }); - return { cleared: false, newFailureCount }; - } + if (!model) return { cleared: false, newFailureCount: 0 }; + // Every key shape: a 5xx lock lives under the exact key, a quota lock under the + // family key — a healthy response must walk back whichever one is escalating. + return exactModelLock.decayFailureCounts( + modelFailureState, + getModelLockKeys(provider, connectionId, model) + ); } /** @@ -1119,8 +1113,7 @@ export function getAllModelLockouts(): ModelLockoutInfo[] { cleanupModelLockKey(key, now); } for (const [key, entry] of modelLockouts) { - const [provider, connectionId, ...modelParts] = key.split(":"); - const model = modelParts.join(":"); + const { provider, connectionId, model } = exactModelLock.parseModelLockKey(key); active.push({ provider, connectionId, diff --git a/open-sse/services/accountFallback/exactModelLock.ts b/open-sse/services/accountFallback/exactModelLock.ts index 838d6a9173..280a3c64f6 100644 --- a/open-sse/services/accountFallback/exactModelLock.ts +++ b/open-sse/services/accountFallback/exactModelLock.ts @@ -156,3 +156,71 @@ export function createLockExactModel( if (next) modelLockouts.set(key, next); }; } + +/** Which key namespace a lockout writes to — see resolveLockoutScope(). */ +export type LockoutScope = "exact" | "quota_family"; + +/** + * Statuses that are evidence about the account's quota / entitlement and therefore + * lock the quota family (codex: the whole `codex` / `spark` scope; other providers: + * getQuotaScopedModelForProvider). 404 stays on this side only because + * getModelLockKey() already narrows a not_found lock to the bare model. + */ +const QUOTA_FAMILY_LOCKOUT_STATUSES: ReadonlySet = new Set([402, 403, 404, 429]); + +/** + * A 5xx — a transport failure (`terminated`, EHOSTUNREACH, connect timeout), an + * upstream server error, or OmniRoute's own synthesized 502 from quality + * validation — says something about one model endpoint at that moment, not about + * the account's quota family. Locking the family on it let a single empty stream + * on one `gpt-5.6-*` model remove every `gpt-5*` model of the codex connection + * from routing for 2–30 min (escalating) while its quota was untouched. Such + * failures lock the exact provider/connection/model tuple instead. A caller's + * explicit `scope` always wins (Antigravity passes "exact" for its own reasons). + */ +export function resolveLockoutScope(status: number, explicit?: LockoutScope): LockoutScope { + if (explicit) return explicit; + return QUOTA_FAMILY_LOCKOUT_STATUSES.has(status) ? "quota_family" : "exact"; +} + +/** Split a `provider:connectionId:[exact:]model` key back into the parts the dashboard lists. */ +export function parseModelLockKey(key: string): { + provider: string; + connectionId: string; + model: string; + scope: LockoutScope; +} { + const [provider, connectionId, ...modelParts] = key.split(":"); + const scope: LockoutScope = modelParts[0] === "exact" ? "exact" : "quota_family"; + const model = (scope === "exact" ? modelParts.slice(1) : modelParts).join(":"); + return { provider, connectionId, model, scope }; +} + +/** + * Success-decay across every key shape (quota-family, not_found, exact): halve each + * stored failureCount, dropping the entry once it reaches 0. `cleared` is true only + * when every entry that existed was dropped; `newFailureCount` is the largest count + * still stored. + */ +export function decayFailureCounts( + modelFailureState: Map, + keys: string[] +): { cleared: boolean; newFailureCount: number } { + let seen = 0; + let dropped = 0; + let newFailureCount = 0; + for (const key of keys) { + const failure = modelFailureState.get(key); + if (!failure) continue; + seen += 1; + const next = Math.floor(failure.failureCount / 2); + if (next === 0) { + modelFailureState.delete(key); + dropped += 1; + } else { + modelFailureState.set(key, { ...failure, failureCount: next }); + newFailureCount = Math.max(newFailureCount, next); + } + } + return { cleared: seen > 0 && dropped === seen, newFailureCount }; +} diff --git a/stryker.conf.json b/stryker.conf.json index eb10733040..83a6099e74 100644 --- a/stryker.conf.json +++ b/stryker.conf.json @@ -310,6 +310,7 @@ "tests/unit/model-cooldowns-route.test.ts", "tests/unit/model-lockout-decay.test.ts", "tests/unit/model-lockout-exact-cooldown-cap.test.ts", + "tests/unit/model-lockout-5xx-exact-scope.test.ts", "tests/unit/model-lockout-max-cooldown.test.ts", "tests/unit/native-codex-turn-pin-10379.test.ts", "tests/unit/native-codex-turn-pin-model-scoped-fallback.test.ts", diff --git a/tests/unit/model-lockout-5xx-exact-scope.test.ts b/tests/unit/model-lockout-5xx-exact-scope.test.ts new file mode 100644 index 0000000000..d008b8e84d --- /dev/null +++ b/tests/unit/model-lockout-5xx-exact-scope.test.ts @@ -0,0 +1,189 @@ +/** + * Model lockout scope by status: a 5xx (transport failure, upstream server error, + * or OmniRoute's own synthesized 502 from quality validation) locks the exact + * provider/connection/model tuple, never the quota family — a single bad stream + * on one codex model must not remove every `gpt-5*` model of the connection from + * routing. Quota / entitlement statuses (429/403/402) keep the family scope. + * + * Harness mirrors tests/unit/model-lockout-max-cooldown.test.ts (temp DATA_DIR, + * real handleComboChat with a mocked handleSingleModel). + */ +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"; +import type { ComboLogger } from "../../open-sse/services/combo/types.ts"; + +const TEST_DATA_DIR = fs.mkdtempSync(path.join(os.tmpdir(), "omr-lockout-5xx-scope-")); +process.env.DATA_DIR = TEST_DATA_DIR; +process.env.API_KEY_SECRET = "test-lockout-5xx-scope-secret"; // pragma: allowlist secret + +const core = await import("../../src/lib/db/core.ts"); +const { handleComboChat } = await import("../../open-sse/services/combo.ts"); +const { + recordModelLockoutFailure, + isModelLocked, + getModelLockoutInfo, + getAllModelLockouts, + clearModelLock, + decayModelFailureCount, + clearAllModelLockouts, +} = await import("../../open-sse/services/accountFallback.ts"); +const { resolveLockoutScope, parseModelLockKey } = + await import("../../open-sse/services/accountFallback/exactModelLock.ts"); + +const CONN = "conn-codex-1"; +const LUNA = "gpt-5.6-luna"; +const SOL = "gpt-5.6-sol"; +const TERRA = "gpt-5.6-terra"; + +test.beforeEach(() => clearAllModelLockouts()); +test.after(() => { + clearAllModelLockouts(); + try { + core.resetDbInstance(); + fs.rmSync(TEST_DATA_DIR, { recursive: true, force: true, maxRetries: 5, retryDelay: 100 }); + } catch {} +}); + +test("resolveLockoutScope: quota statuses → family, everything else → exact, explicit wins", () => { + for (const status of [429, 403, 402, 404]) { + assert.equal(resolveLockoutScope(status), "quota_family", `status ${status}`); + } + for (const status of [500, 502, 503, 504, 520, 400]) { + assert.equal(resolveLockoutScope(status), "exact", `status ${status}`); + } + assert.equal(resolveLockoutScope(502, "quota_family"), "quota_family"); + assert.equal(resolveLockoutScope(429, "exact"), "exact"); +}); + +test("codex 502 on one model locks only that model — sibling gpt-5* models stay routable", () => { + const lock = recordModelLockoutFailure( + "codex", + CONN, + LUNA, + "quality_failure", + 502, + 120_000, + null, + { maxCooldownMs: 1_800_000 } + ); + assert.ok(lock.cooldownMs > 0); + assert.equal(isModelLocked("codex", CONN, LUNA), true, "the failing model is locked"); + assert.equal(isModelLocked("codex", CONN, SOL), false, "sibling scope member stays routable"); + assert.equal(isModelLocked("codex", CONN, TERRA), false); + assert.equal(isModelLocked("cx", CONN, SOL), false, "alias spelling agrees"); +}); + +test("codex 429 on one model still locks the whole quota scope (unchanged)", () => { + recordModelLockoutFailure("codex", CONN, LUNA, "rate_limit", 429, 120_000, null, { + maxCooldownMs: 1_800_000, + }); + assert.equal(isModelLocked("codex", CONN, LUNA), true); + assert.equal(isModelLocked("codex", CONN, SOL), true, "429 is a scope-wide quota signal"); + assert.equal(isModelLocked("codex", CONN, TERRA), true); +}); + +test("explicit scope option overrides the status default", () => { + recordModelLockoutFailure("codex", CONN, LUNA, "unknown", 502, 120_000, null, { + maxCooldownMs: 1_800_000, + scope: "quota_family", + }); + assert.equal(isModelLocked("codex", CONN, SOL), true, "caller asked for the family"); +}); + +test("exact 5xx lock keeps escalating per failure and decays on success", () => { + const originalNow = Date.now; + try { + let fakeNow = Date.now(); + Date.now = () => fakeNow; + const first = recordModelLockoutFailure("codex", CONN, LUNA, "unknown", 502, 1000, null, { + maxCooldownMs: 60_000, + }); + fakeNow += 1100; + const second = recordModelLockoutFailure("codex", CONN, LUNA, "unknown", 502, 1000, null, { + maxCooldownMs: 60_000, + }); + assert.equal(first.failureCount, 1); + assert.equal(second.failureCount, 2); + assert.equal(second.cooldownMs, 2000, "exponential backoff applies to the exact key too"); + assert.equal(getModelLockoutInfo("codex", CONN, LUNA)?.failureCount, 2); + + const decayed = decayModelFailureCount("codex", CONN, LUNA); + assert.deepEqual(decayed, { cleared: false, newFailureCount: 1 }); + const cleared = decayModelFailureCount("codex", CONN, LUNA); + assert.deepEqual(cleared, { cleared: true, newFailureCount: 0 }); + assert.deepEqual(decayModelFailureCount("codex", CONN, LUNA), { + cleared: false, + newFailureCount: 0, + }); + } finally { + Date.now = originalNow; + } +}); + +test("dashboard listing shows the bare model for an exact lock and can clear it by that name", () => { + recordModelLockoutFailure("codex", CONN, LUNA, "unknown", 503, 120_000, null, { + maxCooldownMs: 1_800_000, + }); + const listed = getAllModelLockouts().filter((l) => l.connectionId === CONN); + assert.equal(listed.length, 1); + assert.equal(listed[0].provider, "codex"); + assert.equal(listed[0].model, LUNA, "no `exact:` marker leaks into the listing"); + assert.equal(clearModelLock("codex", CONN, listed[0].model), true); + assert.equal(isModelLocked("codex", CONN, LUNA), false); + assert.deepEqual(parseModelLockKey("codex:c1:exact:gpt-5.6-luna"), { + provider: "codex", + connectionId: "c1", + model: "gpt-5.6-luna", + scope: "exact", + }); + assert.deepEqual(parseModelLockKey("codex:c1:codex"), { + provider: "codex", + connectionId: "c1", + model: "codex", + scope: "quota_family", + }); +}); + +test("handleComboChat: a 502 on one codex model leaves a sibling combo on the same scope dispatchable", async () => { + const settings = { + modelLockout: { + enabled: true, + errorCodes: [502], + baseCooldownMs: 120_000, + maxCooldownMs: 1_800_000, + maxBackoffSteps: 10, + useExponentialBackoff: true, + }, + }; + const log = { info: () => {}, warn: () => {}, error: () => {}, debug: () => {} }; + const run = (model: string, status: number) => + handleComboChat({ + body: {}, + combo: { + name: `scope-${model}`, + strategy: "priority", + models: [`codex/${model}`], + config: { maxRetries: 0, retryDelayMs: 0, fallbackDelayMs: 0 }, + }, + handleSingleModel: async () => + new Response(JSON.stringify(status === 200 ? { ok: true } : { error: { message: "x" } }), { + status, + headers: { "content-type": "application/json" }, + }), + isModelAvailable: async () => true, + log: log as unknown as ComboLogger, + settings, + allCombos: null, + }); + + const failed = await run(LUNA, 502); + assert.notEqual(failed.status, 200); + assert.equal(isModelLocked("codex", "", LUNA), true, "the failing model is locked"); + assert.equal(isModelLocked("codex", "", SOL), false, "sibling is not"); + + const ok = await run(SOL, 200); + assert.equal(ok.status, 200, "the sibling model still dispatches"); +});