[maxmad64bis]

⚠️ base-red inherited: #18791

Summary

A 403 without a quota signal in its body now locks the exact provider/connection/model tuple with the lockout reason auth_error, while a 403 that proves quota exhaustion still locks the connection's quota family with the reason quota_exhausted. Every 403 — a restricted key, a forbidden route, or an unauthorized model — reads as quota_exhausted on the release base and locks the connection's quota family, so a refused model parks every sibling model of the connection for the model-lockout cooldown. The reason and the scope now both read the same quota-proof signal from the error body, which is already available at the two combo lockout sites.

Related Issues

• No linked issue — found while reading the combo lockout reason code

Target Branch

• Base is the active release/v3.8.x branch (the highest open release/v*), not main
• New feature? From 3.8.55 on it is held with the v4-feature label for the v4 channel

Validation

• Change type: routing
• Focused tests and category gates from the golden path — inherited: 2 red test file(s) among those touched also fail on the release base (see the banner above)
• npm run lint
• Reconciled with the current active release base; focused checks rerun afterward
• Production-code changes include a new or updated automated test in this PR

Tests Added Or Updated

• tests/unit/classifyLockoutReason-403.test.ts: seven unit tests for the 403 reason split (no body, quota signal, auth signal, empty and null body) and for the unchanged 429 and other-status reasons.
• tests/unit/model-lockout-403-scope.test.ts: seven tests pinning the lockout scope, replayed through recordModelLockoutFailure and handleComboChat: an unproven 403 locks the exact tuple only and the sibling model still dispatches, a 403 with quota proof parks the family, a non-JSON 403 body whose quota signal sits past the 500-char truncation still lands as auth_error with the exact-model lock, and exact and family keys are distinct namespaces.

Coverage Notes

The changed production code lives in open-sse/services/accountFallback.ts and open-sse/services/combo/executeTargetAttempt.ts, and both the new reason split and the new scope override are exercised by the two test files above.

Reviewer Notes

• Behavior: the reason and the scope share the same quota-proof signal, so the two cannot diverge; the proof is read from the error body that is truncated to its first 500 characters, and the default family lockout and the cooldowns are untouched.
• Behavior: the scope override reaches only the two combo lockout sites; the Antigravity path already passes the exact scope, and 429, 404 and 5xx keep their reasons and scopes.

[saylee21]

One edge worth pinning: errorText only falls back to text.substring(0, 500) when the body does not parse as JSON (decodeBodyText in attemptFailure.ts); a JSON body gets error.message whole. So the cut bites on a body that fails that parse, for example an HTML error page in front of the API, or JSON wrapped in enough padding to break it.

In that case a quota phrase past the 500-char boundary is invisible to looksLikeQuotaExhausted, the 403 classifies as auth_error, and the exact-model scope leaves the exhausted connection serving sibling models — the opposite of what the family scope is for.

Would you add a test with a non-JSON body whose quota signal sits beyond the truncation boundary, so the fallback path is pinned alongside the parsed-JSON cases?

[maxmad64bis]

Good catch — that fallback only bites when the body does not parse as JSON (an HTML error page in front of the API, or JSON broken by padding), so a quota phrase past the 500-char boundary stays invisible to the classifier: the 403 lands as an auth_error with exact-model scope, and the exhausted connection can keep serving sibling models.

Yes — we are adding a test that pins that fallback path: a non-JSON body with the quota signal placed beyond the truncation boundary, asserting the auth_error / exact-model outcome, alongside the existing parsed-JSON cases. It lands in this PR.

Thanks for pointing it out.

[coderabbitai]

No actionable comments were generated in the recent review.

Summary

Summary by CodeRabbit

• Bug Fixes
  • A 403 response without evidence of quota exhaustion now locks only the affected provider, connection, and model, rather than the broader quota family.
  • A 403 response with quota-exhaustion evidence continues to lock the quota family.
  • 403 errors without quota evidence are classified as authentication errors; confirmed quota exhaustion is classified separately.

403 lockout classification now uses error text to distinguish quota exhaustion from other 403 errors. Unproven 403 responses use exact-model scope, while quota-confirmed 403 responses use quota-family scope. Both lockout recording paths apply the classification, with tests covering retry behavior.

Changes: Provider 403 Lockouts

Layer / File(s)	Summary
Classify 403 lockout reason and scope open-sse/services/accountFallback.ts, tests/unit/classifyLockoutReason-403.test.ts, changelog.d/fixes/18652-provider-403-quota-exhausted-connection-lock.md	classifyLockoutReason uses error text to distinguish quota exhaustion from other 403 errors. classifyLockoutScope selects exact-model scope when a 403 has no quota evidence. Tests cover status and error-text classifications.
Apply classification to combo attempts open-sse/services/combo/executeTargetAttempt.ts, tests/unit/model-lockout-403-scope.test.ts, stryker.conf.json	Both lockout recording paths pass the status and error text to the classifiers. Tests cover lock recording, retry dispatch, and exact-model versus family lock-key parsing.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Change: Bug fix

