fix(sse): thread errorText into shouldMarkAccountExhaustedFrom429 (#13008)

* fix(sse): thread errorText into shouldMarkAccountExhaustedFrom429

shouldPreserveQuotaSignals() (open-sse/services/quotaResetParsing.ts) gained
an errorText parameter so an explicit quota-exhausted body could override the
apikey-category default, but only one of its two call sites was updated.
checkFallbackError() passes the upstream body; shouldMarkAccountExhaustedFrom429()
still called it with the provider alone.

With errorText undefined the helper's
`Boolean(errorText) && looksLikeQuotaExhausted(errorText)` branch can never be
true, so for every apikey-category provider without per-model quotas the
connection was never marked quota-exhausted -- even when the upstream body
explicitly said a long-window cap was hit.

Thread errorText through the helper and pass it at the src/sse/handlers/chat.ts
call site. The parameter is optional and additive: OAuth-category providers and
callers that pass no body keep their existing behavior, and plain rate limits
("Rate limit exceeded, retry in 20s", "Too Many Requests") still fall through to
the short generic cooldown.

Regression guard: tests/unit/quota-signal-errortext-threading.test.ts

* test(sse): pin the chat.ts call site that forwards errorText

The errorText parameter added in the previous commit is optional, so dropping
it at the only production call site (src/sse/handlers/chat.ts) is neither a
type error nor a test failure -- the four existing cases call the helper
directly and none of them assert the wiring. The half of the patch that makes
it do anything in production could be reverted, or lost in a refactor, with
the whole suite green. That is the same failure mode this branch fixes (a
two-argument helper with a call site silently passing one), one level up.

handleSingleModelChat is not exported, so the call cannot be driven or spied
without widening the production surface. Assert at the source level instead,
following tests/unit/api-key-provider-quota-bypass-scope.test.ts, and parse
the argument list rather than regex-matching formatted text so a Prettier
reflow cannot cause a false failure or a false pass. Also pins that there is
exactly one call site, and what errorStr holds.

Verified by mutation: dropping errorStr at chat.ts now fails 1 of 5.

* chore: number quota signal changelog fragment

Co-Authored-By: Paperclip <noreply@paperclip.ing>

---------

Co-authored-by: TogetherWeOwn <eng@togetherweown.com>
Co-authored-by: togetherweown[bot] <togetherweown[bot]@users.noreply.github.com>
Co-authored-by: Paperclip <noreply@paperclip.ing>

changelog.d/fixes/13008-quota-signal-errortext-threading.md

@@ -0,0 +1 @@
+- **fix(resilience):** An apikey-category 429 whose body explicitly says a long-window quota was exhausted no longer skips the quota cache — `shouldPreserveQuotaSignals()` (`open-sse/services/quotaResetParsing.ts`) gained an `errorText` parameter in the #6638 fix, but only one of its two call sites was updated: `checkFallbackError()` passes the upstream body while `shouldMarkAccountExhaustedFrom429()` (`open-sse/services/accountFallback.ts`) still called it with the provider alone. With `errorText` undefined the helper's `Boolean(errorText) && looksLikeQuotaExhausted(errorText)` branch can never be true, so for every apikey-category provider without per-model quotas the connection was never marked quota-exhausted. `errorText` is now threaded through the helper and passed at the `src/sse/handlers/chat.ts` call site. Plain rate limits (`Rate limit exceeded, retry in 20s`, `Too Many Requests`) still fall through to the short generic cooldown. Regression guard: `tests/unit/quota-signal-errortext-threading.test.ts`.

open-sse/services/accountFallback.ts

@@ -1009,15 +1009,21 @@ export function shouldMarkAccountExhaustedFrom429(
   provider: string | null | undefined,
   model: string | null | undefined = null,
   connectionPassthroughModels?: boolean,
-  failureKind?: FailureKind
+  failureKind?: FailureKind,
+  errorText?: string | null
 ): boolean {
   // A plain 429 means transient rate limiting / high traffic for many OAuth providers.
   // Only connection-poison the quota cache when the upstream body explicitly says
   // the long-window quota is exhausted; otherwise fallback should try another account
   // without making this one look quota-depleted for 5 minutes.
   if (failureKind === "rate_limit" || failureKind === "transient") return false;
+  // `errorText` is what lets an apikey-category provider opt back in: without the
+  // upstream body, `shouldPreserveQuotaSignals` has nothing to match against
+  // `looksLikeQuotaExhausted`, so every apikey 429 reads as plain rate limiting —
+  // including one whose body explicitly says a daily/weekly/monthly cap was hit.
+  // Mirrors the two-argument call in `checkFallbackError` below.
   return (
-    shouldPreserveQuotaSignals(provider) &&
+    shouldPreserveQuotaSignals(provider, errorText) &&
     !hasPerModelQuota(provider, model, connectionPassthroughModels)
   );
 }

src/sse/handlers/chat.ts

@@ -2411,7 +2411,13 @@ async function handleSingleModelChat(
         const passthroughModels = credentials.providerSpecificData?.passthroughModels;
         if (
           result.status === 429 &&
-          shouldMarkAccountExhaustedFrom429(provider, model, passthroughModels, failureKind) &&
+          shouldMarkAccountExhaustedFrom429(
+            provider,
+            model,
+            passthroughModels,
+            failureKind,
+            errorStr
+          ) &&
           // T-PROBE: a probe must not poison the 5min quotaCache for real
           // traffic (#9817).
           !(await shouldIsolateProbeFailures())

tests/unit/quota-signal-errortext-threading.test.ts

@@ -0,0 +1,202 @@
+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";
+import { fileURLToPath } from "node:url";
+
+// #10460 pattern: DATA_DIR must be assigned BEFORE any transitive DB import.
+// accountFallback.ts statically imports `@/lib/db/providers` -> `src/lib/db/core.ts`,
+// whose DATA_DIR is captured once at module-load time.
+const TEST_DATA_DIR = fs.mkdtempSync(path.join(os.tmpdir(), "omniroute-quota-errortext-"));
+process.env.DATA_DIR = TEST_DATA_DIR;
+process.env.API_KEY_SECRET = process.env.API_KEY_SECRET || "quota-errortext-test-secret";
+
+const { shouldMarkAccountExhaustedFrom429 } =
+  await import("../../open-sse/services/accountFallback.ts");
+
+/**
+ * `shouldPreserveQuotaSignals(provider, errorText)` (open-sse/services/quotaResetParsing.ts)
+ * gained its second parameter with the #6638 fix, but only ONE of its two call sites was
+ * updated: `checkFallbackError` passes `errorText`, while
+ * `shouldMarkAccountExhaustedFrom429` still called it with the provider alone. With
+ * `errorText` undefined the helper's `Boolean(errorText) && looksLikeQuotaExhausted(...)`
+ * branch can never be true, so for every apikey-category provider the quota cache was
+ * never marked exhausted — even when the upstream body explicitly said a long-window cap
+ * was hit. These cases pin both directions of the now-threaded argument.
+ */
+
+// An explicit long-window quota body — the exact shape #6638 was reported with.
+const QUOTA_EXHAUSTED_BODY = JSON.stringify({
+  error: "You have exceeded your weekly usage quota. Your quota will reset in 3 days.",
+});
+
+test("shouldMarkAccountExhaustedFrom429 seeds the quota cache for an apikey 429 whose body says the quota is exhausted", () => {
+  // `openai` is apikey-category and has no per-model quota, so the result is decided
+  // purely by whether the body-text quota signal reaches shouldPreserveQuotaSignals.
+  assert.equal(
+    shouldMarkAccountExhaustedFrom429(
+      "openai",
+      "gpt-4o-mini",
+      undefined,
+      undefined,
+      QUOTA_EXHAUSTED_BODY
+    ),
+    true
+  );
+  assert.equal(
+    shouldMarkAccountExhaustedFrom429(
+      "anthropic",
+      "claude-sonnet-4-6",
+      undefined,
+      undefined,
+      QUOTA_EXHAUSTED_BODY
+    ),
+    true
+  );
+});
+
+test("shouldMarkAccountExhaustedFrom429 still ignores a plain apikey rate limit", () => {
+  // Neither body matches QUOTA_PATTERNS, so a plain 429 must keep falling through to the
+  // short generic cooldown instead of poisoning the connection's quota cache.
+  assert.equal(
+    shouldMarkAccountExhaustedFrom429(
+      "openai",
+      "gpt-4o-mini",
+      undefined,
+      undefined,
+      "Rate limit exceeded, retry in 20s"
+    ),
+    false
+  );
+  assert.equal(
+    shouldMarkAccountExhaustedFrom429(
+      "openai",
+      "gpt-4o-mini",
+      undefined,
+      undefined,
+      "Too Many Requests"
+    ),
+    false
+  );
+});
+
+test("shouldMarkAccountExhaustedFrom429 keeps its pre-existing behavior when no errorText is supplied", () => {
+  // The new parameter is optional and additive: OAuth-category providers still preserve
+  // quota signals unconditionally, and apikey-category ones still default to "not
+  // exhausted" without an explicit body signal.
+  assert.equal(shouldMarkAccountExhaustedFrom429("claude", "claude-sonnet-4-6"), true);
+  assert.equal(shouldMarkAccountExhaustedFrom429("openai", "gpt-4o-mini"), false);
+});
+
+test("shouldMarkAccountExhaustedFrom429 lets a transient failureKind win over a quota body", () => {
+  // The failureKind short-circuit runs before the body-text check and must stay that way:
+  // a 429 the classifier already called transient never poisons the quota cache.
+  assert.equal(
+    shouldMarkAccountExhaustedFrom429(
+      "openai",
+      "gpt-4o-mini",
+      undefined,
+      "rate_limit",
+      QUOTA_EXHAUSTED_BODY
+    ),
+    false
+  );
+  assert.equal(
+    shouldMarkAccountExhaustedFrom429(
+      "openai",
+      "gpt-4o-mini",
+      undefined,
+      "transient",
+      QUOTA_EXHAUSTED_BODY
+    ),
+    false
+  );
+});
+
+/**
+ * The cases above pin the helper. This one pins the WIRING, and it is the reason the
+ * fix does anything in production.
+ *
+ * `errorText` is an OPTIONAL 5th parameter, so dropping it at the call site is neither a
+ * type error nor a helper-test failure — exactly the shape of the bug being fixed (a
+ * two-argument helper whose call site silently passes one). Without this case the
+ * production half of the patch could be reverted, or lost in a refactor, with the whole
+ * suite green.
+ *
+ * `handleSingleModelChat` is not exported from `src/sse/handlers/chat.ts`, so the call
+ * cannot be driven or spied without changing the production surface. A source-level
+ * assertion is the precedent for that situation in this suite — see
+ * `tests/unit/api-key-provider-quota-bypass-scope.test.ts`. Parse the argument list
+ * rather than regex-matching the formatted text, so Prettier reflowing the call cannot
+ * turn this guard into a false failure (or, worse, a false pass).
+ */
+const repoRoot = path.resolve(path.dirname(fileURLToPath(import.meta.url)), "../..");
+
+/** Top-level (paren/bracket/brace-depth 0) comma split of one argument list. */
+function splitTopLevelArgs(argList: string): string[] {
+  const args: string[] = [];
+  let depth = 0;
+  let current = "";
+  for (const ch of argList) {
+    if (ch === "(" || ch === "[" || ch === "{") depth++;
+    else if (ch === ")" || ch === "]" || ch === "}") depth--;
+    if (ch === "," && depth === 0) {
+      args.push(current.trim());
+      current = "";
+      continue;
+    }
+    current += ch;
+  }
+  if (current.trim().length > 0) args.push(current.trim());
+  return args;
+}
+
+/** Every `fn(...)` call in `source`, returned as its list of top-level arguments. */
+function callSiteArgs(source: string, fn: string): string[][] {
+  const calls: string[][] = [];
+  const needle = `${fn}(`;
+  let from = 0;
+  for (;;) {
+    const start = source.indexOf(needle, from);
+    if (start === -1) break;
+    from = start + needle.length;
+    // Skip the import/declaration forms — only real invocations carry arguments.
+    const before = source.slice(Math.max(0, start - 9), start);
+    if (/\bfunction\s+$/.test(before)) continue;
+    let depth = 1;
+    let i = from;
+    while (i < source.length && depth > 0) {
+      const ch = source[i];
+      if (ch === "(") depth++;
+      else if (ch === ")") depth--;
+      i++;
+    }
+    calls.push(splitTopLevelArgs(source.slice(from, i - 1)));
+  }
+  return calls;
+}
+
+test("chat.ts forwards the upstream body as the 5th argument to shouldMarkAccountExhaustedFrom429", () => {
+  const source = fs.readFileSync(path.join(repoRoot, "src/sse/handlers/chat.ts"), "utf8");
+  const calls = callSiteArgs(source, "shouldMarkAccountExhaustedFrom429").filter(
+    // Drop the `import { … }` specifier, which parses as a zero-argument "call".
+    (args) => args.length > 0
+  );
+
+  assert.equal(
+    calls.length,
+    1,
+    "expected exactly one shouldMarkAccountExhaustedFrom429 call site in chat.ts; " +
+      "a new one must forward errorText too"
+  );
+  assert.deepEqual(calls[0], ["provider", "model", "passthroughModels", "failureKind", "errorStr"]);
+
+  // Pin what `errorStr` is, so the guard cannot pass on a same-named local that no longer
+  // holds the upstream body (chat.ts:2282).
+  assert.match(source, /const errorStr = String\(result\.rawMessage \?\? result\.error \?\? ""\);/);
+});
+
+test.after(() => {
+  fs.rmSync(TEST_DATA_DIR, { recursive: true, force: true });
+});