feat(sse): honor provider-rule lock scope for agentrouter (connection vs model) - #10419
Merged
Merged
Conversation
…agentrouter-only) Add honorsRuleLockScope() as an exclusive allowlist (agentrouter today) and surface the matched ProviderErrorRule's scope as checkFallbackError's new ruleScope field. The agentrouter 403 path now consults the provider rules before the generic apikey-FORBIDDEN early-return, so a recognized body like "无权访问模型" carries the rule's declared reason/cooldown/scope instead of the generic short auth cooldown. Every other provider's behavior is unchanged — ruleScope stays undefined outside the allowlist.
markAccountUnavailable now consults checkFallbackError's ruleScope (#10334 Task 1) before the generic per-model-quota branch: when honorsRuleLockScope(provider) && ruleScope === "connection" (agentrouter account-wide "额度不足" quota exhaustion today), it cools the whole connection instead of locking a single passthrough model — including when the caller is combo (isCombo/persistUnavailableState:false), which would otherwise downgrade to a per-model lock. Never sets a terminal status. Exclusive to agentrouter; every other passthroughModels provider (ollama-cloud, vertex, ...) keeps today's per-model lockout byte-for-byte. The existing #3027 model-lockout branch for agentrouter's 403 "无权访问模型" (ruleScope "model") is unmodified.
…er states Adversarial review of the #10334 Task 2 connection-scope branch found the "never terminal" invariant relied only on ruleScope === "connection", which is not structurally guaranteed against a future agentrouter rule pairing that scope with a permanent/credits-exhausted reason. Extract the guard into an exported, independently-testable predicate (isAgentrouterConnectionQuotaScope) that also requires reason === QUOTA_EXHAUSTED and !permanent/!creditsExhausted. Also: document the disableCooling(#2997) interaction and the providerErrorRules.ts 6h-vs-30min-cap discrepancy the review flagged, and add position-guard + exclusivity-positive tests so a future refactor that reorders the branch or stops locking non-agentrouter providers cannot pass silently.
…equest applyComboTargetExhaustion now marks an agentrouter connection into the in-memory exhaustedConnections set when checkFallbackError reports a connection-scope quota result (isAgentrouterConnectionQuotaScope, reused from the persistence layer), so remaining same-connection targets in the SAME combo request are skipped instead of each burning its own upstream call before the persisted cooldown takes effect on the next request. Gated strictly to the agentrouter allowlist — every other provider is unaffected. Also updates RESILIENCE_GUIDE.md §7 to correct two stale claims: the agentrouter-model-access-denied rule does fire in production now (feeds the per-model lockout's cooldown), and rule scope is consumed end-to-end for providers in HONORS_RULE_LOCK_SCOPE_PROVIDERS instead of staying purely informational.
…p claims Fix round 1 from review of the agentrouter same-request combo skip (#10334): document and test that the new branch deliberately never populates transientRateLimitedProviders (it would re-open the connection the branch just exhausted via combo.ts's allowRateLimitedConnection force-allow), correct a code comment claiming the branch is 429-only (the quota rule also matches a raw 403, which lands in the same set via the same guard), and fix two RESILIENCE_GUIDE.md claims about the same-request skip: it only matches targets that already carry the exhausted connectionId, and the persisted cooldown was never gated on "next request" timing.
…typing The raw-403 test for the agentrouter connection-scope combo-skip branch only asserted Set contents, which markAuthLevelExhaustion produces identically for a 403 with a connectionId — deleting the new branch would have left it green. Add a local log spy and assert the emitted message is the new branch's (#10334 / "account quota exhausted"), not markAuthLevelExhaustion's (#8133 / "auth failure"). Also fixes a pre-existing tsc error in the same file: getProviderConnectionById returns rateLimitedUntil as unknown, which new Date() can't accept directly.
…router-lock-scope
…router-lock-scope
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #10334.
What was wrong
The
scopefield that provider error rules declare (open-sse/config/providerErrorRules.ts) was informational only — no production code read it. For agentrouter (passthroughModels: true→hasPerModelQuota), an account-wide quota exhaustion (额度不足, restated 403/400→429 by #10335) therefore landed in per-model lockout: every remaining model of the exhausted account still burned one upstream call before locking itself. Andagentrouter-model-access-denied(403 +无权访问模型) never fired at all, becausecheckFallbackError's apikey-FORBIDDEN branch returned before the provider-rule lookup.What changed (agentrouter-only, by owner decision)
The issue suggested wiring this generically; the owner scoped it to agentrouter so no other provider changes behavior. Exclusivity is double-gated and covered by tests.
checkFallbackErrorsurfacesruleScope— populated only whenhonorsRuleLockScope(provider)is true (newHONORS_RULE_LOCK_SCOPE_PROVIDERSallowlist, deliberately separate fromFULL_TEXT_RULE_PROVIDERS: one controls what a rule matches against, the other whether the matched scope changes caller behavior). The 403 path now consults the rules before the generic apikey early-return.markAccountUnavailablehonorsscope: "connection"— applies a temporary connection cooldown (rateLimitedUntil+testStatus: "unavailable") instead of a per-model lockout, and wins over the combo path that would otherwise downgrade tolockModel. Guarded byisAgentrouterConnectionQuotaScope(), which additionally requiresreason === QUOTA_EXHAUSTED && !permanent && !creditsExhaustedso a future permanent-state rule withscope: "connection"can never be silently downgraded to a 3s transient retry loop.applyComboTargetExhaustionmarks the connection intoexhaustedConnections. The early return also deliberately stops markingtransientRateLimitedProviders, which is what previously becameallowRateLimitedConnection: trueand force-reused the connection that had just been cooled down.Documented in
docs/architecture/RESILIENCE_GUIDE.md§7, including the honest limits:disableCooling: trueconnections keep the old per-model behavior, the rule's declared 6h model-access cooldown is clamped bymlSettings.maxCooldownMs(30 min by default), and the same-request skip only applies to targets that carry their ownconnectionId.Validation
markAccountUnavailableguard-rail + combo target-exhaustion).ollama-cloud/vertexkeep per-model lockout with the connection active (asserted positively, not just negatively).markAuthLevelExhaustionon the raw-403 path (their set effects are identical).classifyProviderError(429, "用户额度不足", "agentrouter")never becomesquota_exhausted— otherwisechatCore's terminalcredits_exhaustedbranch would become reachable.typecheck:core,check:cycles,check:docs-all,check:mutation-test-coverage, eslint on touched files — all clean.