Merge Risk: Minimal · up to 56027

A 403 without quota evidence now locks only the failing model, and a 403 with quota evidence still locks the whole quota family. No actionable merge-blocking risk was identified in the reviewed change.

Security architecture risk: Low · up to 56027

The change narrows the effect of authorization failures rather than expanding access or privileges. Existing lock expiration and recovery mechanisms support the narrower scope. Residual uncertainty concerns differing quota classifications and shared-connection isolation, not a demonstrated authorization bypass.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

• observed — An exact lock names the canonical provider, connection identifier and normalized model. A quota-family lock retains the existing provider-and-connection boundary with provider-specific family grouping. For a given tuple, the PR narrows unproven 403 effects; quota-matching text cannot create a broader family scope than the base already applied to every 403.

Trust Boundaries and Controls

• inferred — The changed trust decision concerns routing-health state, not permission to access a provider. Response text can select exact versus family lockout but does not supply credentials or replace the resolved lock identity. Whether an upstream can reflect attacker-controlled text, and whether shared connection identifiers provide tenant isolation, remains unestablished.

Resilience and Maintainability Implications

• observed — The richer quota verdict still marks identified connections exhausted independently of lock scope, including structured balance-exhaustion 403s. Both auth_error and quota_exhausted remain excluded from short cooldown-wait retries. These are countercontrols to treating narrower scope or changed reason alone as a demonstrated retry or quota-control bypass.

Pre-merge checks: 6 · 1

Failed checks: 1 warning

Check name	Status	Explanation	Resolution
Docstring Coverage		Docstring coverage is 33.33% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documenta…	Write docstrings for the functions missing them to satisfy the coverage threshold.

Passed checks: 6 passed

Check name	Status	Explanation
Title check		The title follows the required Conventional Commits format and accurately describes the exact-model lockout change for 403 responses without quota proof.
Description check		The description includes all required sections, explains the behavior change, identifies the target branch, documents validation and inherited test failures, lists updated tests, provides coverage not…
Linked Issues check		Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check		Check skipped because no linked issues were found for this pull request.
Tests For Production Code		Production code changed under open-sse/services/accountFallback.ts and open-sse/services/combo/executeTargetAttempt.ts. The PR adds two unit test files, tests/unit/classifyLockoutReason-403.test.ts an…
No Ai Attribution		No prohibited AI attribution is present. The three reviewed commits have no Co-Authored-By trailers and no Generated with footer. The supplied PR title and body also contain no prohibited AI assistant…

Full details: Description check

Explanation

The description includes all required sections, explains the behavior change, identifies the target branch, documents validation and inherited test failures, lists updated tests, provides coverage notes, and calls out reviewer risks.

Full details: Tests For Production Code

Explanation

Production code changed under open-sse/services/accountFallback.ts and open-sse/services/combo/executeTargetAttempt.ts. The PR adds two unit test files, tests/unit/classifyLockoutReason-403.test.ts and tests/unit/model-lockout-403-scope.test.ts, and registers both in stryker.conf.json.

Full details: No Ai Attribution

Explanation

No prohibited AI attribution is present. The three reviewed commits have no Co-Authored-By trailers and no Generated with footer. The supplied PR title and body also contain no prohibited AI assistant or bot attribution.

Full details: Docstring Coverage

Explanation

Docstring coverage is 33.33% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 3 functions across 4 files. (2 skipped: 2 unsupported.)

Resolution

Write docstrings for the functions missing them to satisfy the coverage threshold.

Finishing Touches

Generate unit tests (beta)

• Create a new PR

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Usage-based review receipt

• Mode: Continue automatically
• Reviewed files: 6
• Waived: $1.50 (charged $0.00)
• View usage details

Note

This review exceeded your plan’s limits and used usage-based reviews—free during trial, billed after paid activation unless disabled. Manage usage-based reviews.

Recent review info

Run configuration

• Source mode: Auto
• Repository: cheaper-inference/OmniRoute
• Branch: release/v3.8.53
• File: .coderabbit.yaml
• Commit: f000c11
• Layers: Repository YAML
• Review profile: CHILL
• Plan: Advanced
• Run ID: fc7b4b3b-6317-423c-815e-af138116c075

Commits

Reviewing files that changed from the base of the PR and between f000c11 and 5602752.

Files selected for processing: 6

• changelog.d/fixes/18652-provider-403-quota-exhausted-connection-lock.md
• open-sse/services/accountFallback.ts
• open-sse/services/combo/executeTargetAttempt.ts
• stryker.conf.json
• tests/unit/classifyLockoutReason-403.test.ts
• tests/unit/model-lockout-403-scope.test.ts

Limit details: You’ve used all 10 included reviews currently available.