From c522761ce58db62b9117d3bb2ec491a15ea5f1ca Mon Sep 17 00:00:00 2001
From: Hernan Javier Ardila Sanchez <hjasgr@gmail.com>
Date: Sun, 7 Jun 2026 16:13:29 +0200
Subject: [PATCH] feat(error-rules): provider-specific error classification
 with scope (#3370)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Integrated into release/v3.8.15. PR has genuine value beyond #3369: (1) getProviderErrorRuleMatch now accepts native Headers objects from fetch(); (2) checkFallbackError also uses the provider rule registry — the real end-to-end wiring in the combo fallback path; (3) S4 end-to-end test proving the wiring fires. Merge commit on contributor branch resolved the add/add conflict by taking the #3370 version throughout.
---
 open-sse/config/providerErrorRules.ts   | 10 ++++++--
 open-sse/services/accountFallback.ts    | 16 +++++++++++-
 tests/unit/provider-error-rules.test.ts | 34 ++++++++++++++++++++++++-
 3 files changed, 56 insertions(+), 4 deletions(-)

diff --git a/open-sse/config/providerErrorRules.ts b/open-sse/config/providerErrorRules.ts
index 2bd1ca1e7c..d9f2797fe3 100644
--- a/open-sse/config/providerErrorRules.ts
+++ b/open-sse/config/providerErrorRules.ts
@@ -125,13 +125,19 @@ export const providerRuleRegistry = new Map<string, ProviderErrorRule[]>([
 export function getProviderErrorRuleMatch(
   provider: string | null | undefined,
   status: number,
-  headers: Record<string, string> | null | undefined,
+  headers: Headers | Record<string, string> | null | undefined,
   body?: unknown
 ): ProviderErrorRuleMatch | null {
   if (!provider) return null;
   const rules = providerRuleRegistry.get(provider);
   if (!rules) return null;
-  const safeHeaders = headers ?? {};
+  // Normalize headers: accept either a `Headers` object (from `fetch()`) or
+  // a plain record. Provider rules access headers via plain object indexing.
+  const safeHeaders: Record<string, string> = !headers
+    ? {}
+    : typeof (headers as Headers).get === "function"
+      ? Object.fromEntries((headers as Headers).entries())
+      : (headers as Record<string, string>);
   for (const rule of rules) {
     const match = rule.match({ status, headers: safeHeaders, body });
     if (match) return match;
diff --git a/open-sse/services/accountFallback.ts b/open-sse/services/accountFallback.ts
index a04f6a4b08..690abf8609 100644
--- a/open-sse/services/accountFallback.ts
+++ b/open-sse/services/accountFallback.ts
@@ -1391,7 +1391,21 @@ export function checkFallbackError(
       : findMatchingErrorRule(status, errorStr);
   if (configuredRule) {
     if (configuredRule.backoff) {
-      return buildRetryableFallback(configuredRule.reason ?? classifyError(status, errorStr));
+      // Provider-specific rules in `providerRuleRegistry` are MORE SPECIFIC
+      // than the configured (global) rule, so we check them first. If a
+      // provider rule matches, it overrides the configured rule's reason
+      // (e.g. Opencode's `x-ratelimit-remaining-requests: 0` overrides
+      // 429 → RATE_LIMIT_EXCEEDED). We do NOT call the full `classifyError`
+      // here because its global status fallback would otherwise override
+      // specific configured reasons (e.g. 503 → SERVER_ERROR would be
+      // shadowed by 503 → MODEL_CAPACITY).
+      const providerMatch = provider
+        ? getProviderErrorRuleMatch(provider, status, headers, structuredError ?? null)
+        : null;
+      const reason = providerMatch
+        ? providerMatch.reason
+        : (configuredRule.reason ?? RateLimitReason.UNKNOWN);
+      return buildRetryableFallback(reason);
     }
     const cooldownMs = configuredRule.cooldownMs ?? 0;
     return {
diff --git a/tests/unit/provider-error-rules.test.ts b/tests/unit/provider-error-rules.test.ts
index fd507af98d..aac4f820a1 100644
--- a/tests/unit/provider-error-rules.test.ts
+++ b/tests/unit/provider-error-rules.test.ts
@@ -14,7 +14,7 @@ import assert from "node:assert/strict";
  *   - Anything else: falls back to global ERROR_RULES.
  */
 
-const { classifyError } = await import(
+const { classifyError, checkFallbackError } = await import(
   "../../open-sse/services/accountFallback.ts"
 );
 const { RateLimitReason } = await import(
@@ -86,3 +86,35 @@ test("S3: Regression — provider with no rules falls back to global ERROR_RULES
   const reasonNoCtx = classifyError(429, "rate limit reached");
   assert.equal(reasonNoCtx, RateLimitReason.RATE_LIMIT_EXCEEDED);
 });
+
+test("S4: End-to-end — checkFallbackError forwards provider+headers to classifyError", () => {
+  // The wiring test: when combo.ts calls checkFallbackError with provider=opencode
+  // and headers containing x-ratelimit-remaining-requests: 0, the reason must be
+  // QUOTA_EXHAUSTED (not RATE_LIMIT_EXCEEDED). This proves the registry is ACTIVE
+  // in the production fallback path, not just callable in isolation.
+  //
+  // Simulate what combo.ts:3849 does — it passes provider, headers, and structuredError.
+  // For Opencode with account-wide quota exhausted, the fallback should signal
+  // quota_exhausted so the combo router skips remaining targets from the same provider.
+  const result = checkFallbackError(
+    429,
+    "rate limit reached", // generic body that would normally be RATE_LIMIT_EXCEEDED
+    0, // backoffLevel
+    null, // model
+    "opencode", // provider
+    { "x-ratelimit-remaining-requests": "0" }, // headers
+    null, // profileOverride
+    null // structuredError
+  );
+
+  assert.equal(
+    result.reason,
+    RateLimitReason.QUOTA_EXHAUSTED,
+    "checkFallbackError must forward provider+headers to classifyError so the Opencode quota rule fires"
+  );
+  assert.equal(
+    result.shouldFallback,
+    true,
+    "quota_exhausted must trigger fallback to the next provider"
+  );
+});