From 1110f9ca5f0f2f407214438ecd758f8e0da0be6a Mon Sep 17 00:00:00 2001 From: Ravi Tharuma <25951435+RaviTharuma@users.noreply.github.com> Date: Thu, 23 Jul 2026 14:50:41 +0200 Subject: [PATCH] fix(combo): advance on model-scoped 400s wrapped as invalid/Bad Request (#8252) #2101 still hard-stopped model-not-supported failures when upstream wrapped them as invalid_request_error / Bad Request. Keep models in the combo and try the next target (#8251, residual of #5249). - export MODEL_ACCESS_DENIED_PATTERNS + broaden does-not-support shapes - add isModelScoped400() and exempt it from the body-specific stop guard - regression tests for wrapper forms; body-specific stop still intact Co-authored-by: Ravi Tharuma --- open-sse/services/accountFallback.ts | 10 +- open-sse/services/combo.ts | 32 ++++- .../combo-model-scoped-400-advance.test.ts | 133 ++++++++++++++++++ ...mbo-param-validation-fallback-4519.test.ts | 12 +- 4 files changed, 182 insertions(+), 5 deletions(-) create mode 100644 tests/unit/combo-model-scoped-400-advance.test.ts diff --git a/open-sse/services/accountFallback.ts b/open-sse/services/accountFallback.ts index 16cfd2590c..6e8bb22a44 100644 --- a/open-sse/services/accountFallback.ts +++ b/open-sse/services/accountFallback.ts @@ -241,10 +241,18 @@ const MODEL_ACCESS_AMBIGUOUS_TYPES = new Set([ // Model access patterns — the account does not have access to the requested model // but a different account (e.g. PRO vs free tier) may support it. -const MODEL_ACCESS_DENIED_PATTERNS = [ +// Exported so combo.ts #2101 can exempt model-scoped 400s from the body-specific +// stop guard (#5249): "model not supported" must advance to the next combo target +// even when the message also contains wrapper words like "invalid" / "bad request". +export const MODEL_ACCESS_DENIED_PATTERNS = [ /\binvalid model\b/i, /\bmodel.*not.*(?:available|found|supported|accessible)\b/i, /\bmodel.*(?:does not exist|doesn't exist)\b/i, + // "does not support" / "unsupported model" — GitHub Copilot / OpenAI-compatible + // often phrase model rejection this way without the "is not supported" word order. + /\bmodel\b[\s\S]{0,80}?\b(?:does\s+not\s+support|doesn't\s+support|unsupported)\b/i, + /\b(?:does\s+not\s+support|doesn't\s+support|unsupported)\b[\s\S]{0,80}?\bmodel\b/i, + /\bunsupported\s+model\b/i, /\baccess.*denied.*model\b/i, /\bmodel.*access.*denied\b/i, /\bplease select a different model\b/i, diff --git a/open-sse/services/combo.ts b/open-sse/services/combo.ts index b19fe68311..f8bdb85eff 100644 --- a/open-sse/services/combo.ts +++ b/open-sse/services/combo.ts @@ -15,6 +15,7 @@ import { getRuntimeProviderProfile, hasPerModelQuota, isModelLocked, + MODEL_ACCESS_DENIED_PATTERNS, recordModelLockoutFailure, recordProviderFailure, selectLockoutCooldownMs, @@ -697,6 +698,28 @@ export function isParamValidation400(errorText) { /\bis illegal.*range\b/i.test(errorText) ); } +/** + * #5249 / #2101: model-scoped 400s must NEVER stop the combo. + * Upstream often wraps "model X is not supported" in `invalid_request_error` / + * "Bad Request" envelopes. Those wrapper words match the body-specific stop + * substrings, so without this exemption the combo hard-stops on the first + * unavailable model instead of trying the next target. Keep the models in the + * combo — if one rejects, advance. + * @param {string} errorText + */ +export function isModelScoped400(errorText) { + const text = String(errorText || ""); + if (!text) return false; + if (MODEL_ACCESS_DENIED_PATTERNS.some((p) => p.test(text))) return true; + // Extra model-rejection shapes that providers emit outside the shared list + // (Responses API, Copilot, gateway wrappers). + return ( + /\bmodel\b[\s\S]{0,80}?\b(?:not\s+supported|unsupported|unknown|unavailable)\b/i.test(text) || + /\b(?:not\s+supported|unsupported|unknown)\b[\s\S]{0,80}?\bmodel\b/i.test(text) || + /\bunsupported_api_for_model\b/i.test(text) || + /\bdoes\s+not\s+support\s+(?:the\s+)?responses\s+api\b/i.test(text) + ); +} /** @param {object} options */ export async function handleComboChat({ @@ -2302,16 +2325,19 @@ export async function handleComboChat({ // #2101: Prevent infinite fallback loops with 400 Bad Request errors that are genuinely // body-specific (malformed JSON, bad format, missing required fields). - // Context overflow and parameter validation errors are NOT body-specific: + // These should NOT stop the combo: // - Context overflow: different models have different context windows // - Max_tokens / param errors: different models have different output limits - // - Model access denied: different providers serve different model sets - // These should fall through so the next combo target can try. + // - Model access denied / "not supported": different providers serve different + // model sets — keep the model in the combo and try the next target (#5249). + // Wrapper words like "invalid" / "bad request" still stop only when the text is + // NOT model-scoped (e.g. "invalid message format"). if ( result.status === 400 && fallbackResult.shouldFallback && !isContextOverflow400(errorText) && !isParamValidation400(errorText) && + !isModelScoped400(errorText) && (errorText.toLowerCase().includes("context") || errorText.toLowerCase().includes("prompt") || errorText.toLowerCase().includes("token") || diff --git a/tests/unit/combo-model-scoped-400-advance.test.ts b/tests/unit/combo-model-scoped-400-advance.test.ts new file mode 100644 index 0000000000..2243f6c871 --- /dev/null +++ b/tests/unit/combo-model-scoped-400-advance.test.ts @@ -0,0 +1,133 @@ +/** + * Combo must ADVANCE on model-scoped 400s even when the error text also + * contains #2101 stop substrings ("invalid", "bad request"). + * + * User intent: keep models in the combo whether or not every provider supports + * them. If github rejects `claude-fable-5` with "not supported", try the next + * target (claude / antigravity / …) instead of hard-stopping the combo. + * + * Regression shapes covered: + * - "requested model is not supported" (baseline #5249) + * - "invalid_request_error: model X is not supported" (wrapper + model) + * - "Bad Request: The model is not supported" (status text wrapper) + * - "model X does not support Responses API" (API capability rejection) + * + * Genuinely body-specific 400s ("invalid message format") must still stop — + * covered by combo-body-specific-400-stop-4279.test.ts. + */ +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-model-400-")); +process.env.DATA_DIR = TEST_DATA_DIR; +process.env.API_KEY_SECRET = process.env.API_KEY_SECRET || "combo-model-400-test-secret"; + +const { handleComboChat, isModelScoped400 } = await import("../../open-sse/services/combo.ts"); + +const noop = () => {}; +const log = { info: noop, warn: noop, debug: noop, error: noop }; + +function makeCombo(models: string[]) { + return { + name: "test-combo-model-400", + strategy: "priority", + models: models.map((m) => ({ model: m })), + }; +} + +function okResponse(modelStr: string) { + return Response.json({ + id: "ok", + object: "chat.completion", + choices: [{ message: { role: "assistant", content: `ok from ${modelStr}` } }], + }); +} + +test("isModelScoped400 recognizes model-not-supported shapes (incl. invalid/Bad Request wrappers)", () => { + assert.equal(isModelScoped400("requested model is not supported"), true); + assert.equal(isModelScoped400("model claude-fable-5 is not supported"), true); + assert.equal(isModelScoped400("invalid_request_error: model is not supported"), true); + assert.equal(isModelScoped400("Bad Request: The model is not supported"), true); + assert.equal(isModelScoped400("model claude-fable-5 does not support Responses API."), true); + assert.equal(isModelScoped400("unsupported_api_for_model"), true); + assert.equal(isModelScoped400("The model `x` does not exist or you do not have access to it."), true); + // Genuinely body-specific — must NOT be treated as model-scoped + assert.equal(isModelScoped400("Invalid message format: the request body is malformed."), false); + assert.equal(isModelScoped400("malformed JSON in request body"), false); + assert.equal(isModelScoped400("Invalid field: foo is not a recognized field"), false); +}); + +async function assertAdvancesOn(errorMessage: string, label: string) { + const modelsCalled: string[] = []; + const response = await handleComboChat({ + body: { model: "test", messages: [{ role: "user", content: "hi" }] }, + combo: makeCombo(["github/claude-fable-5", "claude/claude-fable-5"]), + handleSingleModel: async (_body: unknown, modelStr: string) => { + modelsCalled.push(modelStr); + if (modelStr === "github/claude-fable-5") { + return Response.json({ error: { message: errorMessage, type: "invalid_request_error" } }, { status: 400 }); + } + return okResponse(modelStr); + }, + log, + settings: {}, + allCombos: [], + }); + + assert.equal( + response.status, + 200, + `${label}: combo should advance to second model and return 200 (got ${response.status})` + ); + assert.deepEqual( + modelsCalled, + ["github/claude-fable-5", "claude/claude-fable-5"], + `${label}: must try both targets; tried: ${modelsCalled.join(", ")}` + ); +} + +test("combo advances when first target returns plain 'model is not supported'", async () => { + await assertAdvancesOn("requested model is not supported", "plain"); +}); + +test("combo advances when first target wraps model rejection in invalid_request_error", async () => { + await assertAdvancesOn( + "invalid_request_error: model claude-fable-5 is not supported", + "invalid_request_error wrapper" + ); +}); + +test("combo advances when first target returns Bad Request + model not supported", async () => { + await assertAdvancesOn("Bad Request: The model is not supported", "Bad Request wrapper"); +}); + +test("combo advances when first target rejects model for Responses API", async () => { + await assertAdvancesOn( + "model claude-fable-5 does not support Responses API.", + "Responses API capability" + ); +}); + +test("combo still STOPS on genuinely body-specific invalid message format", async () => { + const modelsCalled: string[] = []; + const response = await handleComboChat({ + body: { model: "test", messages: [{ role: "user", content: "hi" }] }, + combo: makeCombo(["a/model-1", "b/model-2", "c/model-3"]), + handleSingleModel: async (_body: unknown, modelStr: string) => { + modelsCalled.push(modelStr); + return Response.json( + { detail: "Invalid message format: the request body is malformed." }, + { status: 400 } + ); + }, + log, + settings: {}, + allCombos: [], + }); + + assert.equal(modelsCalled.length, 1, `body-specific 400 must stop at target 1; tried: ${modelsCalled.join(", ")}`); + assert.equal(response.status, 400, "body-specific 400 must surface to the client"); +}); diff --git a/tests/unit/combo-param-validation-fallback-4519.test.ts b/tests/unit/combo-param-validation-fallback-4519.test.ts index 479251db27..fc3fb422ea 100644 --- a/tests/unit/combo-param-validation-fallback-4519.test.ts +++ b/tests/unit/combo-param-validation-fallback-4519.test.ts @@ -11,7 +11,7 @@ import assert from "node:assert/strict"; // 400 and rate-limit as 429 (instead of rewriting everything to 502). const { checkFallbackError } = await import("../../open-sse/services/accountFallback.ts"); -const { isContextOverflow400, isParamValidation400 } = await import( +const { isContextOverflow400, isParamValidation400, isModelScoped400 } = await import( "../../open-sse/services/combo.ts" ); const { normalizeUpstreamFailure } = await import( @@ -38,6 +38,16 @@ test("#4519 combo guard: a genuinely body-specific 400 is NOT classified as over const malformed = "Invalid JSON: unexpected token at position 12"; assert.equal(isContextOverflow400(malformed), false); assert.equal(isParamValidation400(malformed), false); + assert.equal(isModelScoped400(malformed), false); +}); + +test("#5249 combo guard: model-not-supported is model-scoped (must advance, not stop)", () => { + assert.equal(isModelScoped400("requested model is not supported"), true); + assert.equal(isModelScoped400("invalid_request_error: model claude-fable-5 is not supported"), true); + assert.equal(isModelScoped400("Bad Request: The model is not supported"), true); + // still not overflow/param — those are separate advance reasons + assert.equal(isContextOverflow400("model is not supported"), false); + assert.equal(isParamValidation400("model is not supported"), false); }); test("#4519 normalizeUpstreamFailure preserves context_length_exceeded as 400", () => {