diff --git a/docs/architecture/RESILIENCE_GUIDE.md b/docs/architecture/RESILIENCE_GUIDE.md index 2529e41b38..98629da7db 100644 --- a/docs/architecture/RESILIENCE_GUIDE.md +++ b/docs/architecture/RESILIENCE_GUIDE.md @@ -289,6 +289,123 @@ single-shot, so usage accounting and semaphore release are not duplicated. --- +## 7. Upstream Status Restatement (misstated quota errors) + +**Scope:** one upstream gateway that reports temporary quota exhaustion with the wrong HTTP status. + +**Purpose:** correct a misleading status BEFORE classification, so downstream consumers (fallback engine, combo aggregation, the client-facing response) see the true retryable nature of the failure. + +Some gateways signal TEMPORARY quota exhaustion with a non-retryable HTTP +status. `agentrouter.org` returns `403` (sometimes `400`) with a Chinese body +(`用户额度不足` / `额度不足`) instead of the standard `429`. Clients like Claude +Code treat `403` as permanent and abort the session, and without correction +the fallback engine would classify it as `AUTH_ERROR` instead of a quota +event. + +**Implementation:** + +- Registry + matcher: `open-sse/config/upstreamStatusRestatement.ts` — a + per-provider list of rules (`{id, fromStatuses, toStatus, textMarkers, +excludeMarkers, defaultRetryAfterMs}`), matched via `applyStatusRestatement()`. +- Call site: the `providerFailure:` block in `open-sse/handlers/chatCore.ts` + (around line 3654), right after `parseUpstreamError()` parses an upstream + response with an error HTTP status (`!providerResponse.ok`), and before any + classification runs, so every downstream consumer sees the corrected + status. Errors embedded inside a `200` SSE stream follow a separate, + later stream-parsing path and are **not** covered by this hook today — a + known limitation, not yet needed for agentrouter's misstatus (which + surfaces as an error HTTP status). +- Retry eligibility: `429` is in `RETRY_AFTER_ELIGIBLE_STATUSES` + (`open-sse/services/combo/unavailableRetryGate.ts`), so a restated error + carries a real retry window instead of surfacing as a dead `403`. +- The synthetic `60s` `defaultRetryAfterMs` (`upstreamStatusRestatement.ts`) + is only what the restated response tells the **client**; it is not itself + the connection's internal cooldown/lockout duration — that is governed + separately by whichever mechanism actually handles the restated error + (Connection Cooldown's escalating backoff, §2, base `3s` for API-key + providers; or Model Lockout, §3, for per-model-quota providers like + agentrouter). The router can become eligible to retry internally sooner + than the 60s window it advertises to the client — intentional headroom, + not a bug. + +Permanent errors (agentrouter's `无权访问模型` — no access to this model) are +NEVER restated: `excludeMarkers` vetoes the rule even when `textMarkers` hit, +so the error keeps its original status and nothing retries it forever. A +separate provider classification rule +(`agentrouter-model-access-denied` in `open-sse/config/providerErrorRules.ts`) +declares an `auth_error`/scope-`model` match for this text, but it does not +fire on the live production path today: the rule only matches `status === +403`, and `checkFallbackError`'s apikey-category `FORBIDDEN` branch +(`open-sse/services/accountFallback.ts`) returns early for a plain 403 +*before* the provider-rule lookup ever runs. In practice a `无权访问模型` 403 +is handled the same way as the base apikey-provider 403 path (see Connection +Cooldown, §2), not as a 6h model lockout. The rule still exists as a +declarative classification consumable by future callers of `classifyError` +with context — wiring it into the production `checkFallbackError` path is +tracked as a follow-up, not yet done. + +Restated quota errors (`额度不足`) do reach a provider rule in production +(`agentrouter-user-quota-exhausted`, scope `"connection"`), but `scope` on +`ProviderErrorRuleMatch` is currently informational — the persistence path +(`checkFallbackError` → `combo.ts`) only consumes `reason` and `cooldownMs`, +never `scope`. What actually happens for agentrouter (`passthroughModels: +true` → `hasPerModelQuota()` returns `true`) is a **per-model** lockout via +`recordModelLockoutFailure()`: the connection itself is never cooled down for +this error (`combo.ts` skips `recordProviderCooldown` for 429 when +`hasPerModelQuota` is true), so other models on the same account keep being +tried — each one burns one call and its own lockout before combo routing +moves on. Honoring `scope` end-to-end (so a `"connection"` match actually +locks the connection) is tracked as a follow-up. + +### Two-stage design: status restatement, then classification + +Status restatement (`upstreamStatusRestatement.ts`) and provider +classification rules (`open-sse/config/providerErrorRules.ts`, +`providerRuleRegistry`) are separate registries that both key on provider id +and text markers, but they run in different places and serve different +purposes: restatement rewrites the HTTP status early in `chatCore.ts`; +classification rules pick the fallback `reason` and lock `scope` +(`model` / `provider` / `connection`) inside `checkFallbackError()` +(`open-sse/services/accountFallback.ts`). + +Classification rules only see full error **text** (needed to match body +markers like `额度不足`) for providers listed in the `FULL_TEXT_RULE_PROVIDERS` +allowlist in `providerErrorRules.ts` — currently only `"agentrouter"`. For +every other provider, `checkFallbackError` hands `getProviderErrorRuleMatch` +only the structured error (`{code, type}`), which is enough for +header/status/code-based rules but blind to body-text markers. The helper +`resolveRuleMatchBody()` performs this selection: full error text for +allowlisted providers, the structured error otherwise. Adding a provider to +`FULL_TEXT_RULE_PROVIDERS` is an explicit per-provider opt-in — it exists so +that the default path for every provider not on the list stays +byte-for-byte unchanged. + +### Adding a new quota-misstating gateway + +1. Register one rule array in `statusRestatementRegistry` + (`open-sse/config/upstreamStatusRestatement.ts`). Keep `textMarkers` + provider-specific; never reuse generic English phrases that collide with + `CREDITS_EXHAUSTED_SIGNALS` (`open-sse/services/accountFallback.ts`). +2. Optionally register classification rules in + `open-sse/config/providerErrorRules.ts` (`providerRuleRegistry`) to pick + the right lock scope (`connection` for account-wide quota, `model` for + per-model errors). This step only takes effect in production for + providers whose rules need the full error text (body markers): add the + provider id to `FULL_TEXT_RULE_PROVIDERS` in the same file — otherwise + `checkFallbackError` only ever hands the rule the structured + `{code, type}` error and a body-text rule will never match live traffic. + Rules that match purely on `status`/`headers` (like Opencode's or + Minimax's) do not need this opt-in. +3. Add unit tests mirroring `tests/unit/upstream-status-restatement.test.ts` + and `tests/unit/agentrouter-error-rules.test.ts` (including the + not-permanent / not-creditsExhausted guards, and — if the provider needs + the allowlist — a test asserting `resolveRuleMatchBody()` returns the + full text only for that provider). + +No changes to `chatCore.ts`, `classifyError`, or combo are needed. + +--- + ## Other Resilience Features - **19 routing strategies** (priority, weighted, round-robin, context-relay, fill-first, p2c, random, least-used, cost-optimized, reset-aware, reset-window, headroom, strict-random, auto, lkgp, context-optimized, cache-optimized, fusion, pipeline) — see [AUTO-COMBO.md](../routing/AUTO-COMBO.md). diff --git a/open-sse/config/constants.ts b/open-sse/config/constants.ts index 6ddeabe7b5..c45cf542a6 100644 --- a/open-sse/config/constants.ts +++ b/open-sse/config/constants.ts @@ -179,6 +179,24 @@ export const HTTP_STATUS = { SERVICE_UNAVAILABLE: 503, GATEWAY_TIMEOUT: 504, }; + +/** + * #10360 — stable error code for an INTERNAL violation of the executor + * `execute()` result contract (`normalizeExecutorResult` received something + * that is neither a Response nor `{ response: Response }`). + * + * This is our own bug, never a provider/account health signal, so every + * resilience layer must treat it as request-scoped and terminal: no connection + * cooldown, no provider circuit-breaker trip, no retry. It rides on the error's + * `.code` (read by `getUpstreamErrorIdentifier`) and therefore reaches + * `checkFallbackError` as `structuredError.code` and the chat/combo predicates + * as `result.errorCode`. + * + * Lives here (leaf config module) so both `open-sse/handlers/` and + * `open-sse/services/` can import it without creating a cycle. + */ +export const EXECUTOR_CONTRACT_VIOLATION_CODE = "executor_contract_violation"; + export { BACKOFF_CONFIG, COOLDOWN_MS, diff --git a/open-sse/config/providerErrorRules.ts b/open-sse/config/providerErrorRules.ts index ce5c74e702..dd59e991bc 100644 --- a/open-sse/config/providerErrorRules.ts +++ b/open-sse/config/providerErrorRules.ts @@ -29,7 +29,15 @@ export type ProviderErrorRule = { export type ProviderErrorRuleMatch = { reason: ConfiguredErrorReason; - /** Default "provider" — lock the whole connection so other providers take over. */ + /** + * Intended lock scope. NOTE: this field is currently INFORMATIONAL — no + * consumer of `getProviderErrorRuleMatch` (checkFallbackError, combo.ts) + * reads `scope` today; only `reason` and `cooldownMs` are consulted. The + * actual lock scope applied at runtime is decided independently by each + * call site (e.g. `hasPerModelQuota()` deciding model- vs connection-level + * lockout). Honoring this field end-to-end is tracked as a follow-up — + * see `docs/architecture/RESILIENCE_GUIDE.md` §7. + */ scope: "model" | "provider" | "connection"; /** Optional explicit cooldown; falls back to the existing per-reason defaults. */ cooldownMs?: number; @@ -176,6 +184,61 @@ function buildOpenrouterRules(): ProviderErrorRule[] { ]; } +// ─── AgentRouter ──────────────────────────────────────────────────────────── +// agentrouter.org misstates temporary quota exhaustion as 403/400 with a +// Chinese body. upstreamStatusRestatement.ts rewrites the status to 429 +// BEFORE classification, so rules here accept both the raw 403/400 and the +// restated 429 (text is the real discriminator either way). In production, +// the raw 403 path is what actually matters here: checkFallbackError's +// apikey-category FORBIDDEN branch (~line 1699) returns EARLY for a plain +// 403, before these rules are ever consulted — these rules fire on the +// RESTATED 429 (chatCore's upstreamStatusRestatement hook runs first) via +// resolveRuleMatchBody, which is the only path in checkFallbackError that +// hands these rules the full error text instead of just {code, type}. +// - "额度不足": account-wide temporary quota → quota_exhausted, scope +// "connection" (mirror of the Opencode account-wide rationale above). +// NOTE: `scope` on ProviderErrorRuleMatch is currently informational — +// checkFallbackError/combo.ts only consume `reason` and `cooldownMs`, not +// `scope`. For agentrouter specifically (passthroughModels: true → +// hasPerModelQuota() is true), this quota_exhausted match actually +// resolves to a PER-MODEL lockout (recordModelLockoutFailure), not a +// connection-wide lock — other models on the same account keep being +// tried by combo routing (each burning one call) until they lock out +// individually. Honoring `scope` end-to-end is tracked as a follow-up. +// - "无权访问模型": declares auth_error/scope "model" (intent: lock only the +// model so the connection keeps serving the rest — Model Lockout tier). +// This rule does NOT fire on the production path today: it only matches +// `status === 403`, but checkFallbackError's apikey FORBIDDEN branch +// returns early for a plain 403 before this rule is ever consulted (see +// the note above). A live `无权访问模型` 403 is handled like the base +// apikey-provider 403 today. Wiring this rule into that path is tracked +// as a follow-up. +function buildAgentrouterRules(): ProviderErrorRule[] { + const AGENTROUTER_ERROR_STATUSES = new Set([400, 403, 429]); + return [ + { + id: "agentrouter-user-quota-exhausted", + match: ({ status, body }) => { + if (!AGENTROUTER_ERROR_STATUSES.has(status)) return null; + const text = JSON.stringify(body ?? "").toLowerCase(); + if (!text.includes("额度不足")) return null; + return { reason: "quota_exhausted", scope: "connection" }; + }, + }, + { + id: "agentrouter-model-access-denied", + match: ({ status, body }) => { + if (status !== 403) return null; + const text = JSON.stringify(body ?? "").toLowerCase(); + if (!text.includes("无权访问模型")) return null; + // 6h: effectively "until the operator fixes the key's model grants", + // without being an unrecoverable terminal state. + return { reason: "auth_error", scope: "model", cooldownMs: 6 * 60 * 60 * 1000 }; + }, + }, + ]; +} + /** * Global registry. Provider name → ordered list of rules (first match wins). * Add new providers here; the matcher in classifyError will pick them up @@ -189,8 +252,37 @@ export const providerRuleRegistry = new Map([ ["minimax-passthrough", buildMinimaxRules()], ["cloudflare-ai", buildCloudflareAiRules()], ["openrouter", buildOpenrouterRules()], + ["agentrouter", buildAgentrouterRules()], ]); +/** + * Providers whose rules match on the FULL upstream error text. + * checkFallbackError's rule lookup normally passes only the structured + * error ({code, type} — message stripped by the combo callers), which is + * enough for header/status/code rules but blind to body-text markers like + * agentrouter's "额度不足". Providers in this set get the raw error text as + * the match body instead. EXCLUSIVE allowlist by owner decision (2026-08-13): + * adding a provider here is an explicit opt-in — the default path for every + * other provider must remain byte-for-byte unchanged. + */ +const FULL_TEXT_RULE_PROVIDERS = new Set(["agentrouter"]); + +/** + * Resolve the body handed to getProviderErrorRuleMatch inside + * checkFallbackError: full error text for FULL_TEXT_RULE_PROVIDERS, + * the structured error for everyone else. + */ +export function resolveRuleMatchBody( + provider: string | null | undefined, + structuredError: unknown, + errorText: string | null | undefined +): unknown { + if (provider && FULL_TEXT_RULE_PROVIDERS.has(provider.toLowerCase()) && errorText) { + return errorText; + } + return structuredError ?? null; +} + /** * Returns the first matching rule for a provider, or null if none match. * Callers use this to (a) classify the reason and (b) decide whether to diff --git a/open-sse/config/upstreamStatusRestatement.ts b/open-sse/config/upstreamStatusRestatement.ts new file mode 100644 index 0000000000..ccadf2d704 --- /dev/null +++ b/open-sse/config/upstreamStatusRestatement.ts @@ -0,0 +1,129 @@ +/** + * Upstream status restatement — registry of gateways that MISSTATE temporary + * quota exhaustion as a non-retryable HTTP status. + * + * agentrouter.org signals "user quota exhausted" with 403 (sometimes 400) and + * a Chinese body ("用户额度不足") instead of the standard 429. Clients like + * Claude Code treat 403 as permanent and abort the whole session, and our own + * fallback engine classifies it as AUTH_ERROR instead of a quota event. + * + * applyStatusRestatement() is called from exactly ONE place — the + * `providerFailure:` block in open-sse/handlers/chatCore.ts, right after + * parseUpstreamError() parses an upstream response with an error HTTP status + * (!providerResponse.ok), and before any classification runs — so every + * downstream consumer (checkFallbackError, combo aggregation, the client + * response) sees the corrected status. Errors embedded inside a 200 SSE + * stream follow a separate, later stream-parsing path and are NOT covered by + * this hook today (known limitation; not yet needed for agentrouter's + * misstatus, which surfaces as an error HTTP status). 429 is + * Retry-After-eligible in + * open-sse/services/combo/unavailableRetryGate.ts, so the client also gets a + * retry window instead of a dead 403. + * + * Adding a future gateway with the same defect = register ONE rule array + * below (and, for cooldown-scope refinement, one entry in + * providerErrorRules.ts). No pipeline changes. + * + * Marker discipline: keep textMarkers provider-specific (the Chinese strings + * are upstream error literals, not UI copy). Generic English phrases like + * "insufficient_quota" are in CREDITS_EXHAUSTED_SIGNALS + * (accountFallback.ts) and would flip the connection into a terminal + * credits_exhausted state — never use them as markers here. + * + * Accepted trade-off: matching only on response body text means a + * legitimate 400 whose body ECHOES user-supplied content containing a + * marker (e.g. a prompt that itself contains "额度不足") would be restated to + * 429 and lose the combo's 400 stop-guard. This is treated as an acceptable + * risk because these markers are rare outside a genuine upstream error; + * keeping markers short, provider-specific, and non-generic (as above) + * minimizes false-positive restatement. + */ + +export type UpstreamStatusRestatementRule = { + id: string; + fromStatuses: ReadonlySet; + toStatus: number; + /** Lowercase markers matched against lowercased `message` + JSON(body). Any hit → restate. */ + textMarkers: readonly string[]; + /** Lowercase markers that VETO the rule even when textMarkers hit (permanent errors). */ + excludeMarkers?: readonly string[]; + /** Synthetic Retry-After used ONLY when the upstream provided none. */ + defaultRetryAfterMs?: number; +}; + +export type StatusRestatementInput = { + provider: string | null | undefined; + status: number; + message: string | null | undefined; + body?: unknown; + retryAfterMs?: number | null; +}; + +export type StatusRestatementResult = { + status: number; + retryAfterMs: number | null; + ruleId: string | null; + fromStatus: number; +}; + +// ─── agentrouter ──────────────────────────────────────────────────────────── +// Observed misstatus (ClaudeShield field reports + upstream behavior): +// 403 "用户额度不足" / "额度不足" → temporary user-quota exhaustion → 429 +// 400 variants carrying the same quota text → 429 +// 403 "无权访问模型" (no access to this model) → genuinely permanent, NEVER +// restated — it must keep flowing as 403 so nothing retries it forever. +const AGENTROUTER_RULES: UpstreamStatusRestatementRule[] = [ + { + id: "agentrouter-quota-misstatus", + fromStatuses: new Set([403, 400]), + toStatus: 429, + textMarkers: ["额度不足"], + excludeMarkers: ["无权访问"], + defaultRetryAfterMs: 60_000, + }, +]; + +/** Provider id (lowercase) → ordered rules; first match wins. */ +export const statusRestatementRegistry = new Map([ + ["agentrouter", AGENTROUTER_RULES], +]); + +function stringifyBody(body: unknown): string { + if (body === null || body === undefined) return ""; + if (typeof body === "string") return body; + try { + return JSON.stringify(body); + } catch { + return ""; + } +} + +export function applyStatusRestatement(input: StatusRestatementInput): StatusRestatementResult { + const passthrough: StatusRestatementResult = { + status: input.status, + retryAfterMs: input.retryAfterMs ?? null, + ruleId: null, + fromStatus: input.status, + }; + if (!input.provider) return passthrough; + const rules = statusRestatementRegistry.get(input.provider.toLowerCase()); + if (!rules) return passthrough; + + const haystack = `${input.message ?? ""} ${stringifyBody(input.body)}`.toLowerCase(); + if (!haystack.trim()) return passthrough; + + for (const rule of rules) { + if (!rule.fromStatuses.has(input.status)) continue; + if (!rule.textMarkers.some((marker) => haystack.includes(marker))) continue; + if (rule.excludeMarkers?.some((marker) => haystack.includes(marker))) continue; + const upstreamRetryAfterMs = + typeof input.retryAfterMs === "number" && input.retryAfterMs > 0 ? input.retryAfterMs : null; + return { + status: rule.toStatus, + retryAfterMs: upstreamRetryAfterMs ?? rule.defaultRetryAfterMs ?? null, + ruleId: rule.id, + fromStatus: input.status, + }; + } + return passthrough; +} diff --git a/open-sse/handlers/chatCore.ts b/open-sse/handlers/chatCore.ts index 35e579a45d..8976a4272b 100644 --- a/open-sse/handlers/chatCore.ts +++ b/open-sse/handlers/chatCore.ts @@ -190,6 +190,7 @@ import { DEFAULT_MAX_TOKENS, STREAM_DISCONNECT_GRACE_PERIOD_MS, } from "../config/constants.ts"; +import { applyStatusRestatement } from "../config/upstreamStatusRestatement.ts"; import { createRecoverableStream, makeContinuationBody } from "../services/streamRecovery.ts"; import { resolveResilienceSettings, @@ -3667,6 +3668,27 @@ export async function handleChatCore({ upstreamErrorType = details.errorType as string | undefined; } + // Gateways like agentrouter misstate temporary quota exhaustion as 403/400, + // which downstream classification treats as AUTH_ERROR and clients like + // Claude Code treat as permanent. Restate to 429 (+ synthetic Retry-After) + // BEFORE any classification so both the fallback engine and the surfaced + // client status see a retryable error. Registry-scoped per provider. + const restatement = applyStatusRestatement({ + provider, + status: statusCode, + message, + body: upstreamErrorBody, + retryAfterMs, + }); + if (restatement.ruleId) { + statusCode = restatement.status; + retryAfterMs = restatement.retryAfterMs; + log?.info?.( + "STATUS_RESTATE", + `${provider} ${restatement.fromStatus}→${statusCode} (${restatement.ruleId})` + ); + } + const signatureRecovery = await recoverAnthropicThinkingSignature({ provider, statusCode, diff --git a/open-sse/handlers/chatCore/upstreamTimeouts.ts b/open-sse/handlers/chatCore/upstreamTimeouts.ts index 5de972dcae..9f0ace2b0a 100644 --- a/open-sse/handlers/chatCore/upstreamTimeouts.ts +++ b/open-sse/handlers/chatCore/upstreamTimeouts.ts @@ -1,4 +1,8 @@ -import { FETCH_TIMEOUT_MS } from "../../config/constants.ts"; +import { + EXECUTOR_CONTRACT_VIOLATION_CODE, + FETCH_TIMEOUT_MS, + HTTP_STATUS, +} from "../../config/constants.ts"; import { getModelTimeoutMs } from "../../config/providerModels.ts"; import { getLoggedInputTokens, @@ -98,6 +102,62 @@ export function getExecutorTimeoutMs(executor: unknown, provider?: string, model return resolveProviderTimeoutMs(executor); } +/** + * Cross-realm Response detection (#10360). + * + * `instanceof Response` is a NOMINAL check against `globalThis.Response`, and + * OmniRoute's default egress does not use the global one: `proxyFetch.ts` + * dispatches through the npm `undici` package's `fetch`, whose `Response` is a + * different class from the Node built-in. A bare `instanceof` therefore + * rejected virtually every real upstream response as a "contract violation". + * + * Accept the built-in fast path first, then fall back to a structural probe: + * the `Symbol.toStringTag` brand plus the members the pipeline actually reads + * (`status`/`ok`/`headers.get`/`text`/`clone`). A plain `{ status, ok }` bag + * still fails, so the guard keeps its value. + */ +export function isResponseLike(value: unknown): value is Response { + if (value instanceof Response) return true; + if (!value || typeof value !== "object") return false; + const candidate = value as { + status?: unknown; + ok?: unknown; + headers?: { get?: unknown } | null; + text?: unknown; + clone?: unknown; + }; + return ( + Object.prototype.toString.call(value) === "[object Response]" && + typeof candidate.status === "number" && + typeof candidate.ok === "boolean" && + !!candidate.headers && + typeof candidate.headers.get === "function" && + typeof candidate.text === "function" && + typeof candidate.clone === "function" + ); +} + +/** + * Builds the terminal error thrown on a genuine contract violation (#10360). + * + * Carries `status = 500` and `code = EXECUTOR_CONTRACT_VIOLATION_CODE` so the + * failure is classified as an INTERNAL, non-retryable defect instead of falling + * through chatCore's `BAD_GATEWAY` default. A 502 made every layer treat our own + * bug as a flaky provider: the connection was cooled down as "rate limited", the + * provider breaker counted it, and the batch runner (which retries 429/502/504) + * span for its full 24h window on an error that can never resolve itself. + */ +export function createExecutorContractError(): Error & { status: number; code: string } { + const err = new TypeError("Executor result must contain a Response") as TypeError & { + status: number; + code: string; + }; + err.name = "ExecutorContractError"; + err.status = HTTP_STATUS.SERVER_ERROR; + err.code = EXECUTOR_CONTRACT_VIOLATION_CODE; + return err; +} + export function normalizeExecutorResult(result: unknown): { response: Response; url: string; @@ -105,16 +165,16 @@ export function normalizeExecutorResult(result: unknown): { transformedBody: unknown; transport?: string; } { - if (result instanceof Response) { + if (isResponseLike(result)) { return { response: result, url: "", headers: {}, transformedBody: null }; } if ( !result || typeof result !== "object" || !("response" in result) || - !(result.response instanceof Response) + !isResponseLike(result.response) ) { - throw new TypeError("Executor result must contain a Response"); + throw createExecutorContractError(); } const normalized = result as { response: Response; diff --git a/open-sse/services/accountFallback.ts b/open-sse/services/accountFallback.ts index d8175272c9..5435a0e2ea 100644 --- a/open-sse/services/accountFallback.ts +++ b/open-sse/services/accountFallback.ts @@ -1,5 +1,6 @@ import { BACKOFF_STEPS_MS, + EXECUTOR_CONTRACT_VIOLATION_CODE, PROVIDER_PROFILES, RateLimitReason, HTTP_STATUS, @@ -14,7 +15,7 @@ import { serviceSupervisorCooldown, isNimFunctionDegraded, } from "../config/errorConfig.ts"; -import { getProviderErrorRuleMatch } from "../config/providerErrorRules.ts"; +import { getProviderErrorRuleMatch, resolveRuleMatchBody } from "../config/providerErrorRules.ts"; import * as rot from "./rotationConfig.ts"; import { getPassthroughProviders, getProviderCategory } from "../config/providerRegistry.ts"; import { @@ -1458,6 +1459,21 @@ export function checkFallbackError( * caller can persist an explicit reset window instead of the engine's scaled cooldown. */ configuredCooldownMs?: number; } { + // #10360: an executor-result contract violation is OUR bug, not the provider's. + // Retrying reproduces it verbatim, and cooling the connection down (or tripping + // the provider breaker) punishes a healthy account for an internal defect. Must + // run before every other classification — the surfaced status is a plain 500, + // which the retryable set below would otherwise treat as a transient upstream + // failure and hand a backoff cooldown. + if (structuredError?.code === EXECUTOR_CONTRACT_VIOLATION_CODE) { + return { + shouldFallback: false, + cooldownMs: 0, + reason: EXECUTOR_CONTRACT_VIOLATION_CODE, + skipProviderBreaker: true, + }; + } + const svc = serviceSupervisorCooldown(status, headers); if (svc) return svc; const rg = rot.gateFor(status, rotation?.account); @@ -1727,7 +1743,12 @@ export function checkFallbackError( // specific configured reasons (e.g. 503 → SERVER_ERROR would be // shadowed by 503 → MODEL_CAPACITY). const providerMatch = provider - ? getProviderErrorRuleMatch(provider, status, headers, structuredError ?? null) + ? getProviderErrorRuleMatch( + provider, + status, + headers, + resolveRuleMatchBody(provider, structuredError ?? null, errorStr) + ) : null; const reason = providerMatch ? providerMatch.reason @@ -1760,7 +1781,12 @@ export function checkFallbackError( // generic zero-cooldown default. Mirror the backoff branch above so // provider rules win on cooldown/reason regardless of `backoff`. const providerMatch = provider - ? getProviderErrorRuleMatch(provider, status, headers, structuredError ?? null) + ? getProviderErrorRuleMatch( + provider, + status, + headers, + resolveRuleMatchBody(provider, structuredError ?? null, errorStr) + ) : null; const cooldownMs = providerMatch?.cooldownMs ?? configuredRule.cooldownMs ?? 0; return { diff --git a/open-sse/services/combo/comboPredicates.ts b/open-sse/services/combo/comboPredicates.ts index 6a424d7439..dc87509236 100644 --- a/open-sse/services/combo/comboPredicates.ts +++ b/open-sse/services/combo/comboPredicates.ts @@ -6,6 +6,7 @@ * predicates are re-exported from combo.ts for backward compatibility. */ +import { EXECUTOR_CONTRACT_VIOLATION_CODE } from "../../config/constants.ts"; import { errorResponse } from "../../utils/error.ts"; import { parseModel } from "../model.ts"; import { isSelfInflictedUpstreamTimeout } from "../../handlers/chatCore/cooldownClassification.ts"; @@ -201,6 +202,9 @@ const REQUEST_SCOPED_UPSTREAM_ERROR_CODES: Record = { rate_limit_queue_timeout: true, rate_limit_queue_full: true, rate_limit_queue_wedged: true, + // #10360: our own executor-result contract violation. An internal defect, not + // a provider/account fault — it must never cool a connection or trip a breaker. + [EXECUTOR_CONTRACT_VIOLATION_CODE]: true, }; /** Request/model-specific failures must not poison provider-wide resilience state. */ diff --git a/open-sse/services/compression/outputStyles/catalog.ts b/open-sse/services/compression/outputStyles/catalog.ts index 1330000d6a..a015e0ff83 100644 --- a/open-sse/services/compression/outputStyles/catalog.ts +++ b/open-sse/services/compression/outputStyles/catalog.ts @@ -95,6 +95,30 @@ export const OUTPUT_STYLE_CATALOG: Record = { }, }, }, + // i-have-adhd (action-first output) — integrated into the output-style registry + // so it rides the existing production injector, like ponytail. + // Source: https://github.com/ayghri/i-have-adhd (MIT). The upstream skill's 10 + // ADHD-friendly rules, adapted for proxy injection: agent-harness-specific rules + // (restate plan state, time estimates) reworded as conditionals so they hold for + // plain chat clients too. + "i-have-adhd": { + id: "i-have-adhd", + label: "I have ADHD (action-first)", + description: + "Action-first output: next action leads, steps numbered, one concrete next step, no preamble.", + levels: { + lite: `# I have ADHD (lite)\nLead with the action: command, path, or snippet first, prose after. Number multi-step work; each step one bounded action. End with ONE concrete next step. No preamble, no recap, no closing pleasantries. ${SHARED_BOUNDARIES}`, + full: `# I have ADHD — action-first output\n\nThe reader has ADHD. Shape output so an ADHD brain can act on it:\n1. Lead with the next action — command, path, or snippet first; context after, if at all.\n2. Number multi-step work; each step is one bounded action; use the fewest steps that work.\n3. End with ONE concrete next step doable in under two minutes.\n4. Suppress tangents: finish the first issue, offer the second as a separate question.\n5. In multi-turn work, restate where things stand ("step 3 of 5 done") — the reader cannot hold state between messages.\n6. When human effort is involved, estimate it in concrete units (minutes, an afternoon), never "some work".\n7. Make wins visible: state what now works and how to try it.\n8. Errors matter-of-fact: cause and fix; never "Uh oh".\n9. Cap lists at 5 items; split into "do now" vs "later" beyond that.\n10. No preamble, no recap, no closers ("Hope this helps").\nExceptions: an explicit "explain" request gets a full body (still no preamble/closer); destructive actions get confirmation first; real ambiguity gets one short clarifying question. ${SHARED_BOUNDARIES}`, + ultra: `# I have ADHD (ultra)\nAction first: command/path/snippet, then prose if needed. Numbered bounded steps, fewest that work. One <2-min next step at the end. No tangents — separate question. Multi-turn: restate state. Human effort: concrete time units. Wins visible. Errors: cause + fix. Lists ≤5. Zero preamble/recap/closers. Explain-requests get full body; destructive actions get confirmation; real ambiguity gets one question. ${SHARED_BOUNDARIES}`, + }, + i18n: { + "pt-BR": { + lite: `# Eu tenho TDAH (lite)\nComece pela ação: comando, path ou snippet primeiro, prosa depois. Numere trabalho multi-passo; cada passo é uma ação delimitada. Termine com UMA próxima ação concreta. Sem preâmbulo, sem recap, sem despedidas. ${SHARED_BOUNDARIES}`, + full: `# Eu tenho TDAH — saída action-first\n\nO leitor tem TDAH. Molde a saída para que um cérebro TDAH consiga agir sobre ela:\n1. Comece pela próxima ação — comando, path ou snippet primeiro; contexto depois, se necessário.\n2. Numere trabalho multi-passo; cada passo é uma ação delimitada; use o menor número de passos que funcione.\n3. Termine com UMA próxima ação concreta executável em menos de dois minutos.\n4. Suprima tangentes: termine a primeira questão, ofereça a segunda como pergunta separada.\n5. Em trabalho multi-turno, reafirme onde as coisas estão ("passo 3 de 5 feito") — o leitor não guarda estado entre mensagens.\n6. Quando houver esforço humano, estime em unidades concretas (minutos, uma tarde), nunca "um pouco de trabalho".\n7. Torne vitórias visíveis: diga o que funciona agora e como testar.\n8. Erros de forma direta: causa e fix; nunca "Opa!".\n9. Listas com no máximo 5 itens; acima disso, divida em "agora" vs "depois".\n10. Sem preâmbulo, sem recap, sem despedidas ("Espero ter ajudado").\nExceções: pedido explícito de "explique" recebe corpo completo (ainda sem preâmbulo/despedida); ações destrutivas recebem confirmação antes; ambiguidade real recebe uma pergunta curta de esclarecimento. ${SHARED_BOUNDARIES}`, + ultra: `# Eu tenho TDAH (ultra)\nAção primeiro: comando/path/snippet, prosa depois se precisar. Passos numerados e delimitados, o mínimo que funcione. UMA próxima ação <2 min no fim. Sem tangentes — pergunta separada. Multi-turno: reafirme o estado. Esforço humano: unidades concretas de tempo. Vitórias visíveis. Erros: causa + fix. Listas ≤5. Zero preâmbulo/recap/despedidas. "Explique" recebe corpo completo; ação destrutiva recebe confirmação; ambiguidade real recebe uma pergunta. ${SHARED_BOUNDARIES}`, + }, + }, + }, "terse-cjk": { id: "terse-cjk", label: "Terse CJK (文言)", diff --git a/src/app/api/v1/models/catalogHelpers.ts b/src/app/api/v1/models/catalogHelpers.ts index bb1c600a16..05996252a7 100644 --- a/src/app/api/v1/models/catalogHelpers.ts +++ b/src/app/api/v1/models/catalogHelpers.ts @@ -16,6 +16,7 @@ export interface CustomModelEntry { apiFormat?: string; supportedEndpoints?: string[]; inputTokenLimit?: number; + outputTokenLimit?: number; isHidden?: boolean; // User-set "vision-capable" flag (persisted by addCustomModel / replaceCustomModels // in src/lib/db/models.ts). Surfaced into `/v1/models` via diff --git a/stryker.conf.json b/stryker.conf.json index 1214fc37f8..af974a0ff7 100644 --- a/stryker.conf.json +++ b/stryker.conf.json @@ -63,6 +63,7 @@ "tests/unit/adaptive-admission-route-matrix.test.ts", "tests/unit/adaptive-admission-runtime.test.ts", "tests/unit/adobe-firefly.test.ts", + "tests/unit/agentrouter-error-rules.test.ts", "tests/unit/alibaba-free-tier-exhaustion.test.ts", "tests/unit/anthropic-thinking-signature-recovery.test.ts", "tests/unit/antigravity-429-quota-tdd.test.ts", @@ -219,6 +220,7 @@ "tests/unit/edgetts-provider.test.ts", "tests/unit/embeddings-auth.test.ts", "tests/unit/error-classification.test.ts", + "tests/unit/executor-contract-violation-terminal.test.ts", "tests/unit/error-message-sanitization.test.ts", "tests/unit/error-sensitive-redaction.test.ts", "tests/unit/execute-chat-resource-pressure-breaker.test.ts", diff --git a/tests/unit/agentrouter-error-rules.test.ts b/tests/unit/agentrouter-error-rules.test.ts new file mode 100644 index 0000000000..2315a3076c --- /dev/null +++ b/tests/unit/agentrouter-error-rules.test.ts @@ -0,0 +1,141 @@ +import test from "node:test"; +import assert from "node:assert/strict"; + +/** + * agentrouter.org quota model, as declared by the `providerErrorRules.ts` + * classification layer: + * - "额度不足" (quota insufficient) is ACCOUNT-wide and temporary → the rule + * declares scope "connection". + * - "无权访问模型" (no access to this model) is permanent PER MODEL → the rule + * declares scope "model". + * Status matching accepts both the raw upstream 403 AND the restated 429 + * (upstreamStatusRestatement.ts rewrites 403→429 before classification). + * + * IMPORTANT — `scope` above is what the rule DECLARES, not what production + * enforces: `ProviderErrorRuleMatch.scope` is not consumed by + * checkFallbackError/combo.ts today (only `reason`/`cooldownMs` are). For + * agentrouter (passthroughModels: true → hasPerModelQuota() true), the + * quota_exhausted match actually resolves to a PER-MODEL lockout in + * production, not a connection-wide lock — other models on the same account + * keep being tried by combo routing until they lock out individually. And + * the "无权访问模型" rule never reaches production traffic at all today: it + * only matches raw `status === 403`, but checkFallbackError's apikey + * FORBIDDEN branch returns early for a plain 403 before any provider rule is + * consulted (see A7). See `docs/architecture/RESILIENCE_GUIDE.md` §7 for the + * full writeup and the tracked follow-up to honor `scope`. + */ + +const { providerRuleRegistry, getProviderErrorRuleMatch } = await import( + "../../open-sse/config/providerErrorRules.ts" +); +const { classifyError, checkFallbackError } = await import( + "../../open-sse/services/accountFallback.ts" +); +const { RateLimitReason } = await import("../../open-sse/config/constants.ts"); + +test("A1: agentrouter is registered in providerRuleRegistry", () => { + const rules = providerRuleRegistry.get("agentrouter"); + assert.ok(rules && rules.length > 0); +}); + +test("A2: quota body → quota_exhausted scope connection (restated 429)", () => { + const match = getProviderErrorRuleMatch("agentrouter", 429, {}, { + error: { message: "用户额度不足,请充值" }, + }); + assert.ok(match, "quota body must match"); + assert.equal(match.reason, "quota_exhausted"); + assert.equal(match.scope, "connection"); +}); + +test("A3: quota body also matches the raw (pre-restatement) 403", () => { + const match = getProviderErrorRuleMatch("agentrouter", 403, {}, "用户额度不足"); + assert.ok(match); + assert.equal(match.reason, "quota_exhausted"); +}); + +test("A4: 无权访问模型 → auth_error scope model, at the RULE layer only (getProviderErrorRuleMatch directly) — this rule never receives production traffic (see A7): checkFallbackError's apikey FORBIDDEN branch returns early for a plain 403 before reaching this rule", () => { + const match = getProviderErrorRuleMatch("agentrouter", 403, {}, { + error: { message: "无权访问模型 claude-sonnet-4" }, + }); + assert.ok(match); + assert.equal(match.reason, "auth_error"); + assert.equal(match.scope, "model"); +}); + +test("A5: classifyError layer guard — quota text wins over the 403→AUTH_ERROR status fallback (classifyError itself has no production caller today; the production guard is A6/checkFallbackError)", () => { + const reason = classifyError(403, "用户额度不足", { + provider: "agentrouter", + headers: {}, + body: { error: { message: "用户额度不足" } }, + }); + assert.equal(reason, RateLimitReason.QUOTA_EXHAUSTED); +}); + +test("A6: guard — restated quota error is retryable, never terminal, and now actually classified as quota_exhausted", () => { + // Status 429 (post-restatement) reaches checkFallbackError's provider-rule + // lookup. resolveRuleMatchBody() hands agentrouter the full error text + // (instead of just the stripped {code, type} structuredError every other + // provider gets), so the "额度不足" rule actually fires here — this is the + // production path the restatement hook (Task 2) feeds into. + const result = checkFallbackError(429, "用户额度不足", 0, null, "agentrouter", null); + assert.equal(result.shouldFallback, true); + assert.equal(result.reason, "quota_exhausted"); + assert.ok(!result.permanent, "quota misstatus must never be permanent"); + assert.ok(!result.creditsExhausted, "must not trip CREDITS_EXHAUSTED_SIGNALS"); + assert.ok(result.cooldownMs > 0, "must carry a real cooldown"); +}); + +test("A7: guard — raw 403 quota (hook bypassed) is still not account-deactivation", () => { + // A raw (pre-restatement) 403 never actually reaches the agentrouter provider + // rules in production: checkFallbackError's apikey-category FORBIDDEN branch + // (status === 403 && getProviderCategory(provider) === "apikey") returns + // EARLY via resolveApiKeyForbiddenFallback before the provider-rule lookup + // is ever consulted. In the real pipeline, chatCore's upstreamStatusRestatement + // hook (Task 2) already converts 403→429 before checkFallbackError ever sees + // it, so this early-return path is what a hook-bypassed raw 403 hits — and it + // must still not be misclassified as permanent account deactivation. + const result = checkFallbackError(403, "用户额度不足", 0, null, "agentrouter", null); + assert.equal(result.shouldFallback, true); + assert.ok(!result.permanent); +}); + +test("A8: plain agentrouter 403 (no quota text) keeps the default apikey auth path", () => { + const match = getProviderErrorRuleMatch("agentrouter", 403, {}, "Invalid API key"); + assert.equal(match, null); +}); + +test("A9: resolveRuleMatchBody hands full text ONLY to allowlisted providers", async () => { + const { resolveRuleMatchBody } = await import( + "../../open-sse/config/providerErrorRules.ts" + ); + const structured = { code: "rate_limited", type: "requests" }; + assert.equal(resolveRuleMatchBody("agentrouter", structured, "用户额度不足"), "用户额度不足"); + assert.equal(resolveRuleMatchBody("opencode", structured, "monthly usage limit reached"), structured); + assert.equal(resolveRuleMatchBody("openrouter", null, "some error text"), null); + assert.equal(resolveRuleMatchBody("agentrouter", structured, ""), structured); +}); + +test("A10: other providers' checkFallbackError behavior is unchanged (exclusivity)", () => { + // opencode's body-text rule ("organization_quota_exceeded") must still NOT + // fire through checkFallbackError — the allowlist is agentrouter-only, so + // opencode keeps getting only the stripped structuredError as the match + // body (null here, since no structuredError arg is passed), same as before + // this fix. Baseline captured on the pre-fix code with this exact input: + // { shouldFallback: true, cooldownMs: 3000, baseCooldownMs: 3000, + // newBackoffLevel: 1, usedUpstreamRetryHint: false, + // reason: "rate_limit_exceeded" } + // i.e. it falls through to the generic 429 configured rule, NOT the + // opencode-quota-exhausted-body provider rule — asserting `reason` here is + // exactly what proves the allowlist didn't leak to opencode. + const result = checkFallbackError( + 429, + '{"error":{"message":"organization_quota_exceeded"}}', + 0, + null, + "opencode", + null + ); + assert.ok(result.shouldFallback); + assert.equal(result.reason, "rate_limit_exceeded"); + assert.equal(result.cooldownMs, 3000); +}); diff --git a/tests/unit/compression/i-have-adhd-catalog.test.ts b/tests/unit/compression/i-have-adhd-catalog.test.ts new file mode 100644 index 0000000000..f56ff0f22b --- /dev/null +++ b/tests/unit/compression/i-have-adhd-catalog.test.ts @@ -0,0 +1,76 @@ +/** + * Tests for the i-have-adhd output style — action-first prompt injection. + * + * Verifies: + * - i-have-adhd is registered with lite/full/ultra levels + * - i18n map exists for pt-BR with all three levels + * - Each level (en and pt-BR) contains the SHARED_BOUNDARIES suffix + * - Core concepts present: action-first, numbered steps, no preamble + * - No locale gate (style valid under every language) + */ + +import { describe, it } from "node:test"; +import assert from "node:assert/strict"; +import { + OUTPUT_STYLE_CATALOG, + outputStyleMeta, +} from "../../../open-sse/services/compression/outputStyles/catalog.ts"; + +const ADHD = OUTPUT_STYLE_CATALOG["i-have-adhd"]; + +function assertString(v: unknown, label: string): asserts v is string { + assert.equal(typeof v, "string", `${label} must be a string`); +} + +describe("i-have-adhd output style", () => { + it("is registered in the catalog with lite/full/ultra levels", () => { + assert.ok(ADHD, "i-have-adhd must be in catalog"); + assert.equal(ADHD.id, "i-have-adhd"); + assert.ok(ADHD.label.includes("ADHD")); + assertString(ADHD.levels.lite, "lite"); + assertString(ADHD.levels.full, "full"); + assertString(ADHD.levels.ultra, "ultra"); + }); + + it("every level ends with the shared boundaries suffix", () => { + const shared = "Code blocks, file paths"; + assert.ok(ADHD.levels.lite.includes(shared)); + assert.ok(ADHD.levels.full.includes(shared)); + assert.ok(ADHD.levels.ultra.includes(shared)); + }); + + it("the full level contains the action-first core concepts", () => { + assert.ok(ADHD.levels.full.includes("Lead with the next action")); + assert.ok(/[Nn]umber/.test(ADHD.levels.full), "full mentions numbered steps"); + assert.ok(ADHD.levels.full.includes("No preamble")); + }); + + it("has an i18n map for pt-BR with all three intensity levels", () => { + assert.ok(ADHD.i18n, "i18n must be defined"); + const pt = ADHD.i18n["pt-BR"]; + assert.ok(pt, "pt-BR must exist"); + assertString(pt.lite, "pt-BR.lite"); + assertString(pt.full, "pt-BR.full"); + assertString(pt.ultra, "pt-BR.ultra"); + }); + + it("each pt-BR level ends with shared boundaries", () => { + const shared = "Code blocks"; + const pt = ADHD.i18n?.["pt-BR"]; + assert.ok(pt, "pt-BR i18n must exist"); + assert.ok(pt.lite.includes(shared), "pt-BR.lite contains shared boundaries"); + assert.ok(pt.full.includes(shared), "pt-BR.full contains shared boundaries"); + assert.ok(pt.ultra.includes(shared), "pt-BR.ultra contains shared boundaries"); + }); + + it("pt-BR full contains Portuguese action-first terminology", () => { + const pt = ADHD.i18n?.["pt-BR"]; + assert.ok(pt, "pt-BR i18n must exist"); + assert.ok(/ação/.test(pt.full), "pt-BR.full mentions ação"); + assert.ok(/preâmbulo/.test(pt.full), "pt-BR.full mentions preâmbulo"); + }); + + it("carries no locale gate", () => { + assert.equal(outputStyleMeta("i-have-adhd").locale, undefined); + }); +}); diff --git a/tests/unit/executor-contract-violation-terminal.test.ts b/tests/unit/executor-contract-violation-terminal.test.ts new file mode 100644 index 0000000000..e6c06c8741 --- /dev/null +++ b/tests/unit/executor-contract-violation-terminal.test.ts @@ -0,0 +1,173 @@ +/** + * #10360 — the executor-result contract guard must not hot-loop the router. + * + * Two defects, one symptom (`tests/unit/batch_api.test.ts` hanging forever): + * + * 1. CROSS-REALM FALSE POSITIVE. The guard added in #10256 used a bare + * `result.response instanceof Response`. OmniRoute's default egress + * (`open-sse/utils/proxyFetch.ts`) is the npm `undici` package's `fetch`, + * whose `Response` class is NOT `globalThis.Response` — so every ordinary + * upstream response arrived as a "contract violation". The guard must + * recognize a structurally valid Response from any realm. + * + * 2. TRANSIENT MISCLASSIFICATION. A genuine contract violation is an INTERNAL + * bug, not a flaky upstream. It carried no `.status`, so chatCore's default + * mapped it to 502 → the connection got cooled down as "rate limited", the + * provider breaker counted it, and `processSingleItemWithRetry` (which + * retries 429/502/504 up to 200×/24h) span forever. It must surface as a + * terminal internal 500 carrying a stable error code, and every resilience + * layer must treat that code as request-scoped: no cooldown, no breaker. + */ +import test from "node:test"; +import assert from "node:assert/strict"; +import { Response as UndiciResponse } from "undici"; + +import { normalizeExecutorResult } from "../../open-sse/handlers/chatCore/upstreamTimeouts.ts"; +import { EXECUTOR_CONTRACT_VIOLATION_CODE } from "../../open-sse/config/constants.ts"; +import { + isRequestScopedUpstreamFailure, + shouldSkipConnDisable, +} from "../../open-sse/services/combo/comboPredicates.ts"; +import { shouldTripProviderBreakerForResult } from "../../src/sse/handlers/chatPredicates.ts"; +import { checkFallbackError } from "../../open-sse/services/accountFallback.ts"; + +// ─── 1. Cross-realm Response acceptance ────────────────────────────────────── + +test("undici's Response is a different class than the global one (premise)", () => { + assert.notEqual( + UndiciResponse as unknown, + globalThis.Response as unknown, + "if these ever become the same class the cross-realm guard below is moot" + ); + assert.equal( + new UndiciResponse("x", { status: 200 }) instanceof globalThis.Response, + false, + "premise: an undici Response fails a bare `instanceof Response`" + ); +}); + +test("normalizeExecutorResult accepts a cross-realm Response in the capture-object arm", () => { + const response = new UndiciResponse(JSON.stringify({ ok: true }), { status: 401 }); + + const normalized = normalizeExecutorResult({ + response, + url: "https://api.openai.com/v1/chat/completions", + headers: { "x-req": "1" }, + transformedBody: { a: 1 }, + }); + + assert.equal(normalized.response, response as unknown); + assert.equal(normalized.response.status, 401); + assert.equal(normalized.url, "https://api.openai.com/v1/chat/completions"); + assert.deepEqual(normalized.headers, { "x-req": "1" }); + assert.deepEqual(normalized.transformedBody, { a: 1 }); +}); + +test("normalizeExecutorResult accepts a bare cross-realm Response", () => { + const response = new UndiciResponse("body", { status: 503 }); + + const normalized = normalizeExecutorResult(response); + + assert.equal(normalized.response, response as unknown); + assert.equal(normalized.response.status, 503); + assert.equal(normalized.url, ""); + assert.deepEqual(normalized.headers, {}); + assert.equal(normalized.transformedBody, null); +}); + +// ─── 2. A genuine violation is terminal, not a transient provider failure ──── + +function captureThrow(run: () => unknown): Error & { status?: unknown; code?: unknown } { + try { + run(); + } catch (err) { + return err as Error & { status?: unknown; code?: unknown }; + } + throw new assert.AssertionError({ message: "expected normalizeExecutorResult to throw" }); +} + +test("a genuinely malformed executor result still throws", () => { + assert.throws(() => normalizeExecutorResult({}), /must contain a Response/); + assert.throws(() => normalizeExecutorResult(undefined), /must contain a Response/); + assert.throws(() => normalizeExecutorResult({ response: "not-a-response" }), /must contain a/); + // A partial look-alike (no body readers) must NOT slip past the duck-type. + assert.throws( + () => normalizeExecutorResult({ response: { status: 200, ok: true } }), + /must contain a Response/ + ); +}); + +test("the contract-violation error carries an internal-terminal status + stable code", () => { + const err = captureThrow(() => normalizeExecutorResult({ response: "not-a-response" })); + + assert.equal(err.status, 500, "an internal contract violation is a 500, never a provider 502"); + assert.equal( + err.code, + EXECUTOR_CONTRACT_VIOLATION_CODE, + "chatCore reads `.code` (getUpstreamErrorIdentifier) to tag the surfaced error" + ); + assert.equal(EXECUTOR_CONTRACT_VIOLATION_CODE, "executor_contract_violation"); +}); + +test("the contract-violation code is classified as a request-scoped failure", () => { + assert.equal(isRequestScopedUpstreamFailure({ code: EXECUTOR_CONTRACT_VIOLATION_CODE }), true); +}); + +test("a contract violation must not cool the connection down", () => { + assert.equal( + shouldSkipConnDisable( + { + status: 500, + errorCode: EXECUTOR_CONTRACT_VIOLATION_CODE, + errorType: null, + error: "Executor result must contain a Response", + }, + false, + false, + "openai" + ), + true, + "our own bug must never mark the operator's account as rate-limited/unavailable" + ); +}); + +test("a contract violation must not trip the provider circuit breaker", () => { + assert.equal( + shouldTripProviderBreakerForResult( + { + status: 500, + errorCode: EXECUTOR_CONTRACT_VIOLATION_CODE, + errorType: null, + error: "Executor result must contain a Response", + }, + false, + false + ), + false, + "500 is a breaker-failure status, but this one never reached the provider" + ); +}); + +test("checkFallbackError treats the contract violation as terminal — no retry, no cooldown", () => { + const decision = checkFallbackError( + 500, + "[500]: Executor result must contain a Response", + 0, + "gpt-4o-mini", + "openai", + null, + null, + { code: EXECUTOR_CONTRACT_VIOLATION_CODE } + ); + + assert.equal(decision.shouldFallback, false, "retrying our own bug just reproduces it"); + assert.equal(decision.cooldownMs, 0, "no connection cooldown for an internal defect"); + assert.equal(decision.skipProviderBreaker, true); +}); + +test("a real provider 500 is still retryable (the terminal branch is not over-broad)", () => { + const decision = checkFallbackError(500, "Internal server error", 0, null, "openai"); + + assert.equal(decision.shouldFallback, true); + assert.ok(decision.cooldownMs > 0, "a genuine upstream 500 keeps its backoff cooldown"); +}); diff --git a/tests/unit/model-token-limit-catalog.test.ts b/tests/unit/model-token-limit-catalog.test.ts index f176cf087d..b456ce42ad 100644 --- a/tests/unit/model-token-limit-catalog.test.ts +++ b/tests/unit/model-token-limit-catalog.test.ts @@ -251,6 +251,11 @@ test("v1 model catalog overlays same-id custom metadata before final overrides", { outputTokenLimit: 32000 }, false ); + + const customProjected = await getModel(`${prefix}/${modelId}`); + assert.ok(customProjected); + assert.equal(customProjected.max_output_tokens, 32000); + assert.equal( capabilityOverrides.setModelCapabilityOverride( `${prefix}/${modelId}`, diff --git a/tests/unit/models-catalog-route.test.ts b/tests/unit/models-catalog-route.test.ts index 2db47dbf9c..cebb7cc89d 100644 --- a/tests/unit/models-catalog-route.test.ts +++ b/tests/unit/models-catalog-route.test.ts @@ -1398,8 +1398,15 @@ test("v1 models catalog skips duplicate built-ins and custom models from inactiv const duplicateBuiltins = body.data.filter((item) => item.id === "openai/gpt-4o-2024-11-20"); assert.equal(response.status, 200); + // Still exactly one entry: the custom row overlays the built-in, it does not duplicate it. assert.equal(duplicateBuiltins.length, 1); - assert.equal(duplicateBuiltins[0].custom === true, false); + // #10248 changed the contract: a custom row for an id that already exists is the + // operator-owned overlay for that model (catalog.ts:1330) — its explicitly stored + // fields win over the discovered metadata, and the merged entry is flagged `custom`. + // Before #10248 the duplicate was skipped outright, so this asserted `false`. + assert.equal(duplicateBuiltins[0].custom, true); + // The overlay must keep the catalog identity rather than becoming a detached entry. + assert.equal(duplicateBuiltins[0].id, "openai/gpt-4o-2024-11-20"); assert.equal( body.data.some((item) => item.id === "cl/inactive-only" || item.id === "cline/inactive-only"), false diff --git a/tests/unit/upstream-status-restatement.test.ts b/tests/unit/upstream-status-restatement.test.ts new file mode 100644 index 0000000000..7c59bc8b15 --- /dev/null +++ b/tests/unit/upstream-status-restatement.test.ts @@ -0,0 +1,126 @@ +import test from "node:test"; +import assert from "node:assert/strict"; + +/** + * Gateways like agentrouter.org misstate TEMPORARY quota exhaustion as 403/400 + * (Chinese body "用户额度不足"), which Claude Code treats as permanent and dies. + * applyStatusRestatement() rewrites such statuses to 429 (+ synthetic + * Retry-After) in ONE place, before fallback classification and before the + * status ever reaches the client. Registry-driven: future gateways with the + * same defect register one rule array — no pipeline changes. + */ + +const { applyStatusRestatement, statusRestatementRegistry } = await import( + "../../open-sse/config/upstreamStatusRestatement.ts" +); + +test("R1: agentrouter 403 + 用户额度不足 → 429 with synthetic Retry-After", () => { + const out = applyStatusRestatement({ + provider: "agentrouter", + status: 403, + message: '{"error":{"message":"用户额度不足","type":"insufficient_user_quota"}}', + retryAfterMs: null, + }); + assert.equal(out.status, 429); + assert.equal(out.fromStatus, 403); + assert.equal(out.ruleId, "agentrouter-quota-misstatus"); + assert.equal(out.retryAfterMs, 60_000); +}); + +test("R2: agentrouter 403 + 无权访问模型 (no model access) is NOT restated", () => { + const out = applyStatusRestatement({ + provider: "agentrouter", + status: 403, + message: "无权访问模型 claude-sonnet-4", + retryAfterMs: null, + }); + assert.equal(out.status, 403); + assert.equal(out.ruleId, null); +}); + +test("R3: quota marker in body (not message) still restates", () => { + const out = applyStatusRestatement({ + provider: "agentrouter", + status: 403, + message: "Forbidden", + body: { error: { message: "用户额度不足,请充值" } }, + retryAfterMs: null, + }); + assert.equal(out.status, 429); +}); + +test("R4: upstream-provided retryAfterMs wins over the synthetic default", () => { + const out = applyStatusRestatement({ + provider: "agentrouter", + status: 403, + message: "用户额度不足", + retryAfterMs: 5_000, + }); + assert.equal(out.status, 429); + assert.equal(out.retryAfterMs, 5_000); +}); + +test("R5: agentrouter 400 with quota marker also restates (gateway variant)", () => { + const out = applyStatusRestatement({ + provider: "agentrouter", + status: 400, + message: "额度不足", + retryAfterMs: null, + }); + assert.equal(out.status, 429); +}); + +test("R6: agentrouter 403 without quota markers is untouched (real auth error)", () => { + const out = applyStatusRestatement({ + provider: "agentrouter", + status: 403, + message: "Invalid API key", + retryAfterMs: null, + }); + assert.equal(out.status, 403); + assert.equal(out.ruleId, null); +}); + +test("R7: other providers never match agentrouter rules (registry-scoped)", () => { + const out = applyStatusRestatement({ + provider: "openai", + status: 403, + message: "用户额度不足", + retryAfterMs: null, + }); + assert.equal(out.status, 403); +}); + +test("R8: statuses a rule does not list pass through (already-correct 429)", () => { + const out = applyStatusRestatement({ + provider: "agentrouter", + status: 429, + message: "用户额度不足", + retryAfterMs: 1_000, + }); + assert.equal(out.status, 429); + assert.equal(out.ruleId, null); + assert.equal(out.retryAfterMs, 1_000); +}); + +test("R9: registry exposes agentrouter so future gateways copy the one-line recipe", () => { + const rules = statusRestatementRegistry.get("agentrouter"); + assert.ok(rules && rules.length > 0); +}); + +test("R10: chatCore wires applyStatusRestatement into the providerFailure block", async () => { + // chatCore is a god-file that cannot be imported standalone in unit tests + // (side-effectful DB/env wiring), so the wiring contract is asserted at the + // source level: the hook must exist, run against the parsed error, and + // reassign both statusCode and retryAfterMs BEFORE classification. + const { readFile } = await import("node:fs/promises"); + const src = await readFile( + new URL("../../open-sse/handlers/chatCore.ts", import.meta.url), + "utf8" + ); + assert.match(src, /applyStatusRestatement\(/, "chatCore must call applyStatusRestatement"); + const hookIndex = src.indexOf("applyStatusRestatement("); + const classifyIndex = src.indexOf("classifyProviderError(statusCode"); + assert.ok(hookIndex > -1 && classifyIndex > -1 && hookIndex < classifyIndex, + "restatement must run BEFORE classifyProviderError so fallback sees the corrected status"); +});