Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
53 changes: 35 additions & 18 deletions open-sse/services/combo.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import {
hasPerModelQuota,
isAccountSemaphoreFull,
isModelLocked,
lockModelIfPerModelQuota,
MODEL_ACCESS_DENIED_PATTERNS,
recordModelLockoutFailure,
recordProviderFailure,
Expand Down Expand Up @@ -2231,6 +2232,26 @@ async function handleComboChatInner({
return { ok: false, response: result };
}

// A model-scoped 400 ("The requested model is not supported" / "not
// available for integrator") is permanent for THIS connection — the
// account/integration will not gain support for the model mid-session.
// Combo still advances to the next target immediately (unchanged,
// preserves #5249's cross-provider fallback), but without a lockout
// here the SAME dead model gets retried on every future, separate
// request forever (observed: every auto-combo request wasted several
// upstream 400s on the same GitHub models, all day). isModelLocked()
// is checked before dispatch (see the pre-check above this loop), so
// this lockout is honored on the next request.
if (result.status === 400 && isModelScoped400(errorText) && provider && rawModel) {
lockModelIfPerModelQuota(
provider,
targetWithConnection.connectionId || "",
rawModel,
"model_capacity",
60 * 60 * 1000 // 1h
);
}

// Trigger shared provider circuit breaker for 5xx errors and connection failures. If the
// next target is on the same provider, don't mark it failed (a different model may still
// succeed) — #8376: EXCEPT a proxy-unreachable failure, which poisons every model alike.
Expand Down Expand Up @@ -3286,24 +3307,20 @@ async function handleRoundRobinCombo({
"COMBO-RR",
`Maximum combo attempts (${maxGlobalAttempts}) exceeded. Terminating loop to prevent runaway requests.`
);
return errorResponseWithComboDiagnostics(
503,
"Maximum combo retry limit reached",
{
poolSize: modelCount,
attempted: globalAttempts,
excluded: [
...[...exhaustedProviders].map((p) => ({ provider: p, reason: "exhausted" })),
...[...exhaustedConnections].map((c) => formatExhaustedConnectionKey(String(c))),
],
attemptOrder: rrOutcomes.map((o) => ({
provider: o.model.split("/")[0] || "unknown",
model: o.model,
})),
terminalReason: "max_attempts_exceeded",
recovery: buildRecoveryHint("max_attempts_exceeded"),
}
);
return errorResponseWithComboDiagnostics(503, "Maximum combo retry limit reached", {
poolSize: modelCount,
attempted: globalAttempts,
excluded: [
...[...exhaustedProviders].map((p) => ({ provider: p, reason: "exhausted" })),
...[...exhaustedConnections].map((c) => formatExhaustedConnectionKey(String(c))),
],
attemptOrder: rrOutcomes.map((o) => ({
provider: o.model.split("/")[0] || "unknown",
model: o.model,
})),
terminalReason: "max_attempts_exceeded",
recovery: buildRecoveryHint("max_attempts_exceeded"),
});
}
if (retry > 0) {
log.info(
Expand Down
2 changes: 2 additions & 0 deletions stryker.conf.json
Original file line number Diff line number Diff line change
Expand Up @@ -180,6 +180,7 @@
"tests/unit/combo-lockout-quota-reset-6863.test.ts",
"tests/unit/combo-max-depth-config.test.ts",
"tests/unit/combo-model-lockout-honors-reset-1308.test.ts",
"tests/unit/combo-model-scoped-400-advance.test.ts",
"tests/unit/combo-omnimodel-tag-stripping.test.ts",
"tests/unit/combo-param-validation-fallback-4519.test.ts",
"tests/unit/combo-prescreen.test.ts",
Expand Down Expand Up @@ -247,6 +248,7 @@
"tests/unit/fusion-vision-panel-3378.test.ts",
"tests/unit/gemini-web-capabilities-9356.test.ts",
"tests/unit/gemini-web-missing-browser-3516.test.ts",
"tests/unit/github-model-not-supported-lockout.test.ts",
"tests/unit/grok-cli-oauth.test.ts",
"tests/unit/guardrails-api-3496.test.ts",
"tests/unit/guardrails/visionBridge-responses-9597.test.ts",
Expand Down
26 changes: 23 additions & 3 deletions tests/unit/combo-model-scoped-400-advance.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,16 @@ 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 { clearAllModelLockouts } = await import("../../open-sse/services/accountFallback.ts");

// Reused "github/claude-fable-5" across sub-tests below now persists a
// cross-request model lockout on a 400 "model not supported" (matches
// production combo.ts behavior). Reset it between tests so each sub-test
// still exercises a fresh dispatch instead of being skipped by a lockout
// left over from a previous sub-test in this file.
test.beforeEach(() => {
clearAllModelLockouts();
});

const noop = () => {};
const log = { info: noop, warn: noop, debug: noop, error: noop };
Expand Down Expand Up @@ -53,7 +63,10 @@ test("isModelScoped400 recognizes model-not-supported shapes (incl. invalid/Bad
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);
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);
Expand All @@ -68,7 +81,10 @@ async function assertAdvancesOn(errorMessage: string, label: string) {
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 Response.json(
{ error: { message: errorMessage, type: "invalid_request_error" } },
{ status: 400 }
);
}
return okResponse(modelStr);
},
Expand Down Expand Up @@ -128,6 +144,10 @@ test("combo still STOPS on genuinely body-specific invalid message format", asyn
allCombos: [],
});

assert.equal(modelsCalled.length, 1, `body-specific 400 must stop at target 1; tried: ${modelsCalled.join(", ")}`);
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");
});
50 changes: 50 additions & 0 deletions tests/unit/github-model-not-supported-lockout.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
/**
* Regression guard: a GitHub Copilot 400 "The requested model is not
* supported" / "not available for integrator ..." — permanent for THIS
* account/integration — must get locked out via lockModelIfPerModelQuota so
* future, separate requests skip the same dead model instead of retrying it
* on every single auto-combo request forever (observed: every request in
* production logs wasted several upstream 400 calls on the same GitHub
* models — gpt-5.4, gpt-5.5, gpt-5.6-luna, etc. — all day).
*
* Combo's existing #5249/#2101 guard already lets the current request keep
* rotating to the next target — that behavior is unchanged and untested
* here. This guards the NEW cross-request lockout side effect only.
*/
import test from "node:test";
import assert from "node:assert/strict";

