diff --git a/changelog.d/fixes/7268-model-not-supported-401-lockout.md b/changelog.d/fixes/7268-model-not-supported-401-lockout.md new file mode 100644 index 0000000000..d4ae8888b9 --- /dev/null +++ b/changelog.d/fixes/7268-model-not-supported-401-lockout.md @@ -0,0 +1 @@ +- fix(sse): classify 401 "model X is not supported" as model-not-found so it locks the model out instead of looping forever (#7268) diff --git a/changelog.d/fixes/7387-sticky-quota-exhausted.md b/changelog.d/fixes/7387-sticky-quota-exhausted.md new file mode 100644 index 0000000000..1273c78a37 --- /dev/null +++ b/changelog.d/fixes/7387-sticky-quota-exhausted.md @@ -0,0 +1 @@ +- fix(sse): combo session stickiness now releases a connection whose per-window quota is exhausted, matching the provider-level session-affinity pin (#7387) diff --git a/open-sse/services/combo/sessionStickiness.ts b/open-sse/services/combo/sessionStickiness.ts index 2b9f746133..d6d6eb06dd 100644 --- a/open-sse/services/combo/sessionStickiness.ts +++ b/open-sse/services/combo/sessionStickiness.ts @@ -35,6 +35,15 @@ * the same dynamic-import-with-injectable-override seam (fail-open on lookup * errors, mirroring resolveSaturation) and gates the pin alongside headroom. * For tests the fetcher is injected via __setStickinessConnectionFetcherForTests. + * • Quota-exhaustion gate (#7387): testStatus/rateLimitedUntil alone still + * miss a connection whose 5h/weekly quota window is depleted but that + * hasn't (yet) received a hard failure severe enough to flip either field — + * exactly what a quota-preflight/dashboard-detected depletion looks like + * before any upstream 429 lands for this run. isAccountQuotaExhausted() + * (src/domain/quotaCache.ts) is the authoritative per-window signal the rest + * of the credential-selection pipeline already gates on (auth.ts, + * sessionAffinityPin.ts); it now also releases the combo-level sticky pin. + * For tests the checker is injected via __setStickinessQuotaCheckerForTests. * * No barrel import — consistent with the other combo/* helpers. * @@ -164,6 +173,51 @@ export function isStickyConnectionTerminallyUnhealthy( return Number.isFinite(rl) && rl > now; } +// ─── Per-window quota-exhaustion gate (#7387) ──────────────────────────────── + +/** + * Injectable quota-exhaustion checker seam (for unit tests that don't want to + * hydrate the real in-memory quota cache). + */ +export type QuotaExhaustionChecker = (connectionId: string) => boolean; + +let _quotaExhaustionOverride: QuotaExhaustionChecker | null = null; + +/** Test-only: inject the quota-exhaustion checker; pass null to restore default. */ +export function __setStickinessQuotaCheckerForTests( + checker: QuotaExhaustionChecker | null +): void { + _quotaExhaustionOverride = checker; +} + +/** + * Is the sticky-bound connection's per-window (5h/weekly) quota exhausted? + * + * `isStickyConnectionTerminallyUnhealthy` above only looks at testStatus/ + * rateLimitedUntil (#6692) — it misses a connection whose quota window is + * fully depleted (per src/domain/quotaCache.ts::isAccountQuotaExhausted, the + * same authoritative per-window signal src/sse/services/auth.ts and + * sessionAffinityPin.ts already gate on) but that hasn't yet received a hard + * failure severe enough to flip testStatus or set rateLimitedUntil. Without + * this check the combo-level sticky pin re-promotes the depleted account on + * every request, defeating whatever strategy picked a healthy one. (#7387) + * + * Dynamic import (mirroring resolveConnectionHealth/resolveSaturation above) + * so this open-sse/ leaf keeps no static edge into src/domain/. Fail-open + * (false) on any lookup error — an unresolved check must never drop a + * healthy pin. + */ +async function isStickyConnectionQuotaExhausted(connectionId: string): Promise { + if (_quotaExhaustionOverride) return _quotaExhaustionOverride(connectionId); + + try { + const mod = await import("../../../src/domain/quotaCache"); + return Boolean(mod.isAccountQuotaExhausted(connectionId)); + } catch { + return false; + } +} + /** * Resolve the HeadroomSaturation for a connection by fetching both the 5h and * weekly utilisation signals. Uses the same dynamic-import pattern as @@ -374,15 +428,17 @@ export async function applySessionStickiness( // accounts report healthy 5h/weekly utilization, so headroom alone never // catches them). const stickyTarget = orderedTargets[stickyIdx]; - const [sat, connHealth] = await Promise.all([ + const [sat, connHealth, quotaExhausted] = await Promise.all([ resolveSaturation(connectionId, stickyTarget.provider), resolveConnectionHealth(connectionId, stickyTarget.provider), + isStickyConnectionQuotaExhausted(connectionId), ]); const headroom = computeHeadroom(sat); if ( headroom <= STICKINESS_HEADROOM_THRESHOLD || - isStickyConnectionTerminallyUnhealthy(connHealth, Date.now()) + isStickyConnectionTerminallyUnhealthy(connHealth, Date.now()) || + quotaExhausted ) { // Connection saturated or durably unhealthy — rebind on next success clearStickyBinding(messageHash); diff --git a/open-sse/services/errorClassifier.ts b/open-sse/services/errorClassifier.ts index 3d978fec4c..d3765737d0 100644 --- a/open-sse/services/errorClassifier.ts +++ b/open-sse/services/errorClassifier.ts @@ -99,6 +99,19 @@ export function isContextOverflow(errorText: string): boolean { return CONTEXT_OVERFLOW_REGEX.test(String(errorText || "")); } +// Matches phrasing like `Model minimax-m3-free is not supported` or +// `model "gpt-9" is not supported` — free-tier/aggregator providers name the +// specific model in the sentence instead of using a fixed fragment like +// "model not supported". Shared by modelFamilyFallback.ts's +// isModelUnavailableError() (400/403/404) and this module's 401 branch below, +// so the same phrasing locks the model out on either status. Bounded +// quantifier ({0,80}) keeps it ReDoS-safe. (#7268) +const MODEL_NAMED_UNSUPPORTED_REGEX = /\bmodel\b[^\n]{0,80}\bis not supported\b/i; + +export function containsModelUnavailableMessage(errorMessage: string): boolean { + return MODEL_NAMED_UNSUPPORTED_REGEX.test(String(errorMessage || "").toLowerCase()); +} + function responseBodyToString(responseBody: unknown): string { if (typeof responseBody === "string") return responseBody; if (responseBody !== null && typeof responseBody === "object") { @@ -158,6 +171,16 @@ export function classifyProviderError( if (oauthInvalid) { return PROVIDER_ERROR_TYPES.OAUTH_INVALID_TOKEN; } + // Some free-tier/aggregator providers return 401 (instead of 404) for a + // model the account isn't entitled to, with a body like "Model X is not + // supported". Without this check the error falls through to a generic + // UNAUTHORIZED classification, which never triggers lockModel() in + // chatCore.ts — auto-combo keeps re-selecting the same broken model on + // every request. Detect the phrasing here, same as the 404 branch above + // always does regardless of body content. (#7268) + if (containsModelUnavailableMessage(bodyStr)) { + return PROVIDER_ERROR_TYPES.MODEL_NOT_FOUND; + } return accountDeactivated ? PROVIDER_ERROR_TYPES.ACCOUNT_DEACTIVATED : PROVIDER_ERROR_TYPES.UNAUTHORIZED; diff --git a/open-sse/services/modelFamilyFallback.ts b/open-sse/services/modelFamilyFallback.ts index 16ef338874..81f569764e 100644 --- a/open-sse/services/modelFamilyFallback.ts +++ b/open-sse/services/modelFamilyFallback.ts @@ -13,7 +13,7 @@ import { getModelContextLimit } from "../../src/lib/modelCapabilities"; import { parseModel } from "./model.ts"; -import { CONTEXT_OVERFLOW_REGEX } from "./errorClassifier.ts"; +import { CONTEXT_OVERFLOW_REGEX, containsModelUnavailableMessage } from "./errorClassifier.ts"; import { getRegistryEntry } from "../config/providerRegistry.ts"; // ── Model Family Definitions ───────────────────────────────────────────────── @@ -129,7 +129,8 @@ export function isModelUnavailableError(status: number, errorMessage: string): b if (status !== 400 && status !== 403) return false; const msg = errorMessage.toLowerCase(); - return MODEL_UNAVAILABLE_FRAGMENTS.some((fragment) => msg.includes(fragment)); + if (MODEL_UNAVAILABLE_FRAGMENTS.some((fragment) => msg.includes(fragment))) return true; + return containsModelUnavailableMessage(errorMessage); } export function isContextOverflowError(status: number, errorMessage: string): boolean { diff --git a/tests/unit/repro-7268-401-model-not-supported-lockout.test.ts b/tests/unit/repro-7268-401-model-not-supported-lockout.test.ts new file mode 100644 index 0000000000..f98a42090c --- /dev/null +++ b/tests/unit/repro-7268-401-model-not-supported-lockout.test.ts @@ -0,0 +1,48 @@ +/** + * TDD repro/regression test for issue #7268 — "Model X is not supported" + * 401 responses never lock the model out. + * + * Root cause: classifyProviderError() only inspects the response body for + * status codes 400/403/404 to detect a model-unavailable signal. For status + * 401 it only checks isOAuthInvalidToken()/isAccountDeactivated() and falls + * through to a generic UNAUTHORIZED classification — even when the body + * literally says "Model X is not supported". Because chatCore.ts only calls + * lockModel(..., "model_not_found", ...) on the MODEL_NOT_FOUND branch, the + * broken model is never locked out and auto-combo keeps re-selecting it. + * + * Expected (correct) behavior: a 401 whose body matches a model-unavailable + * fragment (e.g. " is not supported") classifies as MODEL_NOT_FOUND, + * the same way a 404 always does. + */ + +import test from "node:test"; +import assert from "node:assert/strict"; + +const { classifyProviderError, PROVIDER_ERROR_TYPES } = await import( + "../../open-sse/services/errorClassifier.ts" +); +const { isModelUnavailableError } = await import( + "../../open-sse/services/modelFamilyFallback.ts" +); + +test("#7268: classifyProviderError(401, 'Model X is not supported') classifies as MODEL_NOT_FOUND", () => { + const classified = classifyProviderError(401, { error: "Model minimax-m3-free is not supported" }); + assert.equal(classified, PROVIDER_ERROR_TYPES.MODEL_NOT_FOUND); +}); + +test("#7268: classifyProviderError(401, 'Model X is not supported') for a different model name", () => { + const classified = classifyProviderError(401, { error: "Model qwen3.6-plus-free is not supported" }); + assert.equal(classified, PROVIDER_ERROR_TYPES.MODEL_NOT_FOUND); +}); + +test("#7268: a genuine 401 auth error (no model-unavailable wording) stays UNAUTHORIZED", () => { + const classified = classifyProviderError(401, { error: "Invalid API key provided" }); + assert.equal(classified, PROVIDER_ERROR_TYPES.UNAUTHORIZED); +}); + +test("#7268: isModelUnavailableError() recognizes the literal ' is not supported' phrase", () => { + assert.equal( + isModelUnavailableError(400, "Model minimax-m3-free is not supported"), + true + ); +}); diff --git a/tests/unit/repro-7387-sticky-quota-exhausted.test.ts b/tests/unit/repro-7387-sticky-quota-exhausted.test.ts new file mode 100644 index 0000000000..0f09c1851b --- /dev/null +++ b/tests/unit/repro-7387-sticky-quota-exhausted.test.ts @@ -0,0 +1,108 @@ +/** + * TDD repro/regression test for issue #7387 — combo-level session stickiness + * (open-sse/services/combo/sessionStickiness.ts) never checks per-window + * quota exhaustion (src/domain/quotaCache.ts::isAccountQuotaExhausted) before + * re-promoting a bound connection back to position 0 of the target list. + * + * The provider-level session-affinity pin (src/sse/services/sessionAffinityPin.ts) + * already gates on isAccountQuotaExhausted() correctly — sessionStickiness.ts + * is the one place that forgot it, only checking testStatus + * (credits_exhausted/banned/expired) and rateLimitedUntil. + * + * Expected (correct) behavior: once a sticky-bound connection's quota is + * exhausted (per quotaCache, independent of testStatus/rateLimitedUntil), the + * pin must release and the healthy target takes position 0. + */ + +import test from "node:test"; +import assert from "node:assert/strict"; +import type { HeadroomSaturation } from "../../open-sse/services/combo/headroomRanking.ts"; +import type { StickyConnectionHealth } from "../../open-sse/services/combo/sessionStickiness.ts"; + +const stickinessMod = await import("../../open-sse/services/combo/sessionStickiness.ts"); +const { + deriveMessageHash, + applySessionStickiness, + recordStickyBinding, + clearAllStickyBindings, + __setStickinessHeadroomFetcherForTests, + __setStickinessConnectionFetcherForTests, +} = stickinessMod; + +const quotaCacheMod = await import("../../src/domain/quotaCache.ts"); +const { setQuotaCache, isAccountQuotaExhausted, __clearForTests } = quotaCacheMod; + +function makeTarget(connectionId: string) { + return { + kind: "model", + stepId: `step-${connectionId}`, + executionKey: `key-${connectionId}`, + modelStr: `codex/gpt-5-codex/${connectionId}`, + provider: "codex", + providerId: null, + connectionId, + weight: 1, + label: null, + }; +} + +function injectSat(sat: HeadroomSaturation | undefined) { + __setStickinessHeadroomFetcherForTests(async (_id: string) => sat); +} + +function injectConnectionHealth(byId: Record) { + __setStickinessConnectionFetcherForTests(async (connectionId: string) => byId[connectionId]); +} + +test.beforeEach(() => { + clearAllStickyBindings(); + __clearForTests(); +}); + +test.after(() => { + __setStickinessHeadroomFetcherForTests(null); + __setStickinessConnectionFetcherForTests(null); + __clearForTests(); +}); + +test("#7387: sticky pin releases a QUOTA-EXHAUSTED account whose testStatus/rateLimitedUntil are still healthy", async () => { + injectSat({ util5h: 0.05, util7d: 0.05 }); // headroom well above threshold + injectConnectionHealth({ + "conn-codex-exhausted": { testStatus: "active", rateLimitedUntil: null }, + }); + + setQuotaCache("conn-codex-exhausted", "codex", { + session: { remainingPercentage: 0, resetAt: null }, + weekly: { remainingPercentage: 0, resetAt: null }, + }); + assert.equal(isAccountQuotaExhausted("conn-codex-exhausted"), true); + + const targets = [makeTarget("conn-healthy"), makeTarget("conn-codex-exhausted")]; + const messages = [{ role: "user", content: "Multi-turn Codex conversation, turn 1" }]; + const hash = deriveMessageHash(messages)!; + + recordStickyBinding(hash, "conn-codex-exhausted"); // turn 1: served successfully + + const result = await applySessionStickiness(targets, messages); // turn 2+: quota now exhausted + + assert.equal(result.stuck, false, "sticky pin must release once quota is exhausted (#7387)"); + assert.equal(result.targets[0].connectionId, "conn-healthy"); +}); + +test("#7387: sticky pin stays bound when the connection is healthy and NOT quota-exhausted", async () => { + injectSat({ util5h: 0.05, util7d: 0.05 }); + injectConnectionHealth({ + "conn-codex-ok": { testStatus: "active", rateLimitedUntil: null }, + }); + + const targets = [makeTarget("conn-other"), makeTarget("conn-codex-ok")]; + const messages = [{ role: "user", content: "Multi-turn Codex conversation, turn 1 (healthy)" }]; + const hash = deriveMessageHash(messages)!; + + recordStickyBinding(hash, "conn-codex-ok"); + + const result = await applySessionStickiness(targets, messages); + + assert.equal(result.stuck, true); + assert.equal(result.targets[0].connectionId, "conn-codex-ok"); +});