From e05ac345da46ada61254caca6fe367d59dc4fcb2 Mon Sep 17 00:00:00 2001 From: Diego Rodrigues de Sa e Souza Date: Fri, 14 Aug 2026 20:52:53 -0300 Subject: [PATCH] feat(sse): honor provider-rule lock scope for agentrouter (connection vs model) (#10419) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Makes the ProviderErrorRule `scope` field real at the persistence layer, exclusively for agentrouter (owner decision; every other provider keeps byte-identical behavior). checkFallbackError now surfaces `ruleScope` behind the HONORS_RULE_LOCK_SCOPE_PROVIDERS allowlist, and the agentrouter 403 path consults the rules before the generic apikey-FORBIDDEN early-return. markAccountUnavailable honors scope "connection" with a temporary connection cooldown instead of a per-model lockout — guarded so a permanent state can never be downgraded to a transient retry loop — and combo now skips the exhausted account within the same request, which also stops force-reusing the just-cooled connection via allowRateLimitedConnection. Documented in RESILIENCE_GUIDE §7 with the honest limits (disableCooling connections keep per-model behavior; the 6h model-access cooldown is clamped by mlSettings.maxCooldownMs, 30min by default; same-request skip needs targets carrying their own connectionId). Closes #10334 --- docs/architecture/RESILIENCE_GUIDE.md | 112 ++- open-sse/config/providerErrorRules.ts | 82 ++- open-sse/services/accountFallback.ts | 49 +- open-sse/services/combo/targetExhaustion.ts | 91 ++- src/sse/services/auth.ts | 88 +++ stryker.conf.json | 1 + tests/unit/agentrouter-error-rules.test.ts | 78 ++- .../unit/agentrouter-lock-scope-10334.test.ts | 638 ++++++++++++++++++ 8 files changed, 1059 insertions(+), 80 deletions(-) create mode 100644 tests/unit/agentrouter-lock-scope-10334.test.ts diff --git a/docs/architecture/RESILIENCE_GUIDE.md b/docs/architecture/RESILIENCE_GUIDE.md index 98629da7db..da4095e2fc 100644 --- a/docs/architecture/RESILIENCE_GUIDE.md +++ b/docs/architecture/RESILIENCE_GUIDE.md @@ -330,32 +330,75 @@ excludeMarkers, defaultRetryAfterMs}`), matched via `applyStatusRestatement()`. 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. +so the error keeps its original status and nothing retries it forever. The +matching provider classification rule +(`agentrouter-model-access-denied` in `open-sse/config/providerErrorRules.ts`: +`reason: "auth_error"`, `scope: "model"`, a `6h` declared base cooldown) is +consulted by `checkFallbackError` (`open-sse/services/accountFallback.ts`) +*before* the generic apikey-category `FORBIDDEN` early-return, gated on +`honorsRuleLockScope(provider)` (#10334 — currently agentrouter-exclusive via +the `HONORS_RULE_LOCK_SCOPE_PROVIDERS` allowlist in +`providerErrorRules.ts`). The rule's declared 6h cooldown flows through as +`fallbackResult.baseCooldownMs`, but it still feeds the pre-existing +per-model-quota lockout path (`lockModelIfPerModelQuota()` / +`recordModelLockoutFailure()`, unchanged by #10334 except for the cooldown +source): it is clamped down to the operator's `mlSettings.maxCooldownMs` +(default `1_800_000ms` / 30min), like every other model lockout, and the +*persisted lockout reason* stays the pre-existing hardcoded `"forbidden"`, +not the rule's `"auth_error"` — only the cooldown duration is honored +end-to-end, not the reason string. The connection itself stays active; +sibling models on the same connection are unaffected. -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. +Restated quota errors (`额度不足`) reach a provider rule in production +(`agentrouter-user-quota-exhausted`: `reason: "quota_exhausted"`, `scope: +"connection"`, no declared cooldown of its own — the persistence layer's +scaled backoff default applies). Since #10334, `scope` on +`ProviderErrorRuleMatch` IS consumed end-to-end, but **only** for providers in +the `HONORS_RULE_LOCK_SCOPE_PROVIDERS` allowlist (`providerErrorRules.ts` — +today only `"agentrouter"`, gated via `honorsRuleLockScope()`). For every +other provider `scope` remains informational, exactly as before #10334. +`checkFallbackError` surfaces the matched rule's scope as +`fallbackResult.ruleScope`; `isAgentrouterConnectionQuotaScope()` +(`src/sse/services/auth.ts`) is the shared guard that confirms a +`ruleScope` is genuinely safe to honor as a connection-wide, self-recovering +signal (scope `"connection"`, reason `quota_exhausted`, never `permanent`, +never `creditsExhausted` — a defense against a future rule pairing scope +`"connection"` with a permanent account state). Two consumers call it: + +- **Persistence** (`markAccountUnavailable()`, `src/sse/services/auth.ts`): + instead of falling into the passthrough-provider **per-model** lockout + branch (agentrouter is `passthroughModels: true` → `hasPerModelQuota()` + returns `true`), it applies a **temporary connection cooldown** — + `testStatus: "unavailable"` + `rateLimitedUntil`, never a terminal status + (`credits_exhausted`/`banned`/`expired`) — so the connection self-recovers + once the cooldown lapses instead of requiring a manual credential reset. + Skipped for connections with `disableCooling: true` (#2997): that opt-out + falls through to the per-model lockout instead (a documented trade-off — + see the code comment above the branch). +- **Same-request combo routing** (`applyComboTargetExhaustion()`, + `open-sse/services/combo/targetExhaustion.ts`): the same guard marks the + connection into the in-memory `exhaustedConnections` set, keyed + `${provider}:${connectionId}`. This only skips a remaining SAME-REQUEST + target that *itself already carries that exact `connectionId`* on its own + target object (`getExhaustedTargetSkipReason()`, + `open-sse/services/combo/comboPredicates.ts`, `if (provider && +connectionId)` before the `exhaustedConnections` lookup) — a plain + model-list combo, where sibling targets carry no pinned `connectionId` of + their own and one is only resolved per-dispatch from the response's + `X-OmniRoute-Selected-Connection-Id` header, never hits that key match. For + that common case, the real protection against a remaining leg reusing the + just-exhausted account is NOT this Set — it is the persistence layer above + (the connection's `rateLimitedUntil` is now in the future) combined with + this same guard suppressing `transientRateLimitedProviders` for the + failure (see "Two-stage design" and the code comment on the + `isAgentrouterConnectionQuotaScope` branch in `targetExhaustion.ts`): with + that Set left unmarked, `combo.ts`'s `allowRateLimitedConnection` force-allow + (`open-sse/services/combo.ts:1005-1013`, `:2734-2738`) does NOT kick in for + the provider's remaining legs, so credential selection's `rateLimitedUntil` + filter (`src/sse/services/auth.ts:1238`) is honored normally and a + remaining leg either picks a different, still-eligible agentrouter + connection or fails with no credentials available — it does not force its + way back onto the connection this branch just cooled down. ### Two-stage design: status restatement, then classification @@ -380,6 +423,15 @@ allowlisted providers, the structured error otherwise. Adding a provider to that the default path for every provider not on the list stays byte-for-byte unchanged. +A rule's `scope` (`model` / `provider` / `connection`) is a separate opt-in +from `FULL_TEXT_RULE_PROVIDERS`: `checkFallbackError` only surfaces it as +`fallbackResult.ruleScope`, and downstream consumers only honor it as +anything other than an informational label, for providers in the +`HONORS_RULE_LOCK_SCOPE_PROVIDERS` allowlist in the same file (`gated via +honorsRuleLockScope()` — today only `"agentrouter"`). See "Restated quota +errors" above for what a `scope: "connection"` match actually does once a +provider is on that allowlist. + ### Adding a new quota-misstating gateway 1. Register one rule array in `statusRestatementRegistry` @@ -395,7 +447,15 @@ byte-for-byte unchanged. `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. + Minimax's) do not need this opt-in. Separately, if the rule declares + `scope: "connection"` and the intent is an actual connection-wide cooldown + plus same-request combo skip (not just an informational label), add the + provider id to `HONORS_RULE_LOCK_SCOPE_PROVIDERS` in the same file — this + is what gates `isAgentrouterConnectionQuotaScope()`-style consumption in + `markAccountUnavailable()` (`src/sse/services/auth.ts`) and + `applyComboTargetExhaustion()` + (`open-sse/services/combo/targetExhaustion.ts`); without it, `scope` + still flows through `fallbackResult.ruleScope` but nothing acts on it. 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 diff --git a/open-sse/config/providerErrorRules.ts b/open-sse/config/providerErrorRules.ts index dd59e991bc..17d72be598 100644 --- a/open-sse/config/providerErrorRules.ts +++ b/open-sse/config/providerErrorRules.ts @@ -30,13 +30,15 @@ export type ProviderErrorRule = { export type ProviderErrorRuleMatch = { reason: ConfiguredErrorReason; /** - * 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. + * Intended lock scope. #10334: this field is CONSUMED end-to-end only for + * providers in `HONORS_RULE_LOCK_SCOPE_PROVIDERS` (agentrouter-exclusive + * today, gated by `honorsRuleLockScope()`) — for those, `checkFallbackError` + * surfaces it as `ruleScope` on its return value for the persistence layer + * to honor instead of re-deriving scope from `hasPerModelQuota()`. For + * every other provider it remains INFORMATIONAL: `getProviderErrorRuleMatch` + * callers still read only `reason`/`cooldownMs`, and the actual lock scope + * is decided independently by each call site. Widening the allowlist 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. */ @@ -188,31 +190,29 @@ function buildOpenrouterRules(): ProviderErrorRule[] { // 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}. +// restated 429 (text is the real discriminator either way). Both the raw 403 +// path AND the restated 429 path reach these rules in production: +// checkFallbackError's `honorsRuleLockScope("agentrouter")` pre-check +// (#10334) consults these rules BEFORE the generic apikey-category FORBIDDEN +// branch, and the restated 429 reaches them via the existing provider-rule +// lookup in the configured-rule branch. Both paths use resolveRuleMatchBody, +// the only mechanism in checkFallbackError that hands agentrouter's 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. +// `scope` on ProviderErrorRuleMatch is CONSUMED for agentrouter (#10334, +// exclusive allowlist via `honorsRuleLockScope`): checkFallbackError +// surfaces it as `ruleScope` on its return value. Whether the persistence +// layer (markAccountUnavailable / combo target exhaustion) actually +// switches from `hasPerModelQuota()`-derived scope to honoring `ruleScope` +// is Tasks 2/3 of #10334 — this task only surfaces the field. // - "无权访问模型": 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. +// This rule now fires on the production 403 path (#10334): the +// `honorsRuleLockScope` pre-check matches it and returns its declared +// reason/cooldown/scope before the generic apikey-FORBIDDEN early-return +// ever runs. A live `无权访问模型` 403 therefore no longer falls through to +// the base apikey-provider 403 handling. function buildAgentrouterRules(): ProviderErrorRule[] { const AGENTROUTER_ERROR_STATUSES = new Set([400, 403, 429]); return [ @@ -231,8 +231,15 @@ function buildAgentrouterRules(): ProviderErrorRule[] { 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. + // Declares a 6h cooldown, but the effective cooldown is NOT 6h: the + // model-lockout persistence layer (recordModelLockoutFailure, called from + // markAccountUnavailable) clamps every base cooldown — this one included — + // to the configured model-lockout maxCooldownMs, which defaults to + // 1_800_000ms / 30min (src/lib/resilience/modelLockoutSettings.ts, + // DEFAULT_MODEL_LOCKOUT_SETTINGS.maxCooldownMs). So in practice this is + // "locked for ~30min by default (up to 6h if an operator raises the model- + // lockout cap in settings)", not "until the operator fixes the key's model + // grants" — it is a recoverable window, not a real fix-driven unlock. return { reason: "auth_error", scope: "model", cooldownMs: 6 * 60 * 60 * 1000 }; }, }, @@ -255,6 +262,21 @@ export const providerRuleRegistry = new Map([ ["agentrouter", buildAgentrouterRules()], ]); +/** + * Providers whose ProviderErrorRuleMatch.scope is actually CONSUMED at the + * persistence layer (markAccountUnavailable / combo target exhaustion) to pick + * connection-vs-model lock scope. EXCLUSIVE allowlist by owner decision + * (2026-08-14, issue #10334) — deliberately SEPARATE from + * FULL_TEXT_RULE_PROVIDERS: that set controls what body a rule matches against + * (input), this one controls whether the matched scope changes caller behavior + * (output). A provider could need one without the other. + */ +const HONORS_RULE_LOCK_SCOPE_PROVIDERS = new Set(["agentrouter"]); + +export function honorsRuleLockScope(provider: string | null | undefined): boolean { + return !!provider && HONORS_RULE_LOCK_SCOPE_PROVIDERS.has(provider.toLowerCase()); +} + /** * Providers whose rules match on the FULL upstream error text. * checkFallbackError's rule lookup normally passes only the structured diff --git a/open-sse/services/accountFallback.ts b/open-sse/services/accountFallback.ts index 5435a0e2ea..7fb8a341e4 100644 --- a/open-sse/services/accountFallback.ts +++ b/open-sse/services/accountFallback.ts @@ -15,7 +15,11 @@ import { serviceSupervisorCooldown, isNimFunctionDegraded, } from "../config/errorConfig.ts"; -import { getProviderErrorRuleMatch, resolveRuleMatchBody } from "../config/providerErrorRules.ts"; +import { + getProviderErrorRuleMatch, + resolveRuleMatchBody, + honorsRuleLockScope, +} from "../config/providerErrorRules.ts"; import * as rot from "./rotationConfig.ts"; import { getPassthroughProviders, getProviderCategory } from "../config/providerRegistry.ts"; import { @@ -1458,6 +1462,11 @@ export function checkFallbackError( /** #6061: the provider-configured cooldown (ms) before backoff scaling, surfaced so the * caller can persist an explicit reset window instead of the engine's scaled cooldown. */ configuredCooldownMs?: number; + /** #10334 — the matched ProviderErrorRule's declared lock scope, surfaced so the + * persistence layer can honor it instead of re-deriving scope from + * hasPerModelQuota(). Populated ONLY when honorsRuleLockScope(provider) is true; + * always undefined for every other provider, so existing consumers are unaffected. */ + ruleScope?: "model" | "provider" | "connection"; } { // #10360: an executor-result contract violation is OUR bug, not the provider's. // Retrying reproduces it verbatim, and cooling the connection down (or tripping @@ -1712,6 +1721,36 @@ export function checkFallbackError( return { shouldFallback: false, cooldownMs: 0, reason: RateLimitReason.UNKNOWN }; } + // #10334 — agentrouter EXCLUSIVE: consult the provider rules BEFORE the + // apikey-FORBIDDEN early-return below, so a recognized 403 body (e.g. + // "无权访问模型") carries the rule's declared reason/cooldown/scope instead of + // the generic short auth cooldown. Gated on honorsRuleLockScope — for any + // other provider this block is a no-op and the early-return stays identical. + if (status === HTTP_STATUS.FORBIDDEN && provider && honorsRuleLockScope(provider)) { + const forbiddenMatch = getProviderErrorRuleMatch( + provider, + status, + headers, + resolveRuleMatchBody(provider, structuredError ?? null, errorStr) + ); + if (forbiddenMatch) { + const scaled = getScaledBaseCooldown( + forbiddenMatch.reason as RateLimitReasonValue, + backoffLevel + ); + const ruleCooldownMs = forbiddenMatch.cooldownMs; + return { + shouldFallback: true, + cooldownMs: ruleCooldownMs ?? scaled.cooldownMs, + baseCooldownMs: ruleCooldownMs ?? scaled.baseCooldownMs, + configuredCooldownMs: ruleCooldownMs, + newBackoffLevel: ruleCooldownMs !== undefined ? 0 : scaled.newBackoffLevel, + reason: forbiddenMatch.reason, + ruleScope: forbiddenMatch.scope, + }; + } + } + if ( status === HTTP_STATUS.FORBIDDEN && provider && @@ -1764,6 +1803,8 @@ export function checkFallbackError( providerMatch?.cooldownMs !== undefined && providerMatch.cooldownMs > 0 ? providerMatch.cooldownMs : undefined; + const ruleScope = + providerMatch && honorsRuleLockScope(provider) ? providerMatch.scope : undefined; const fallback = buildRetryableFallback(reason); if (providerCooldownMs !== undefined) { return { @@ -1771,9 +1812,10 @@ export function checkFallbackError( cooldownMs: providerCooldownMs, baseCooldownMs: providerCooldownMs, configuredCooldownMs: providerCooldownMs, + ruleScope, }; } - return fallback; + return { ...fallback, ruleScope }; } // #6842: non-backoff configured rules (e.g. status_402) previously never // consulted providerRuleRegistry, so a provider-specific rule (like @@ -1789,12 +1831,15 @@ export function checkFallbackError( ) : null; const cooldownMs = providerMatch?.cooldownMs ?? configuredRule.cooldownMs ?? 0; + const ruleScope = + providerMatch && honorsRuleLockScope(provider) ? providerMatch.scope : undefined; return { shouldFallback: true, cooldownMs, baseCooldownMs: cooldownMs, configuredCooldownMs: cooldownMs, reason: providerMatch?.reason ?? configuredRule.reason ?? RateLimitReason.UNKNOWN, + ruleScope, }; } diff --git a/open-sse/services/combo/targetExhaustion.ts b/open-sse/services/combo/targetExhaustion.ts index c4a880e79a..0325b64b97 100644 --- a/open-sse/services/combo/targetExhaustion.ts +++ b/open-sse/services/combo/targetExhaustion.ts @@ -27,6 +27,10 @@ import { import { RateLimitReason } from "../../config/constants.ts"; import { isProviderCircuitOpenResult, isRequestScopedUpstreamFailure } from "./comboPredicates.ts"; import { isCloudflareFingerprintRejection } from "../errorClassifier.ts"; +// #10334 — agentrouter-exclusive predicate shared with the persistence layer +// (markAccountUnavailable) so the same-request combo skip and the persisted +// connection cooldown agree on exactly which fallbackResult shapes qualify. +import { isAgentrouterConnectionQuotaScope } from "@/sse/services/auth"; import type { ComboLogger, ResolvedComboTarget } from "./types.ts"; // Connection-level failure statuses: the provider connection itself is likely bad (upstream @@ -60,7 +64,13 @@ export type ComboExhaustionSets = { export type ApplyComboTargetExhaustionOptions = { result: { status: number; headers?: Headers | null }; - fallbackResult: Parameters[0]; + fallbackResult: Parameters[0] & { + /** #10334 — agentrouter-exclusive; see isAgentrouterConnectionQuotaScope + * (src/sse/services/auth.ts). Populated only for providers in + * HONORS_RULE_LOCK_SCOPE_PROVIDERS (today: agentrouter only). */ + ruleScope?: "model" | "provider" | "connection"; + permanent?: boolean; + }; errorText: string; rawModel: string; isTokenLimitBreach: boolean; @@ -86,6 +96,56 @@ export function applyComboTargetExhaustion( const { result, sets, log, tag, errorText, structuredError } = opts; const provider = target.provider; + // #10334: agentrouter-exclusive account-wide quota exhaustion ("额度不足") + // must skip remaining SAME-CONNECTION targets within THIS request too, not + // just via the persisted cooldown markAccountUnavailable applies for + // whichever leg runs next. agentrouter is a passthroughModels provider + // (hasPerModelQuota() === true), so without this branch the classification + // below would fall straight through isProviderQuotaExhausted's + // !hasPerModelQuota() guard, and — for the restated-429 case — + // markConnectionLevelExhaustion's connection-level guard (429 is not in + // CONNECTION_LEVEL_ERROR_STATUSES), marking nothing: combo would keep + // burning one upstream call per remaining model of the same exhausted + // account. isAgentrouterConnectionQuotaScope is the same guard + // markAccountUnavailable uses, so both consumers agree on exactly which + // fallbackResult shapes qualify (never a permanent/credits-exhausted + // result, even one carrying ruleScope "connection"). + // + // Runs BEFORE the auth-level (401/403) branch below. This is deliberate, + // not incidental: the "额度不足" rule matches statuses {400, 403, 429} + // (buildAgentrouterRules, providerErrorRules.ts), and Task 1's FORBIDDEN + // pre-check (accountFallback.ts ~1729-1751) surfaces `ruleScope: + // "connection"` for a RAW 403 carrying that body too — so this branch can + // also fire on a 403, not just the restated 429. That is safe: for a 403 + // this branch and markAuthLevelExhaustion below write the SAME set with + // the SAME `${provider}:${connId}` key and both return `true` — they are + // set-equivalent for agentrouter on that status. The Cloudflare-1010 and + // Alibaba free-tier EXEMPTIONS further down in the 401/403 branch cannot + // apply here regardless of ordering: 1010 is a CDN fingerprint rejection + // agentrouter's own text never carries, and the Alibaba exemption is + // gated on isAlibabaModelStudioProvider(provider), which agentrouter is + // not. + // + // Unlike the connection-level/auth-level branches, this path deliberately + // does NOT fall through to markTransientOrConnectionLevel, so + // sets.transientRateLimitedProviders is NEVER populated for this failure. + // That is required, not just incidental: combo.ts (both dispatchers, see + // the `allowRateLimitedConnection` reads keyed off + // transientRateLimitedProviders) uses that set to force-allow reusing a + // rate-limited CONNECTION for the provider's remaining legs — i.e. it + // bypasses the very `rateLimitedUntil` filter this branch (and Task 2's + // markAccountUnavailable) just set. Marking it here would silently + // re-open the account this branch just cooled down. One secondary + // consequence: a SIBLING agentrouter connection that is merely + // rate-limited (not the one this branch exhausted) will also no longer be + // force-allowed for a later leg on the same provider — a remaining leg + // can now resolve to "no credentials available" instead of retrying a + // rate-limited sibling account, which is the intended, safer outcome. + if (isAgentrouterConnectionQuotaScope(provider, opts.fallbackResult)) { + markAgentrouterConnectionQuotaExhaustion(target, { sets, log, tag }); + return true; + } + // #8133/#8137: auth-level failures (401/403) mean that connection's credentials are bad. // Split out to keep applyComboTargetExhaustion under the complexity ceiling. // Cloudflare 1010 (a 403 carrying error_code 1010 / browser_signature_banned) is NOT an @@ -259,6 +319,35 @@ function markAuthLevelExhaustion( } } +/** + * #10334: agentrouter-exclusive connection-scope account quota exhaustion. Mirrors + * markAuthLevelExhaustion's connectionId-present/absent split — when the target carries a + * connectionId, only that connection's account is exhausted (sibling agentrouter connections + * for the same user may still have quota); fall back to whole-provider exhaustion only when no + * connectionId is available. + */ +function markAgentrouterConnectionQuotaExhaustion( + target: ResolvedComboTarget, + opts: Pick +): void { + const { sets, log, tag } = opts; + const provider = target.provider; + const connId = target.connectionId ?? undefined; + if (connId) { + sets.exhaustedConnections.add(`${provider}:${connId}`); + log.info( + tag, + `Provider ${provider} connection ${connId} account quota exhausted (rule scope=connection) — marking for skip on remaining targets (#10334)` + ); + } else { + sets.exhaustedProviders.add(provider as string); + log.info( + tag, + `Provider ${provider} account quota exhausted (rule scope=connection, no connectionId) — marking for skip on remaining targets (#10334)` + ); + } +} + /** * #1731v2: connection-level errors (408/5xx, excluding the OmniRoute circuit-open signal) suggest * the provider connection itself is bad → skip remaining same-connection (or same-provider, when diff --git a/src/sse/services/auth.ts b/src/sse/services/auth.ts index 1802423c44..5a012cb5f5 100644 --- a/src/sse/services/auth.ts +++ b/src/sse/services/auth.ts @@ -45,6 +45,7 @@ import { } from "@omniroute/open-sse/services/accountFallback.ts"; import { isLocalProvider } from "@omniroute/open-sse/config/providerRegistry.ts"; import { COOLDOWN_MS, RateLimitReason } from "@omniroute/open-sse/config/constants.ts"; +import { honorsRuleLockScope } from "@omniroute/open-sse/config/providerErrorRules.ts"; import { preflightQuota, isQuotaPreflightEnabled, @@ -1981,6 +1982,46 @@ export async function getProviderCredentialsWithQuotaPreflight( } } +/** + * #10334 — Guard for the agentrouter-exclusive "connection scope" quota + * cooldown branch in markAccountUnavailable. The "never terminal" invariant of + * that branch is NOT structurally guaranteed by `ruleScope === "connection"` + * alone — it also depends on the provider rule table only ever pairing scope + * "connection" with a genuinely transient reason. Today + * (`buildAgentrouterRules()` in providerErrorRules.ts) that is true: the only + * rule declaring scope "connection" is the quota-exhausted one. But a FUTURE + * agentrouter rule for a permanent account state (e.g. "账号已封禁") — or a 402 + * added to `AGENTROUTER_ERROR_STATUSES` with scope "connection", a natural- + * looking choice for an account ban — would otherwise be silently downgraded + * to a transient cooldown here instead of going through + * resolveTerminalConnectionStatus()/auto-disable below. Require the + * reason/permanent/creditsExhausted signals checkFallbackError already + * computes to explicitly confirm "this is quota, not a permanent state" + * before taking the early return. + * + * Exported (not just inlined) so a synthetic permanent/credits-exhausted + * `fallbackResult` can be tested directly — no rule in the table produces + * that combination today, so this predicate is the only way to pin the guard + * without editing the (production) rule table just for a test. + */ +export function isAgentrouterConnectionQuotaScope( + provider: string | null | undefined, + fallbackResult: { + ruleScope?: "model" | "provider" | "connection"; + reason?: string; + permanent?: boolean; + creditsExhausted?: boolean; + } +): boolean { + return ( + honorsRuleLockScope(provider) && + fallbackResult.ruleScope === "connection" && + fallbackResult.reason === RateLimitReason.QUOTA_EXHAUSTED && + !fallbackResult.permanent && + !fallbackResult.creditsExhausted + ); +} + /** Persist exponential-backoff state for an unavailable provider connection. */ export async function markAccountUnavailable( connectionId: string, @@ -2101,6 +2142,53 @@ export async function markAccountUnavailable( const disableCooling = connProviderSpecificData.disableCooling === true; const isPerModelQuotaProvider = hasPerModelQuota(provider, model, connectionPassthroughModels); + + // #10334 — agentrouter EXCLUSIVE: the matched provider rule declared scope + // "connection" for account-wide quota exhaustion ("额度不足"). agentrouter is + // a passthroughModels provider (isPerModelQuotaProvider === true), so without + // this branch the next `if` would treat it like any other passthrough 429 and + // lock a SINGLE model — leaving combo routing to burn one upstream call per + // remaining model of the same exhausted account. Must run BEFORE that block. + // Deliberately ignores persistUnavailableState/isCombo: for combo the caller + // downgrades persistUnavailableState to false, and the generic path further + // below would then lock per MODEL instead of cooling the connection — exactly + // what this scope must override. NEVER sets a terminal status: this is a + // renewing quota window, not "credits_exhausted"/"banned"/"expired". + // + // The "never terminal" invariant above is NOT structurally guaranteed by + // ruleScope === "connection" alone — see isAgentrouterConnectionQuotaScope's + // doc comment for why (a future permanent-state rule could pair scope + // "connection" with a non-quota reason). That predicate is the actual guard. + const ruleScopeIsConnection = isAgentrouterConnectionQuotaScope(provider, fallbackResult); + // #2997's disableCooling opt-out is respected here (`!disableCooling` below): + // a connection with disableCooling=true skips this branch entirely and falls + // into the per-model-quota block further down, which locks the model for up + // to ~30min (mlSettings.maxCooldownMs) instead of cooling the connection for + // the rule's shorter transient window. That is a deliberate, if counter- + // intuitive, consequence of #2997's scope (opt-out was designed only for the + // CONNECTION-level cooldown, never extended to model lockout) — "opting out + // of cooldown" ends up producing a LONGER effective block for this one rule. + // Not addressed here; flagged for a future #2997 follow-up if it proves to be + // a real operator complaint. + if (ruleScopeIsConnection && provider && !disableCooling) { + const connectionCooldownMs = + fallbackResult.cooldownMs > 0 ? fallbackResult.cooldownMs : COOLDOWN_MS.rateLimit; + await updateProviderConnection(connectionId, { + lastErrorType: fallbackResult.reason || RateLimitReason.QUOTA_EXHAUSTED, + lastError: `Account quota exhausted (${provider})`, + lastErrorAt: new Date().toISOString(), + errorCode: status, + backoffLevel: fallbackResult.newBackoffLevel ?? backoffLevel, + rateLimitedUntil: getUnavailableUntil(connectionCooldownMs), + testStatus: "unavailable", + }); + log.info( + "AUTH", + `Connection-scoped cooldown for ${provider}:${connectionId.slice(0, 8)} — ${status} ${fallbackResult.reason} ${Math.ceil(connectionCooldownMs / 1000)}s (rule scope=connection, overrides per-model lockout)` + ); + return { shouldFallback: true, cooldownMs: connectionCooldownMs }; + } + const isNvidiaModelGone = provider === "nvidia" && status === 410; const modelLockoutOptions = { maxCooldownMs: effectiveProviderProfile?.maxCooldownMs }; if ( diff --git a/stryker.conf.json b/stryker.conf.json index af974a0ff7..a9a06bdcac 100644 --- a/stryker.conf.json +++ b/stryker.conf.json @@ -64,6 +64,7 @@ "tests/unit/adaptive-admission-runtime.test.ts", "tests/unit/adobe-firefly.test.ts", "tests/unit/agentrouter-error-rules.test.ts", + "tests/unit/agentrouter-lock-scope-10334.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", diff --git a/tests/unit/agentrouter-error-rules.test.ts b/tests/unit/agentrouter-error-rules.test.ts index 2315a3076c..272d21993b 100644 --- a/tests/unit/agentrouter-error-rules.test.ts +++ b/tests/unit/agentrouter-error-rules.test.ts @@ -11,18 +11,17 @@ import assert from "node:assert/strict"; * 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`. + * #10334 — `ProviderErrorRuleMatch.scope` is now CONSUMED for agentrouter: + * `checkFallbackError` surfaces it as `ruleScope` on its return value (see + * A11/A12 below), and a raw 403 is no longer an early-return dead end for + * this provider — `honorsRuleLockScope("agentrouter")` gates a dedicated + * pre-check that consults the provider rules BEFORE the generic apikey + * FORBIDDEN branch (see A7/A12). This is an EXCLUSIVE allowlist + * (`honorsRuleLockScope`, A14): every other provider's `scope` stays + * declared-but-unconsumed exactly as before (A13). See + * `docs/architecture/RESILIENCE_GUIDE.md` §7 for the full writeup — Tasks 2/3 + * of #10334 wire the surfaced `ruleScope` into the persistence layer + * (markAccountUnavailable / combo target exhaustion). */ const { providerRuleRegistry, getProviderErrorRuleMatch } = await import( @@ -53,7 +52,7 @@ test("A3: quota body also matches the raw (pre-restatement) 403", () => { 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", () => { +test("A4: 无权访问模型 → auth_error scope model, at the RULE layer (getProviderErrorRuleMatch directly) — since #10334 this rule DOES receive production traffic for agentrouter via the honorsRuleLockScope pre-check in checkFallbackError (see A12)", () => { const match = getProviderErrorRuleMatch("agentrouter", 403, {}, { error: { message: "无权访问模型 claude-sonnet-4" }, }); @@ -86,14 +85,15 @@ test("A6: guard — restated quota error is retryable, never terminal, and now a }); 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. + // Since #10334, a raw (pre-restatement) 403 for agentrouter DOES reach the + // provider rules: checkFallbackError's honorsRuleLockScope pre-check runs + // BEFORE the generic apikey-category FORBIDDEN branch and matches the + // "额度不足" rule here (reason quota_exhausted, scope connection — see A11). + // In the real pipeline, chatCore's upstreamStatusRestatement hook (Task 2) + // still converts 403→429 before checkFallbackError sees it, so this raw-403 + // path is what a hook-bypassed request hits — and it must still not be + // misclassified as permanent account deactivation, regardless of which + // branch (pre-check or the old apikey-FORBIDDEN fallback) ultimately fires. const result = checkFallbackError(403, "用户额度不足", 0, null, "agentrouter", null); assert.equal(result.shouldFallback, true); assert.ok(!result.permanent); @@ -139,3 +139,39 @@ test("A10: other providers' checkFallbackError behavior is unchanged (exclusivit assert.equal(result.reason, "rate_limit_exceeded"); assert.equal(result.cooldownMs, 3000); }); + +test("A11: checkFallbackError surfaces ruleScope=connection for agentrouter quota", () => { + const result = checkFallbackError(429, "用户额度不足", 0, null, "agentrouter", null); + assert.equal(result.ruleScope, "connection"); + assert.equal(result.reason, "quota_exhausted"); + assert.ok(!result.permanent); +}); + +test("A12: checkFallbackError 403 无权访问模型 carries the rule's scope + cooldown", () => { + const result = checkFallbackError(403, "无权访问模型 claude-opus-5", 0, null, "agentrouter", null); + assert.equal(result.ruleScope, "model"); + assert.equal(result.reason, "auth_error"); + assert.equal(result.baseCooldownMs, 6 * 60 * 60 * 1000); +}); + +test("A13: exclusivity — ruleScope stays undefined for other providers", () => { + const opencode = checkFallbackError( + 429, + '{"error":{"message":"organization_quota_exceeded"}}', + 0, + null, + "opencode", + null + ); + assert.equal(opencode.ruleScope, undefined); + const openrouter = checkFallbackError(402, "credits exhausted", 0, null, "openrouter", null); + assert.equal(openrouter.ruleScope, undefined); +}); + +test("A14: honorsRuleLockScope allowlist is agentrouter-only", async () => { + const { honorsRuleLockScope } = await import("../../open-sse/config/providerErrorRules.ts"); + assert.equal(honorsRuleLockScope("agentrouter"), true); + assert.equal(honorsRuleLockScope("AgentRouter"), true); + assert.equal(honorsRuleLockScope("opencode"), false); + assert.equal(honorsRuleLockScope(null), false); +}); diff --git a/tests/unit/agentrouter-lock-scope-10334.test.ts b/tests/unit/agentrouter-lock-scope-10334.test.ts new file mode 100644 index 0000000000..1205ec5cd2 --- /dev/null +++ b/tests/unit/agentrouter-lock-scope-10334.test.ts @@ -0,0 +1,638 @@ +// #10334 — agentrouter EXCLUSIVE: markAccountUnavailable must honor the +// provider rule's declared lock scope instead of always deriving it from +// hasPerModelQuota(). agentrouter is a passthroughModels provider, so a +// naive account-wide quota exhaustion ("额度不足") would otherwise be treated +// as a per-model 429 and lock only ONE model, leaving combo routing to burn +// one upstream call per remaining model of the same exhausted account. This +// suite pins the connection-scoped cooldown behavior AND its invariants: +// never a terminal status, must also win when the caller is combo (isCombo), +// must not lock the model, and must be EXCLUSIVE to agentrouter — every other +// passthroughModels/compatible provider keeps today's per-model lockout. +import test from "node:test"; +import assert from "node:assert/strict"; +import fs from "node:fs"; +import os from "node:os"; +import path from "node:path"; + +const TEST_DATA_DIR = fs.mkdtempSync(path.join(os.tmpdir(), "omniroute-agentrouter-lock-")); +process.env.DATA_DIR = TEST_DATA_DIR; + +const core = await import("../../src/lib/db/core.ts"); +const providersDb = await import("../../src/lib/db/providers.ts"); +const auth = await import("../../src/sse/services/auth.ts"); +const accountFallback = await import("../../open-sse/services/accountFallback.ts"); +const { applyComboTargetExhaustion } = await import( + "../../open-sse/services/combo/targetExhaustion.ts" +); +const { classifyProviderError } = await import("../../open-sse/services/errorClassifier.ts"); + +const QUOTA_EXHAUSTED_429 = '{"error":{"message":"账户额度不足，请充值后重试"}}'; +const MODEL_ACCESS_DENIED_403 = '{"error":{"message":"无权访问模型 claude-opus-5"}}'; + +async function resetStorage() { + core.resetDbInstance(); + fs.rmSync(TEST_DATA_DIR, { recursive: true, force: true }); + fs.mkdirSync(TEST_DATA_DIR, { recursive: true }); +} + +async function seedConnection( + provider: string, + overrides: Record = {} +): Promise { + const conn = await providersDb.createProviderConnection({ + provider, + authType: "apikey", + apiKey: `${provider}-key`, + isActive: true, + testStatus: "active", + ...overrides, + }); + return (conn as Record).id as string; +} + +test.after(() => { + core.resetDbInstance(); + fs.rmSync(TEST_DATA_DIR, { recursive: true, force: true }); +}); + +test("agentrouter 429 account quota exhausted -> connection cooldown, never terminal", async () => { + await resetStorage(); + const connId = await seedConnection("agentrouter"); + + const result = await auth.markAccountUnavailable( + connId, + 429, + QUOTA_EXHAUSTED_429, + "agentrouter", + "claude-opus-5" + ); + + assert.equal(result.shouldFallback, true); + assert.ok(result.cooldownMs > 0, "connection cooldown must be positive"); + + const after = await providersDb.getProviderConnectionById(connId); + assert.equal(after.testStatus, "unavailable"); + assert.notEqual(after.testStatus, "credits_exhausted"); + assert.ok(after.rateLimitedUntil, "connection must carry a rateLimitedUntil"); + assert.ok( + new Date(String(after.rateLimitedUntil)).getTime() > Date.now(), + "rateLimitedUntil must be in the future" + ); +}); + +test("agentrouter 429 quota exhausted with isCombo: true still cools the connection (not a model lock)", async () => { + await resetStorage(); + const connId = await seedConnection("agentrouter"); + + const result = await auth.markAccountUnavailable( + connId, + 429, + QUOTA_EXHAUSTED_429, + "agentrouter", + "claude-opus-5", + null, + { isCombo: true, persistUnavailableState: false } + ); + + assert.equal(result.shouldFallback, true); + assert.ok(result.cooldownMs > 0); + + const after = await providersDb.getProviderConnectionById(connId); + assert.equal(after.testStatus, "unavailable"); + assert.notEqual(after.testStatus, "credits_exhausted"); + assert.ok(after.rateLimitedUntil, "connection must be cooled down even for combo callers"); +}); + +test("agentrouter quota cooldown does NOT lock the model", async () => { + await resetStorage(); + const connId = await seedConnection("agentrouter"); + + await auth.markAccountUnavailable( + connId, + 429, + QUOTA_EXHAUSTED_429, + "agentrouter", + "claude-opus-5" + ); + + const lockout = accountFallback.getModelLockoutInfo("agentrouter", connId, "claude-opus-5"); + assert.equal(lockout, null, "connection-scoped quota must not also record a model lockout"); +}); + +test("agentrouter 403 model-access-denied -> model lockout, connection stays active", async () => { + await resetStorage(); + const connId = await seedConnection("agentrouter"); + + const result = await auth.markAccountUnavailable( + connId, + 403, + MODEL_ACCESS_DENIED_403, + "agentrouter", + "claude-opus-5" + ); + + assert.equal(result.shouldFallback, true); + + const after = await providersDb.getProviderConnectionById(connId); + assert.equal(after.testStatus, "active"); + assert.ok(!after.rateLimitedUntil, "connection must not be rate-limited by a model-scoped rule"); + + // #3027's existing per-model-quota-provider branch handles this 403 (it is + // unmodified by #10334 except that it now reads the rule's declared + // cooldown via fallbackResult.baseCooldownMs) — the recorded reason stays + // the pre-existing hardcoded "forbidden", not the rule's "auth_error". + const lockout = accountFallback.getModelLockoutInfo("agentrouter", connId, "claude-opus-5"); + assert.equal(lockout?.reason, "forbidden"); + // The 6h base cooldown declared by the "agentrouter-model-access-denied" + // rule (open-sse/config/providerErrorRules.ts) must flow through as + // fallbackResult.baseCooldownMs instead of the generic + // COOLDOWN_MS.serviceUnavailable (2s) default — it then gets clamped down + // to the model-lockout maxCooldownMs setting (default 1_800_000ms / 30min) + // by recordModelLockoutFailure, same as every other model lockout. What + // this pins is that the rule's cooldown was consulted at all: a plain 2s + // default would be immediately visible as a tiny remainingMs, not ~max. + assert.ok( + lockout && lockout.remainingMs > 1_700_000, + `expected the rule cooldown to be clamped to ~maxCooldownMs (1_800_000ms), got ${lockout?.remainingMs}ms` + ); +}); + +test("exclusivity: ollama-cloud with an equivalent account-wide-looking 429 keeps today's per-model lockout, no connection cooldown", async () => { + await resetStorage(); + const connId = await seedConnection("ollama-cloud"); + + const result = await auth.markAccountUnavailable( + connId, + 429, + QUOTA_EXHAUSTED_429, + "ollama-cloud", + "claude-opus-5" + ); + + assert.equal(result.shouldFallback, true); + + const after = await providersDb.getProviderConnectionById(connId); + // ollama-cloud is NOT in the honorsRuleLockScope allowlist: today's + // per-model-quota behavior for a 429 must be unchanged — connection stays + // active, no rateLimitedUntil. + assert.equal(after.testStatus, "active"); + assert.ok(!after.rateLimitedUntil, "non-agentrouter providers must not gain connection cooldown"); + + // Positive assertion, not just the negative: the model lockout must have + // actually been recorded. Without this, a future refactor that stops + // locking anything for these providers would pass this test silently. + const lockout = accountFallback.getModelLockoutInfo("ollama-cloud", connId, "claude-opus-5"); + assert.ok(lockout, "expected the pre-existing per-model lockout to be recorded"); +}); + +test("exclusivity: vertex with an equivalent account-wide-looking 429 keeps today's per-model lockout, no connection cooldown", async () => { + await resetStorage(); + const connId = await seedConnection("vertex"); + + const result = await auth.markAccountUnavailable( + connId, + 429, + QUOTA_EXHAUSTED_429, + "vertex", + "claude-opus-5" + ); + + assert.equal(result.shouldFallback, true); + + const after = await providersDb.getProviderConnectionById(connId); + assert.equal(after.testStatus, "active"); + assert.ok(!after.rateLimitedUntil, "non-agentrouter providers must not gain connection cooldown"); + + // Positive assertion, not just the negative — see the ollama-cloud case above. + const lockout = accountFallback.getModelLockoutInfo("vertex", connId, "claude-opus-5"); + assert.ok(lockout, "expected the pre-existing per-model lockout to be recorded"); +}); + +// ─── Fix round 1 (#10334 review) ─────────────────────────────────────────── + +// Important finding: the "never terminal" invariant is not structurally +// guaranteed by `ruleScope === "connection"` alone — it depends on the +// provider rule table only ever pairing scope "connection" with a genuinely +// transient reason. isAgentrouterConnectionQuotaScope() is the actual guard; +// pin its predicate directly with synthetic fallbackResult shapes, since no +// rule in the current table produces a permanent/credits-exhausted result +// with scope "connection" (exercising it end-to-end would require editing +// the production rule table just for a test). +test("isAgentrouterConnectionQuotaScope: rejects a permanent rule result even with scope connection", () => { + const permanentConnectionScopeResult = { + ruleScope: "connection" as const, + reason: "auth_error", + permanent: true, + }; + assert.equal( + auth.isAgentrouterConnectionQuotaScope("agentrouter", permanentConnectionScopeResult), + false, + "a future permanent-state rule with scope connection must NOT take the transient-cooldown branch" + ); +}); + +test("isAgentrouterConnectionQuotaScope: rejects a credits-exhausted rule result even with scope connection", () => { + const creditsExhaustedConnectionScopeResult = { + ruleScope: "connection" as const, + reason: "quota_exhausted", + creditsExhausted: true, + }; + assert.equal( + auth.isAgentrouterConnectionQuotaScope("agentrouter", creditsExhaustedConnectionScopeResult), + false, + "a future credits-exhausted rule with scope connection must NOT take the transient-cooldown branch" + ); +}); + +test("isAgentrouterConnectionQuotaScope: accepts the real quota-exhausted/connection shape", () => { + const quotaConnectionScopeResult = { + ruleScope: "connection" as const, + reason: "quota_exhausted", + }; + assert.equal( + auth.isAgentrouterConnectionQuotaScope("agentrouter", quotaConnectionScopeResult), + true, + "today's only connection-scope rule result (quota_exhausted, no permanent/creditsExhausted) must pass" + ); +}); + +test("isAgentrouterConnectionQuotaScope: rejects non-agentrouter providers regardless of shape", () => { + const quotaConnectionScopeResult = { + ruleScope: "connection" as const, + reason: "quota_exhausted", + }; + assert.equal( + auth.isAgentrouterConnectionQuotaScope("ollama-cloud", quotaConnectionScopeResult), + false, + "honorsRuleLockScope must still gate every provider outside the agentrouter allowlist" + ); +}); + +// Minor finding: guard the branch's POSITION in markAccountUnavailable. If a +// future refactor moved the branch above the terminal-status guard (~line +// 2023) or the anti-thundering-herd guard (~line 2038), a credits_exhausted +// connection would be silently overwritten, or a live cooldown would be +// shortened — and the 6 tests above would stay green because none of them +// seed a connection with pre-existing terminal/cooldown state. +test("position guard: a connection already credits_exhausted stays terminal through an agentrouter quota 429", async () => { + await resetStorage(); + const connId = await seedConnection("agentrouter", { testStatus: "credits_exhausted" }); + + const result = await auth.markAccountUnavailable( + connId, + 429, + QUOTA_EXHAUSTED_429, + "agentrouter", + "claude-opus-5" + ); + + assert.equal(result.shouldFallback, true); + assert.equal(result.cooldownMs, 0, "terminal-status short-circuit returns cooldownMs 0"); + + const after = await providersDb.getProviderConnectionById(connId); + assert.equal( + after.testStatus, + "credits_exhausted", + "the connection-scope branch must never overwrite a pre-existing terminal status" + ); +}); + +test("position guard: an existing live cooldown is not shortened by the connection-scope branch", async () => { + await resetStorage(); + const futureCooldown = new Date(Date.now() + 10 * 60 * 1000).toISOString(); + const connId = await seedConnection("agentrouter", { + testStatus: "unavailable", + rateLimitedUntil: futureCooldown, + }); + + const result = await auth.markAccountUnavailable( + connId, + 429, + QUOTA_EXHAUSTED_429, + "agentrouter", + "claude-opus-5" + ); + + assert.equal(result.shouldFallback, true); + + const after = await providersDb.getProviderConnectionById(connId); + assert.equal( + after.rateLimitedUntil, + futureCooldown, + "the anti-thundering-herd guard must win: an existing live cooldown must not be reset/shortened" + ); +}); + +// Minor finding: disableCooling=true skips the connection-scope branch (the +// `!disableCooling` condition), so the #10334 bug survives for connections +// with that opt-out — they fall into the ~30min per-model lockout instead of +// the shorter connection cooldown. Documented in the block comment above the +// branch; pin the behavior so a future change to the guard is deliberate. +test("disableCooling=true skips the connection-scope branch and falls back to per-model lockout", async () => { + await resetStorage(); + const connId = await seedConnection("agentrouter", { + providerSpecificData: { disableCooling: true }, + }); + + const result = await auth.markAccountUnavailable( + connId, + 429, + QUOTA_EXHAUSTED_429, + "agentrouter", + "claude-opus-5" + ); + + assert.equal(result.shouldFallback, true); + + const after = await providersDb.getProviderConnectionById(connId); + // Connection is NOT cooled down — disableCooling's documented CONNECTION- + // level opt-out (#2997) is honored. + assert.equal(after.testStatus, "active"); + assert.ok(!after.rateLimitedUntil, "disableCooling must keep the connection selectable"); + + // But the model IS locked out instead (the #10334 bug's exact symptom for + // disableCooling connections — a deliberate, documented trade-off). + const lockout = accountFallback.getModelLockoutInfo("agentrouter", connId, "claude-opus-5"); + assert.ok( + lockout, + "expected a per-model lockout when disableCooling bypasses the connection branch" + ); +}); + +// ─── Task 3 (#10334): combo skips the exhausted agentrouter connection +// WITHIN THE SAME REQUEST ────────────────────────────────────────────────── +// The tests above pin markAccountUnavailable's PERSISTED connection cooldown +// — that only protects the NEXT request. applyComboTargetExhaustion (the +// #1731/#1731v2 shared classifier both combo dispatchers call after every +// target's upstream error — open-sse/services/combo/targetExhaustion.ts) is +// what decides whether remaining targets of the CURRENT request are skipped. +// Without a matching gate there, a combo with 5 legs on the same exhausted +// agentrouter account would still burn all 5 upstream calls before the +// persisted cooldown from the tests above ever kicks in. + +function comboSets() { + return { + exhaustedProviders: new Set(), + exhaustedConnections: new Set(), + transientRateLimitedProviders: new Set(), + }; +} + +function comboTarget(overrides: Record = {}) { + return { + kind: "model", + executionKey: "ek", + modelStr: "agentrouter/claude-opus-5", + provider: "agentrouter", + providerId: null, + connectionId: "conn-agentrouter-1", + ...overrides, + } as Parameters[0]; +} + +const comboLog = { info() {}, warn() {}, error() {}, debug() {} }; + +const comboBaseOpts = { + errorText: QUOTA_EXHAUSTED_429, + rawModel: "claude-opus-5", + isTokenLimitBreach: false, + allAccountsRateLimited: false, + requestScopedFailure: false, + log: comboLog, + tag: "COMBO", + exhaustedLogLevel: "info" as const, +}; + +// The real shape checkFallbackError surfaces for agentrouter's restated 429 +// (open-sse/config/providerErrorRules.ts's "agentrouter-user-quota-exhausted" +// rule: reason "quota_exhausted", scope "connection") — same shape pinned by +// isAgentrouterConnectionQuotaScope's own tests above. +const CONNECTION_SCOPE_FALLBACK_RESULT = { + ruleScope: "connection" as const, + reason: "quota_exhausted", +}; + +test("combo in-request skip: agentrouter connection-scope quota marks exhaustedConnections (#10334)", () => { + const sets = comboSets(); + const exhausted = applyComboTargetExhaustion(comboTarget(), { + ...comboBaseOpts, + result: { status: 429 }, + fallbackResult: CONNECTION_SCOPE_FALLBACK_RESULT, + sets, + }); + assert.equal( + exhausted, + true, + "combo must treat this like an exhausted target — no same-target retry" + ); + assert.ok( + sets.exhaustedConnections.has("agentrouter:conn-agentrouter-1"), + "the exhausted account's connection must be marked so remaining same-connection targets are skipped this request" + ); + assert.equal( + sets.exhaustedProviders.size, + 0, + "must NOT exhaust the whole provider — sibling agentrouter connections keep their own quota" + ); + // Important finding (review round 1): unlike markConnectionLevelExhaustion's + // path, this branch must NEVER populate transientRateLimitedProviders. That + // set drives combo.ts's `allowRateLimitedConnection` force-allow + // (open-sse/services/combo.ts:1005-1013 and :2734-2738), which bypasses the + // `rateLimitedUntil` filter in credential selection (src/sse/services/auth.ts:1238) + // for the provider's remaining legs this request. Marking it here would + // silently re-open the very connection Task 2's markAccountUnavailable (and + // this branch) just cooled down. + assert.equal( + sets.transientRateLimitedProviders.size, + 0, + "must NOT mark transientRateLimitedProviders — that would force-allow reusing the connection this branch just exhausted" + ); +}); + +test("combo in-request skip: no connectionId falls back to whole-provider exhaustion", () => { + const sets = comboSets(); + const exhausted = applyComboTargetExhaustion(comboTarget({ connectionId: null }), { + ...comboBaseOpts, + result: { status: 429 }, + fallbackResult: CONNECTION_SCOPE_FALLBACK_RESULT, + sets, + }); + assert.equal(exhausted, true); + assert.ok( + sets.exhaustedProviders.has("agentrouter"), + "no connectionId to scope to — must fall back to whole-provider, mirroring markAuthLevelExhaustion" + ); + assert.equal(sets.exhaustedConnections.size, 0); +}); + +test("exclusivity: an equivalent connection-scope-shaped result for ollama-cloud marks nothing (#10334 is agentrouter-only)", () => { + const sets = comboSets(); + // Synthetic: production never actually produces ruleScope for a + // non-allowlisted provider (honorsRuleLockScope gates it upstream inside + // checkFallbackError) — feeding it here directly proves + // applyComboTargetExhaustion ALSO re-checks the provider via + // isAgentrouterConnectionQuotaScope rather than trusting whatever shape + // it is handed. + const exhausted = applyComboTargetExhaustion( + comboTarget({ provider: "ollama-cloud", connectionId: "conn-ollama-1" }), + { + ...comboBaseOpts, + result: { status: 429 }, + fallbackResult: CONNECTION_SCOPE_FALLBACK_RESULT, + sets, + } + ); + assert.equal( + exhausted, + false, + "ollama-cloud must fall through to today's per-model-quota behavior unchanged" + ); + assert.equal(sets.exhaustedConnections.size, 0); + assert.equal(sets.exhaustedProviders.size, 0); +}); + +test("exclusivity: vertex with the same synthetic connection-scope result marks nothing", () => { + const sets = comboSets(); + const exhausted = applyComboTargetExhaustion( + comboTarget({ provider: "vertex", connectionId: "conn-vertex-1" }), + { + ...comboBaseOpts, + result: { status: 429 }, + fallbackResult: CONNECTION_SCOPE_FALLBACK_RESULT, + sets, + } + ); + assert.equal(exhausted, false); + assert.equal(sets.exhaustedConnections.size, 0); + assert.equal(sets.exhaustedProviders.size, 0); +}); + +test("guard: a permanent agentrouter fallbackResult with scope connection does NOT mark the connection exhausted here either", () => { + const sets = comboSets(); + const exhausted = applyComboTargetExhaustion(comboTarget(), { + ...comboBaseOpts, + result: { status: 429 }, + fallbackResult: { ruleScope: "connection" as const, reason: "auth_error", permanent: true }, + sets, + }); + assert.equal(exhausted, false); + assert.equal(sets.exhaustedConnections.has("agentrouter:conn-agentrouter-1"), false); + assert.equal(sets.exhaustedProviders.size, 0); +}); + +test("guard: a credits-exhausted agentrouter fallbackResult with scope connection does NOT mark the connection exhausted here either", () => { + const sets = comboSets(); + const exhausted = applyComboTargetExhaustion(comboTarget(), { + ...comboBaseOpts, + result: { status: 429 }, + fallbackResult: { + ruleScope: "connection" as const, + reason: "quota_exhausted", + creditsExhausted: true, + }, + sets, + }); + assert.equal(exhausted, false); + assert.equal(sets.exhaustedConnections.has("agentrouter:conn-agentrouter-1"), false); + assert.equal(sets.exhaustedProviders.size, 0); +}); + +// Minor finding (review round 1): the connection-scope branch is NOT +// 429-only. The "额度不足" rule (buildAgentrouterRules, providerErrorRules.ts) +// matches statuses {400, 403, 429}, and Task 1's FORBIDDEN pre-check +// (accountFallback.ts ~1729-1751, gated on honorsRuleLockScope) surfaces +// `ruleScope: "connection"` for a RAW 403 carrying that body too — before the +// generic apikey FORBIDDEN early-return, and before markAuthLevelExhaustion +// below ever sees it. Pin that a raw 403 with this shape takes the SAME +// connection-scope branch (not markAuthLevelExhaustion) and lands in the SAME +// set with the SAME key — the two paths are set-equivalent for agentrouter on +// this status, so this is not a behavior change, just documenting which +// branch actually runs. +// +// Fix round 2 finding: the Set-content assertions alone (exhausted===true, +// the connection key present, the other two sets empty) do NOT discriminate +// which branch ran — markAuthLevelExhaustion (the 401/403 branch below) +// produces the byte-identical Set effects for a 403 with a connectionId (same +// key, same untouched sibling sets, same `true` return), so deleting the new +// branch entirely would leave this test green. Use a log spy — the one real +// observable difference between the two paths — to prove the NEW branch +// actually fired: its message is tagged `#10334` / "account quota exhausted" +// (markAgentrouterConnectionQuotaExhaustion), never `#8133` / "auth failure" +// (markAuthLevelExhaustion). +function makeLogSpy() { + const calls: { level: string; tag: string; message: string }[] = []; + const record = (level: string) => (tag: string, message: string) => { + calls.push({ level, tag, message }); + }; + return { + calls, + log: { + info: record("info"), + warn: record("warn"), + error: record("error"), + debug: record("debug"), + }, + }; +} + +test("combo in-request skip: a RAW 403 with connection-scope quota also takes this branch (not markAuthLevelExhaustion)", () => { + const sets = comboSets(); + const spy = makeLogSpy(); + const exhausted = applyComboTargetExhaustion(comboTarget(), { + ...comboBaseOpts, + result: { status: 403 }, + fallbackResult: CONNECTION_SCOPE_FALLBACK_RESULT, + sets, + log: spy.log, + }); + assert.equal(exhausted, true); + assert.ok( + sets.exhaustedConnections.has("agentrouter:conn-agentrouter-1"), + "a raw 403 carrying ruleScope=connection must exhaust the connection just like the restated-429 case" + ); + assert.equal(sets.exhaustedProviders.size, 0); + assert.equal( + sets.transientRateLimitedProviders.size, + 0, + "same suppression as the 429 case — must not force-allow reusing this connection" + ); + // The discriminant: prove the NEW (#10334) branch emitted the log, not + // markAuthLevelExhaustion's (#8133) — the Set assertions above cannot tell + // the two apart on their own. + assert.equal(spy.calls.length, 1, "exactly one log call expected for this failure"); + assert.match( + spy.calls[0].message, + /#10334/, + "must be markAgentrouterConnectionQuotaExhaustion's log line, not markAuthLevelExhaustion's" + ); + assert.ok( + /account quota exhausted/.test(spy.calls[0].message), + "must carry the new branch's wording, not markAuthLevelExhaustion's 'auth failure'" + ); + assert.doesNotMatch( + spy.calls[0].message, + /#8133/, + "must NOT be markAuthLevelExhaustion's log line" + ); +}); + +// ─── Invariant sentinel ───────────────────────────────────────────────── +// classifyProviderError (open-sse/services/errorClassifier.ts) must NEVER +// classify agentrouter's restated 429 body ("用户额度不足") as quota_exhausted. +// If it ever does, open-sse/handlers/chatCore.ts's providerFailure handling +// (~line 3835-3856) can reach the terminal `else` branch +// (`testStatus: "credits_exhausted"`) for agentrouter whenever +// lockModelIfPerModelQuota does not itself claim the failure — turning a +// transient, self-recovering account-quota window into a connection that +// requires a manual operator reset. agentrouter is an apikey-category +// provider (not oauth), so shouldPreserveQuotaSignalsFor429 in +// errorClassifier.ts returns false for it and the 429 branch falls through +// to RATE_LIMITED instead — pin that this stays true. +test("sentinel: classifyProviderError never returns quota_exhausted for agentrouter's restated 429 body", () => { + const classification = classifyProviderError(429, "用户额度不足", "agentrouter"); + assert.notEqual( + classification, + "quota_exhausted", + "a quota_exhausted classification here would route agentrouter's transient account quota into chatCore's terminal credits_exhausted branch (~chatCore.ts:3849)" + ); +});