const { isModelScoped400 } = await import("../../open-sse/services/combo/comboPredicates.ts");
const { lockModelIfPerModelQuota, isModelLocked, hasPerModelQuota } =
await import("../../open-sse/services/accountFallback.ts");

const GITHUB_NOT_SUPPORTED_400 = "[400]: The requested model is not supported.";
const GITHUB_INTEGRATOR_400 =
'[400]: The requested model is not available for integrator "vscode-chat". ' +
"Available models: [gpt-4.1 claude-fable-5]. Verify the correct Copilot-Integration-Id header is being sent.";

test("isModelScoped400 matches GitHub's two 'model not supported' phrasings", () => {
assert.equal(isModelScoped400(GITHUB_NOT_SUPPORTED_400), true);
assert.equal(isModelScoped400(GITHUB_INTEGRATOR_400), true);
});

test("github has per-model quota (locks the model, not the whole connection)", () => {
assert.equal(hasPerModelQuota("github"), true);
});

test("lockModelIfPerModelQuota locks a model-not-supported GitHub model for future requests", () => {
const connectionId = `github-${Date.now()}`;

const locked = lockModelIfPerModelQuota(
"github",
connectionId,
"gpt-5.4",
"model_capacity",
60 * 60 * 1000
);

assert.equal(locked, true);
assert.equal(isModelLocked("github", connectionId, "gpt-5.4"), true);
// A sibling model on the same connection must stay eligible.
assert.equal(isModelLocked("github", connectionId, "gpt-5.5"), false);
});
Loading