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
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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: <p>"` 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)
Expand Down
17 changes: 15 additions & 2 deletions src/sse/handlers/chatHelpers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Comment on lines +588 to +597

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The comment states that 404 flows through the generic-error catch-all path in open-sse/services/accountFallback.ts (lines 1593-1599). However, checkFallbackError actually matches the status_404 rule defined in ERROR_RULES (in open-sse/config/errorConfig.ts line 156) via findMatchingErrorRule, which returns shouldFallback: true with a 2-minute cooldown (COOLDOWN_MS.notFound).

While this doesn't negatively affect the behavior (since there are no active credentials to apply a cooldown to anyway), we should correct the comment to avoid misleading future maintainers about how the 404 status code is routed through the fallback engine.

Suggested change
// 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.
// 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` (matching the status_404 rule in
// open-sse/config/errorConfig.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(
Expand Down
10 changes: 6 additions & 4 deletions tests/integration/chat-pipeline.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 () => {
Expand Down Expand Up @@ -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 () => {
Expand Down
6 changes: 4 additions & 2 deletions tests/integration/combo-routing-e2e.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 () => {
Expand Down
7 changes: 4 additions & 3 deletions tests/integration/llama-cpp-provider.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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: {
Expand All @@ -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/);
});
17 changes: 15 additions & 2 deletions tests/unit/chat-helpers.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Comment on lines +250 to +262

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

Similar to the comment in chatHelpers.ts, 404 actually matches the status_404 rule in ERROR_RULES (defined in open-sse/config/errorConfig.ts) rather than flowing through the generic-error catch-all path. Let's update this comment to be technically accurate.

  // 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` (matching the status_404 rule in
  // open-sse/config/errorConfig.ts) 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,
Expand All @@ -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/);
});
Expand Down
10 changes: 7 additions & 3 deletions tests/unit/chat-route-coverage.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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: {
Expand All @@ -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 () => {
Expand Down
16 changes: 10 additions & 6 deletions tests/unit/vscode-token-routes.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 () => {
Expand All @@ -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 () => {
Expand Down
Loading