From 4279751abdf97cdd44027e2d8bb19e6d4166098b Mon Sep 17 00:00:00 2001 From: Diego Rodrigues de Sa e Souza <8016841+diegosouzapw@users.noreply.github.com> Date: Sun, 21 Jun 2026 14:05:53 -0300 Subject: [PATCH] fix(sse): skip disabled providers in combo fallback (#4500) Rebuilt onto release/v3.8.33 (squash-base-stale). Integrated into release/v3.8.33. --- CHANGELOG.md | 1 + src/sse/handlers/chatHelpers.ts | 17 +++++++++++++++-- tests/integration/chat-pipeline.test.ts | 10 ++++++---- tests/integration/combo-routing-e2e.test.ts | 6 ++++-- tests/integration/llama-cpp-provider.test.ts | 7 ++++--- tests/unit/chat-helpers.test.ts | 17 +++++++++++++++-- tests/unit/chat-route-coverage.test.ts | 10 +++++++--- tests/unit/vscode-token-routes.test.ts | 16 ++++++++++------ 8 files changed, 62 insertions(+), 22 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 40f36fc96a..991c2a8369 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -45,6 +45,7 @@ _In development — bullets added per PR; finalized at release._ ### 🐛 Fixed +- **fix(sse): combo routing now skips a provider whose credentials are all disabled instead of failing the whole request** — when a combo like `antigravity/opus → github/opus` hit a leg whose only configured connections were disabled (or where no connections existed at all), `handleNoCredentials` returned `400 BAD_REQUEST`, which the combo target loop treats as a hard stop (combo's 400-break guard from PR #4316 / issue #4279 prevents infinite fallback loops on body-specific 4xx errors). The combo therefore died on the first leg even when later targets were perfectly healthy. The no-active-credentials branch now returns `404 NOT_FOUND` with `"No active credentials for provider:

"` instead — `404` flows through `checkFallbackError` as `shouldFallback: true` (generic-error catch-all path in `open-sse/services/accountFallback.ts`), so the next combo target is tried. The log level for this branch also drops from `error` to `warn` because zero active credentials is an expected operator-driven state, not a server fault. Inspired-by upstream decolua/9router PR #336. (thanks @East-rayyy) - **fix(embeddings):** forward output dimensions to Gemini for consistent embedding dims. (thanks @nguyenha935) - **fix(translator):** sanitize Read tool args from non-Anthropic models to prevent retry loops. (thanks @GodrezJr2) - **fix(usage):** reuse Gemini CLI project ID for quota checks (avoid re-discovery). (thanks @Delcado19) diff --git a/src/sse/handlers/chatHelpers.ts b/src/sse/handlers/chatHelpers.ts index 94ddcf547a..7de7672207 100644 --- a/src/sse/handlers/chatHelpers.ts +++ b/src/sse/handlers/chatHelpers.ts @@ -585,8 +585,21 @@ export function handleNoCredentials( return errorResponse(lastStatus, lastError); } if (!excludeConnectionId) { - log.error("AUTH", `No credentials for provider: ${provider}`); - return errorResponse(HTTP_STATUS.BAD_REQUEST, `No credentials for provider: ${provider}`); + // Ported from upstream decolua/9router#336 (Ibrahim Ryan): surface as 404 + // NOT_FOUND instead of 400 BAD_REQUEST so combo routing can fall through to + // the next target. The combo target loop (open-sse/services/combo.ts) treats + // 400 as a hard stop to break body-specific infinite fallback loops + // (PR #4316 / issue #4279). 404 flows through checkFallbackError as + // `shouldFallback: true` (generic-error catch-all path in + // open-sse/services/accountFallback.ts), letting a combo like + // `antigravity/opus → github/opus` skip a provider whose credentials are + // all disabled. log level is `warn` rather than `error` because zero active + // credentials is an expected operator-driven state, not a server fault. + log.warn("AUTH", `No active credentials for provider: ${provider}`); + return errorResponse( + HTTP_STATUS.NOT_FOUND, + `No active credentials for provider: ${provider}` + ); } log.warn("CHAT", "No more accounts available", { provider }); return errorResponse( diff --git a/tests/integration/chat-pipeline.test.ts b/tests/integration/chat-pipeline.test.ts index 46178d3ecf..80ac8f0f08 100644 --- a/tests/integration/chat-pipeline.test.ts +++ b/tests/integration/chat-pipeline.test.ts @@ -1184,8 +1184,9 @@ test("chat pipeline allows unauthenticated requests through to provider resoluti // handleChat does not enforce REQUIRE_API_KEY — that's the authz pipeline's job. // Without provider credentials seeded, the request falls through to the "no credentials" path. - assert.equal(response.status, 400); - assert.match(json.error.message, /No credentials for provider/i); + // Upstream port decolua/9router#336: 400 → 404 so combo routing can fall through. + assert.equal(response.status, 404); + assert.match(json.error.message, /No active credentials for provider/i); }); test("chat pipeline returns 400 when the model field is omitted", async () => { @@ -1298,8 +1299,9 @@ test("chat pipeline returns current no-credentials contract when no provider con ); const json = (await response.json()) as any; - assert.equal(response.status, 400); - assert.match(json.error.message, /No credentials for provider: openai/); + // Upstream port decolua/9router#336: 400 → 404 so combo routing can fall through. + assert.equal(response.status, 404); + assert.match(json.error.message, /No active credentials for provider: openai/); }); test("chat pipeline surfaces upstream 500 responses as structured errors", async () => { diff --git a/tests/integration/combo-routing-e2e.test.ts b/tests/integration/combo-routing-e2e.test.ts index e82a997607..fea0c16b8a 100644 --- a/tests/integration/combo-routing-e2e.test.ts +++ b/tests/integration/combo-routing-e2e.test.ts @@ -364,8 +364,10 @@ test("unmapped custom model requests fail after combo resolution falls through", ); const json = (await response.json()) as any; - assert.equal(response.status, 400); - assert.match(json.error.message, /No credentials for provider: tenant/); + // Upstream port decolua/9router#336: 400 → 404 so combo routing can fall through + // to the next target when a provider has zero usable credentials. + assert.equal(response.status, 404); + assert.match(json.error.message, /No active credentials for provider: tenant/); }); test("strategy updates take effect for later requests on the same combo name", async () => { diff --git a/tests/integration/llama-cpp-provider.test.ts b/tests/integration/llama-cpp-provider.test.ts index b67c96e00a..3c87261d32 100644 --- a/tests/integration/llama-cpp-provider.test.ts +++ b/tests/integration/llama-cpp-provider.test.ts @@ -171,7 +171,8 @@ test("llama-cpp provider: alias matching works via model catalog prefix", async assert.equal(json.choices[0].message.content, "42"); }); -test("llama-cpp provider: returns 400 when no connection exists", async () => { +test("llama-cpp provider: returns 404 when no connection exists", async () => { + // Upstream port decolua/9router#336: 400 → 404 so combo routing can fall through. const response = await handleChat( buildRequest({ body: { @@ -182,7 +183,7 @@ test("llama-cpp provider: returns 400 when no connection exists", async () => { }) ); - assert.equal(response.status, 400); + assert.equal(response.status, 404); const json = (await response.json()) as any; - assert.match(json.error.message, /No credentials for provider/); + assert.match(json.error.message, /No active credentials for provider/); }); diff --git a/tests/unit/chat-helpers.test.ts b/tests/unit/chat-helpers.test.ts index ecb5847f7f..e27afdb402 100644 --- a/tests/unit/chat-helpers.test.ts +++ b/tests/unit/chat-helpers.test.ts @@ -247,6 +247,19 @@ test("checkPipelineGates reapplies runtime breaker settings to existing breakers }); test("handleNoCredentials reports missing provider credentials and exhausted accounts", async () => { + // Ported from upstream decolua/9router#336 (Ibrahim Ryan): when a provider has + // zero usable connections (all disabled, or none configured at all), the + // historical 400 BAD_REQUEST classified the failure as non-fallbackable, so a + // combo like `antigravity/opus → github/opus` died on the first leg with a + // hard 400 even though the next combo target was perfectly healthy. + // + // The combo target loop (open-sse/services/combo.ts) deliberately breaks on + // 400 to prevent infinite fallback loops with body-specific 4xx errors + // (#4279/PR#4316). 404 NOT_FOUND, by contrast, flows through checkFallbackError + // as `shouldFallback: true` (generic-error catch-all path, + // open-sse/services/accountFallback.ts:1593-1599) so the next combo target is + // tried. We surface "no active credentials" as 404 so combo can skip past a + // disabled-credentials provider instead of failing the whole request. const missing = handleNoCredentials(null, null, "openai", "gpt-4o-mini", null, null); const exhausted = handleNoCredentials( null, @@ -260,8 +273,8 @@ test("handleNoCredentials reports missing provider credentials and exhausted acc const missingJson = (await missing.json()) as any; const exhaustedJson = (await exhausted.json()) as any; - assert.equal(missing.status, 400); - assert.match(missingJson.error.message, /No credentials for provider: openai/); + assert.equal(missing.status, 404); + assert.match(missingJson.error.message, /No active credentials for provider: openai/); assert.equal(exhausted.status, 500); assert.match(exhaustedJson.error.message, /Primary account failed/); }); diff --git a/tests/unit/chat-route-coverage.test.ts b/tests/unit/chat-route-coverage.test.ts index 6b15123611..28ae908116 100644 --- a/tests/unit/chat-route-coverage.test.ts +++ b/tests/unit/chat-route-coverage.test.ts @@ -300,7 +300,11 @@ test("handleChat keeps the combo error when the global fallback throws", async ( assert.match(json.error.message, /primary combo failed/i); }); -test("handleChat returns 400 when no provider credentials exist", async () => { +test("handleChat returns 404 when no provider credentials exist", async () => { + // Upstream port decolua/9router#336 (Ibrahim Ryan): the no-credentials branch + // of handleNoCredentials now surfaces 404 NOT_FOUND so combo routing can fall + // through to the next target instead of being killed by the combo 400-hard-stop + // guard (open-sse/services/combo.ts, PR #4316 / issue #4279). const response = await handleChat( buildRequest({ body: { @@ -312,8 +316,8 @@ test("handleChat returns 400 when no provider credentials exist", async () => { ); const json = (await response.json()) as any; - assert.equal(response.status, 400); - assert.match(json.error.message, /No credentials for provider: openai/); + assert.equal(response.status, 404); + assert.match(json.error.message, /No active credentials for provider: openai/); }); test("handleChat returns 503 for cooled-down connections and 503 for open circuit breakers", async () => { diff --git a/tests/unit/vscode-token-routes.test.ts b/tests/unit/vscode-token-routes.test.ts index cef7abd78d..344088108d 100644 --- a/tests/unit/vscode-token-routes.test.ts +++ b/tests/unit/vscode-token-routes.test.ts @@ -1115,9 +1115,12 @@ test("vscode tokenized /chat/completions route applies the path token and codex ); const body = (await response.json()) as any; - assert.equal(response.status, 400); - assert.equal(body.error?.code, "bad_request"); - assert.equal(body.error?.message, "No credentials for provider: codex"); + // Upstream port decolua/9router#336: zero-active-credentials now surfaces as + // 404 (combo-fallbackable) instead of 400 (combo hard-stop). The 404 OpenAI + // error code mapping is "model_not_found" (open-sse/config/errorConfig.ts:29). + assert.equal(response.status, 404); + assert.equal(body.error?.code, "model_not_found"); + assert.equal(body.error?.message, "No active credentials for provider: codex"); }); test("vscode tokenized /responses route applies the path token and codex tier rewrite", async () => { @@ -1142,9 +1145,10 @@ test("vscode tokenized /responses route applies the path token and codex tier re ); const body = (await response.json()) as any; - assert.equal(response.status, 400); - assert.equal(body.error?.code, "bad_request"); - assert.equal(body.error?.message, "No credentials for provider: codex"); + // Upstream port decolua/9router#336: see chat/completions sibling test above. + assert.equal(response.status, 404); + assert.equal(body.error?.code, "model_not_found"); + assert.equal(body.error?.message, "No active credentials for provider: codex"); }); test("vscode tokenized api/show route preserves the selected reasoning effort for codex variants", async () => {