From a492d6d7801309f8d248b9be334be71f362ff5e5 Mon Sep 17 00:00:00 2001 From: Diego Rodrigues de Sa e Souza Date: Fri, 21 Aug 2026 13:57:49 -0300 Subject: [PATCH] fix(sse): combo diagnostics provider/connection truncation and quota recovery hint (#10967, #10966) (#11012) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ⭐5 — Fix(#10967,#10966): combo diag exhausted_connection truncava o UUID por hardcode de provider="unknown"; recovery hint de quota caía em "retry" genérico. TDD RED→GREEN, 149 testes-irmãos verdes. UNSTABLE é o base-red inherited #9985. --- .../fixes/10967-10966-combo-diag-recovery.md | 2 + open-sse/services/combo.ts | 19 +++-- open-sse/services/combo/comboDiagFormat.ts | 29 +++++++ open-sse/services/combo/pinRecovery.ts | 6 ++ open-sse/services/combo/quotaExhaustion.ts | 12 ++- ...bo-diag-exhausted-connection-10967.test.ts | 62 ++++++++++++++ tests/unit/combo-recovery-quota-10966.test.ts | 81 +++++++++++++++++++ 7 files changed, 205 insertions(+), 6 deletions(-) create mode 100644 changelog.d/fixes/10967-10966-combo-diag-recovery.md create mode 100644 open-sse/services/combo/comboDiagFormat.ts create mode 100644 tests/unit/combo-diag-exhausted-connection-10967.test.ts create mode 100644 tests/unit/combo-recovery-quota-10966.test.ts diff --git a/changelog.d/fixes/10967-10966-combo-diag-recovery.md b/changelog.d/fixes/10967-10966-combo-diag-recovery.md new file mode 100644 index 0000000000..e962b14981 --- /dev/null +++ b/changelog.d/fixes/10967-10966-combo-diag-recovery.md @@ -0,0 +1,2 @@ +- fix(sse): combo diagnostics no longer truncate `exhausted_connection` entries to a hardcoded `provider: "unknown"` with the provider prefix eaten by an 8-char slice — the real provider id is preserved and only the connection id is truncated (#10967) +- fix(sse): combo terminal failures caused entirely by quota/account-balance exhaustion (including a durable HTTP 403 `insufficient_quota` / `AUTHZ_INSUFFICIENT_BALANCE`) now stamp a stable `quota_exhausted` diagnostics reason with a `switch-combo` recovery hint instead of the misleading default `retry` action (#10966) diff --git a/open-sse/services/combo.ts b/open-sse/services/combo.ts index f01d206eca..2d83935794 100644 --- a/open-sse/services/combo.ts +++ b/open-sse/services/combo.ts @@ -34,6 +34,7 @@ import { recordComboFailure, } from "./combo/failureTracker.ts"; import { buildNoUpstreamResponseDiagnostics, buildRecoveryHint } from "./combo/pinRecovery.ts"; +import { formatExhaustedConnectionKey } from "./combo/comboDiagFormat.ts"; import { buildTargetTimeoutRunner } from "./combo/targetTimeoutRunner.ts"; import { recordComboRequest, recordComboShadowRequest, getComboMetrics } from "./comboMetrics.ts"; import { qualityScoreFor } from "./routing/index.ts"; @@ -1081,10 +1082,7 @@ async function handleComboChatInner({ attempted: recordedAttempts, excluded: [ ...[...exhaustedProviders].map((p) => ({ provider: p, reason: "exhausted" })), - ...[...exhaustedConnections].map((c) => ({ - provider: "unknown", - reason: `exhausted_connection:${String(c).slice(0, 8)}`, - })), + ...[...exhaustedConnections].map((c) => formatExhaustedConnectionKey(String(c))), ], attemptOrder: comboAttemptOrder, terminalReason, @@ -2727,11 +2725,22 @@ async function handleComboChatInner({ ); } const retryAfterSeconds = undefined; + // #10966: when every observed failure was independently classified as quota/ + // balance exhaustion (isQuotaExhaustionResponse, tracked via observeFailure's + // allObservedFailuresQuota accumulator), stamp a stable `quota_exhausted` + // terminalReason instead of forwarding the raw upstream error string. The raw + // string falls through buildRecoveryHint's default branch ("retry" / "failed + // transiently"), which is actively misleading for a durable wallet/quota + // exhaustion — retrying the same combo will never refill it. + const terminalReason = + observedFailure && allObservedFailuresQuota + ? "quota_exhausted" + : (lastError ?? "all_models_failed"); return withQuotaExhaustionClassification( errorResponseWithComboDiagnostics( status, msg, - buildComboDiag(lastError ?? "all_models_failed", retryAfterSeconds) + buildComboDiag(terminalReason, retryAfterSeconds) ), observedFailure ? allObservedFailuresQuota : null ); diff --git a/open-sse/services/combo/comboDiagFormat.ts b/open-sse/services/combo/comboDiagFormat.ts new file mode 100644 index 0000000000..caed8dedfd --- /dev/null +++ b/open-sse/services/combo/comboDiagFormat.ts @@ -0,0 +1,29 @@ +/** + * #10967: format an `exhaustedConnections` key stored by targetExhaustion.ts + * (`markAuthLevelExhaustion` / `markAgentrouterConnectionQuotaExhaustion` / + * `markConnectionLevelExhaustion`, all keyed as `` `${provider}:${connectionId}` ``) + * into a diagnostics `excluded` entry. + * + * Before this fix, `buildComboDiag` (combo.ts) hardcoded `provider: "unknown"` and + * `slice(0, 8)`'d the WHOLE key — for a typical `jina-ai:` key that produced + * `exhausted_connection:jina-ai:` (the 7-char provider id + colon consumed the + * entire 8-char budget, the UUID silently dropped, and the real provider id + * discarded in favor of the literal string "unknown"). + * + * Splitting on the FIRST `:` recovers the real provider id and truncates only the + * connection-id half (never the full UUID, matching the public combo projection's + * connection-id redaction policy — #2300). + */ +export function formatExhaustedConnectionKey(key: string): { + provider: string; + reason: string; +} { + const raw = String(key); + const sepIdx = raw.indexOf(":"); + const provider = sepIdx >= 0 ? raw.slice(0, sepIdx) : ""; + const connId = sepIdx >= 0 ? raw.slice(sepIdx + 1) : raw; + return { + provider: provider || "unknown", + reason: `exhausted_connection:${connId.slice(0, 8)}`, + }; +} diff --git a/open-sse/services/combo/pinRecovery.ts b/open-sse/services/combo/pinRecovery.ts index d8cd728443..6923eb5204 100644 --- a/open-sse/services/combo/pinRecovery.ts +++ b/open-sse/services/combo/pinRecovery.ts @@ -32,6 +32,12 @@ export function buildRecoveryHint( next_step: "No active accounts are connected for this combo. Open /dashboard/providers, reconnect at least one, then retry.", }; + case "quota_exhausted": + return { + action: "switch-combo", + next_step: + "Every target in this combo failed with a quota or account-balance exhaustion error. Top up the account/wallet or switch to a combo/provider with available quota — this will not recover on retry.", + }; case "all_models_failed": return { action: "try-auto", diff --git a/open-sse/services/combo/quotaExhaustion.ts b/open-sse/services/combo/quotaExhaustion.ts index 50e2d71863..78c3a8cf1d 100644 --- a/open-sse/services/combo/quotaExhaustion.ts +++ b/open-sse/services/combo/quotaExhaustion.ts @@ -6,6 +6,10 @@ const TERMINAL_QUOTA_CODES = new Set([ "credits_exhausted", "insufficient_quota", "quota_exhausted", + // #10966: durable wallet/balance exhaustion signalled on a 403 (not 402/429) by + // some upstreams — e.g. AUTHZ_INSUFFICIENT_BALANCE, "Insufficient account balance. + // Top up your account at …". + "authz_insufficient_balance", ]); const trustedClassifications = new WeakMap(); @@ -62,7 +66,13 @@ export async function isQuotaExhaustionResponse( const trusted = trustedClassifications.get(response); if (trusted !== undefined) return trusted; - if (response.status !== 402 && response.status !== 429) return false; + // #10966: 403 is included alongside 402/429 — some upstreams (e.g. durable + // wallet/balance exhaustion) return a 403 for a terminal quota condition instead + // of the more common 402/429. The structured-code/text checks below still gate + // this to genuine quota signals (CREDITS_EXHAUSTED_SIGNALS / TERMINAL_QUOTA_CODES / + // checkFallbackError's own classification), so a generic auth-only 403 (invalid + // key, no matching quota signal) still falls through to `false`. + if (response.status !== 402 && response.status !== 429 && response.status !== 403) return false; const { text, structuredError } = await parseError(response); if (provider === "gemini" && response.status === 429) { diff --git a/tests/unit/combo-diag-exhausted-connection-10967.test.ts b/tests/unit/combo-diag-exhausted-connection-10967.test.ts new file mode 100644 index 0000000000..4670cf80db --- /dev/null +++ b/tests/unit/combo-diag-exhausted-connection-10967.test.ts @@ -0,0 +1,62 @@ +// tests/unit/combo-diag-exhausted-connection-10967.test.ts +// #10967: buildComboDiag (open-sse/services/combo.ts) formats exhaustedConnections +// entries via formatExhaustedConnectionKey (open-sse/services/combo/comboDiagFormat.ts). +// Repro: exhaustedConnections is a Set keyed as `${provider}:${connectionId}` +// (open-sse/services/combo/targetExhaustion.ts — markAuthLevelExhaustion / +// markAgentrouterConnectionQuotaExhaustion / markConnectionLevelExhaustion, all three +// call `sets.exhaustedConnections.add(`${provider}:${connId}`)`). +// +// Before the fix, the diagnostics formatter hardcoded `provider: "unknown"` and did +// `String(c).slice(0, 8)` on the WHOLE key, so a real-world key like +// `jina-ai:3f9a1c2e-...` (7-char provider id + colon = 8 chars) produced +// `{ provider: "unknown", reason: "exhausted_connection:jina-ai:" }` — the UUID +// entirely dropped and the real provider id discarded. +// +// This test imports the REAL exported function used by buildComboDiag's +// `excluded` mapping — not a re-implementation of the truncation logic. + +import test from "node:test"; +import assert from "node:assert/strict"; +import { formatExhaustedConnectionKey } from "../../open-sse/services/combo/comboDiagFormat.ts"; + +test("formatExhaustedConnectionKey: real jina-ai UUID key keeps the provider id and truncates only the connection id", () => { + const key = "jina-ai:3f9a1c2e-8b7d-4e11-9c2a-1a2b3c4d5e6f"; + const { provider, reason } = formatExhaustedConnectionKey(key); + + // The exact #10967 regression: provider must NOT be "unknown" when the key + // carries a real provider prefix, and the reason must not read as an empty + // truncated "provider:" artifact. + assert.equal(provider, "jina-ai"); + assert.notEqual(provider, "unknown"); + assert.doesNotMatch(reason, /exhausted_connection:jina-ai:$/); + assert.equal(reason, "exhausted_connection:3f9a1c2e"); +}); + +test("formatExhaustedConnectionKey: connection id is truncated to 8 chars, never leaking the full UUID", () => { + const { reason } = formatExhaustedConnectionKey("openai:aaaaaaaa-bbbb-cccc-dddd-eeeeeeeeeeee"); + assert.equal(reason, "exhausted_connection:aaaaaaaa"); + assert.ok(!reason.includes("bbbb-cccc-dddd"), "must not leak the full connection UUID"); +}); + +test("formatExhaustedConnectionKey: short provider ids (e.g. glm) still resolve correctly", () => { + const { provider, reason } = formatExhaustedConnectionKey("glm:0123456789abcdef"); + assert.equal(provider, "glm"); + assert.equal(reason, "exhausted_connection:01234567"); +}); + +test("formatExhaustedConnectionKey: falls back to unknown when the key carries no provider prefix", () => { + const { provider, reason } = formatExhaustedConnectionKey("no-colon-here"); + assert.equal(provider, "unknown"); + assert.equal(reason, "exhausted_connection:no-colon"); +}); + +test("formatExhaustedConnectionKey: builds the same excluded-entry shape buildComboDiag emits", () => { + // Mirrors the Set-of-keys -> excluded[] mapping that lives inline in + // buildComboDiag (open-sse/services/combo.ts), proving the exported helper + // produces exactly the shape the diagnostics payload expects. + const exhaustedConnections = new Set(["jina-ai:3f9a1c2e-8b7d-4e11-9c2a-1a2b3c4d5e6f"]); + const excluded = [...exhaustedConnections].map((c) => formatExhaustedConnectionKey(String(c))); + assert.deepEqual(excluded, [ + { provider: "jina-ai", reason: "exhausted_connection:3f9a1c2e" }, + ]); +}); diff --git a/tests/unit/combo-recovery-quota-10966.test.ts b/tests/unit/combo-recovery-quota-10966.test.ts new file mode 100644 index 0000000000..8693d054e2 --- /dev/null +++ b/tests/unit/combo-recovery-quota-10966.test.ts @@ -0,0 +1,81 @@ +// tests/unit/combo-recovery-quota-10966.test.ts +// #10966: a combo that exhausts because every observed failure was a durable +// quota/balance error (HTTP 403 `insufficient_quota` / `AUTHZ_INSUFFICIENT_BALANCE`, +// or the more common 402/429 quota signals) must NOT ship `recovery.action = "retry"`. +// +// Root cause chain (both pieces exercised here against the REAL exported functions, +// not re-implemented logic): +// +// 1. open-sse/services/combo/quotaExhaustion.ts::isQuotaExhaustionResponse only +// classified status 402/429 as quota-exhaustion; a 403 insufficient_quota / +// AUTHZ_INSUFFICIENT_BALANCE response (the issue's exact repro) fell straight +// through to `false`, so combo.ts's `allObservedFailuresQuota` accumulator +// never went (and stayed) true for this class of failure. +// 2. open-sse/services/combo/pinRecovery.ts::buildRecoveryHint had no case for a +// stable "quota_exhausted" terminalReason, so even a caller that DID resolve +// the reason correctly fell through to the `default` "retry" / "failed +// transiently" branch — actively wrong advice for a durable wallet exhaustion. + +import test from "node:test"; +import assert from "node:assert/strict"; +import { buildRecoveryHint } from "../../open-sse/services/combo/pinRecovery.ts"; +import { isQuotaExhaustionResponse } from "../../open-sse/services/combo/quotaExhaustion.ts"; + +test("buildRecoveryHint: quota_exhausted terminalReason maps to switch-combo, not retry", () => { + const hint = buildRecoveryHint("quota_exhausted"); + assert.equal(hint.action, "switch-combo"); + assert.notEqual(hint.action, "retry"); + assert.match(hint.next_step, /quota|balance/i); + assert.doesNotMatch(hint.next_step, /failed transiently/i); +}); + +test("buildRecoveryHint: the raw upstream message (pre-fix behavior) still falls through to retry — proves the default branch is the bug surface, not a strawman", () => { + // This is the literal bug from the issue: combo.ts used to pass `lastError` (the + // raw upstream string) straight into buildComboDiag/buildRecoveryHint instead of + // a stable code — this assertion documents that the raw-string path is still + // (correctly) unmapped, which is exactly why combo.ts must stamp "quota_exhausted" + // instead of forwarding the raw message. + const hint = buildRecoveryHint("Insufficient account balance. Top up your account at …"); + assert.equal(hint.action, "retry"); +}); + +test("isQuotaExhaustionResponse: HTTP 403 insufficient_quota code is classified as quota exhaustion (#10966 repro)", async () => { + const response = new Response( + JSON.stringify({ error: { code: "insufficient_quota", message: "Insufficient account balance." } }), + { status: 403 } + ); + const exhausted = await isQuotaExhaustionResponse(response, "some-provider", "some-model", null); + assert.equal(exhausted, true); +}); + +test("isQuotaExhaustionResponse: HTTP 403 AUTHZ_INSUFFICIENT_BALANCE type is classified as quota exhaustion (#10966 repro)", async () => { + const response = new Response( + JSON.stringify({ + error: { + type: "AUTHZ_INSUFFICIENT_BALANCE", + message: "Insufficient account balance. Top up your account at https://example.invalid/billing", + }, + }), + { status: 403 } + ); + const exhausted = await isQuotaExhaustionResponse(response, "some-provider", "some-model", null); + assert.equal(exhausted, true); +}); + +test("isQuotaExhaustionResponse: a generic 403 with no quota signal stays false (no over-widening)", async () => { + const response = new Response( + JSON.stringify({ error: { code: "invalid_api_key", message: "Invalid API key provided." } }), + { status: 403 } + ); + const exhausted = await isQuotaExhaustionResponse(response, "some-provider", "some-model", null); + assert.equal(exhausted, false); +}); + +test("isQuotaExhaustionResponse: 402 payment-required still classified as quota exhaustion (pre-existing behavior preserved)", async () => { + const response = new Response( + JSON.stringify({ error: { code: "insufficient_quota", message: "Payment required." } }), + { status: 402 } + ); + const exhausted = await isQuotaExhaustionResponse(response, "some-provider", "some-model", null); + assert.equal(exhausted, true); +});