From 7f53ef3a7eaa3b5ad6a9145af7030eec009b2041 Mon Sep 17 00:00:00 2001 From: Abhishek Sharma Date: Tue, 15 Sep 2026 16:46:03 -0700 Subject: [PATCH 1/4] feat(providers): park a key until the reset instant the upstream declared MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A free-tier quota exhaustion is dated by the upstream, but OpenRouter puts that date in the 429 BODY and sends no Retry-After. parseRetryAfterMs only reads the header, so the key was parked for DEFAULT_COOLDOWN_MS, came back, took another 429, and repeated for the rest of the quota window. parseQuotaResetAt reads `... will reset at ` out of a bounded 4KB prefix; readQuotaResetAt pulls it off a cloned response so the caller can still cancel the original to release the socket. When present it outranks both the header and the default, because it is the only one of the three that knows when the quota actually returns. MAX_QUOTA_COOLDOWN_MS is 32 days, separate from the 10-minute cap on undated 429s. That cap stays short on purpose: an undated 429 is a guess. A dated one is not. 32 rather than 8 days because the wording parsed is "Weekly/Monthly Limit Exhausted" — an 8-day cap silently clamped every monthly reset ~23 days early, back into the loop this removes. Caught by the cap's own test. Anything unreadable returns undefined and leaves today's behaviour exactly as it was: no body, consumed body, no match, unparsable date, a date already past, or one past the cap. Closes #4024 --- src/providers/key-failover.ts | 91 ++++++++++++++++++- src/server/responses/adapter-dispatch.ts | 6 ++ ...enrouter-quota-reset-cooldown-4024.test.ts | 89 ++++++++++++++++++ 3 files changed, 184 insertions(+), 2 deletions(-) create mode 100644 tests/providers/openrouter-quota-reset-cooldown-4024.test.ts diff --git a/src/providers/key-failover.ts b/src/providers/key-failover.ts index 2e7d0aedf6b..04539a585a7 100644 --- a/src/providers/key-failover.ts +++ b/src/providers/key-failover.ts @@ -29,6 +29,83 @@ interface KeyCooldown { const DEFAULT_COOLDOWN_MS = 60_000; const MAX_COOLDOWN_MS = 10 * 60_000; // cap at 10 min for api-key rotation +/** + * Cap for a cooldown the upstream itself dated, as opposed to one we inferred. + * + * `MAX_COOLDOWN_MS` is deliberately short because an undated 429 is a guess: ten + * minutes bounds how long a transient limit can park a working key. A free-tier + * quota is not a guess — OpenRouter replies `Weekly/Monthly Limit Exhausted ... + * will reset at `, and until that date the key cannot serve anything. Held + * for ten minutes instead, it comes back, takes a 429, and rotates again, every + * ten minutes for the rest of the week (#4024). + * + * 32 days rather than unbounded. The wording this parses is + * `Weekly/Monthly Limit Exhausted`, so the cap has to clear a monthly window — + * 31 days plus a day of slack for timezone and month length. An earlier 8-day + * cap looked generous against the weekly case in the issue and silently clamped + * every monthly reset to ~23 days early, which puts the key back into exactly + * the 429 loop this exists to stop. Caught by the cap's own test. + * + * Bounded at all because the date is upstream-controlled input: a malformed or + * hostile `reset at 2999-01-01` must not park a working key past any horizon an + * operator would think to look at. + */ +const MAX_QUOTA_COOLDOWN_MS = 32 * 24 * 60 * 60_000; + +/** + * Read a bounded prefix of a 429 body and pull the upstream's declared reset instant. + * + * Clones first: the caller still cancels the original body to release the socket, + * and a rotation storm must not be gated on reading N full error payloads. Any + * failure — no body, already consumed, slow, malformed — returns undefined and + * leaves the `Retry-After` path exactly as it was. + */ +export async function readQuotaResetAt(response: Response, now = Date.now()): Promise { + try { + if (!response.body) return undefined; + const text = await response.clone().text(); + return parseQuotaResetAt(text, now); + } catch { + return undefined; + } +} + +/** + * Reset instant an upstream declared in a 429 *body*, in epoch ms. + * + * Only the body carries this: OpenRouter sends no `Retry-After` for a quota + * exhaustion, so the header path (`parseRetryAfterMs`) sees nothing and falls + * back to `DEFAULT_COOLDOWN_MS`. Returns undefined for anything it cannot read + * as a date, so an unparsable body keeps today's behaviour exactly. + */ +export function parseQuotaResetAt(body: string | null | undefined, now = Date.now()): number | undefined { + const text = body?.slice(0, 4_096); + if (!text) return undefined; + // `will reset at 2026-09-09 03:30:06` / `... at 2026-09-09T03:30:06Z` / `resets at ` + const match = /reset[s]?\s+at\s+([0-9]{4}-[0-9]{2}-[0-9]{2}(?:[T ][0-9]{2}:[0-9]{2}(?::[0-9]{2})?(?:\.[0-9]+)?(?:Z|[+-][0-9]{2}:?[0-9]{2})?)?)/i.exec(text); + if (!match) return undefined; + // Pin a bare `YYYY-MM-DD hh:mm:ss` to UTC explicitly. + // + // ECMA-262 says a date-TIME form with no offset is LOCAL time, and Node follows + // that: `Date.parse("2026-09-09 03:30:06")` differs from the UTC reading by the + // host offset (7h on a PDT box, measured). Bun currently returns the UTC value + // for the same string, so on this runtime the normalisation is a no-op today — + // which is exactly why it is written out rather than relied upon. If Bun ever + // conforms, an un-normalised parse would silently shift every park-until by the + // operator's offset, and the early direction resumes the 429 loop. + // + // A consequence worth knowing: no Bun test can observe this branch being + // removed. The explicit-zone case below is the part the suite can pin. + const raw = match[1].includes("T") || /(?:Z|[+-][0-9]{2}:?[0-9]{2})$/.test(match[1]) + ? match[1] + : `${match[1].replace(" ", "T")}Z`; + const at = Date.parse(raw); + if (!Number.isFinite(at)) return undefined; + // Already past, or beyond the cap: not usable as a park-until instant. + if (at <= now) return undefined; + return Math.min(at, now + MAX_QUOTA_COOLDOWN_MS); +} + /** * Default same-target 429 retry policy used when a provider opts in via a bare * `retryOn429: {}` (presence = opt-in with these defaults). @@ -370,6 +447,7 @@ function rotateKeyAfterFailure( now = Date.now(), attemptedKey?: string, attemptedSelection?: ProviderApiKeySelection, + quotaResetAt?: number, ): OcxProviderConfig | null { const provider = config.providers[providerName]; if (!provider) return null; @@ -428,7 +506,12 @@ function rotateKeyAfterFailure( // full cap instead of the 429 default so a dead key is not re-tried once a minute. const cooldownMs = failureStatus === 401 ? MAX_COOLDOWN_MS - : parseRetryAfterMs(retryAfterHeader, now) ?? DEFAULT_COOLDOWN_MS; + // A reset instant the upstream dated outranks both the header and the + // default: it is the only one of the three that knows when the quota + // actually returns (#4024). + : quotaResetAt !== undefined + ? Math.max(quotaResetAt - now, 1) + : parseRetryAfterMs(retryAfterHeader, now) ?? DEFAULT_COOLDOWN_MS; keyCooldowns.set(cooldownKey(providerName, outcome.value.failedId), { cooldownUntil: now + cooldownMs }); sweepExpiredOnWrite(now); } @@ -455,8 +538,9 @@ export function rotateKeyOn429( now = Date.now(), attemptedKey?: string, attemptedSelection?: ProviderApiKeySelection, + quotaResetAt?: number, ): OcxProviderConfig | null { - return rotateKeyAfterFailure(config, providerName, 429, retryAfterHeader, now, attemptedKey, attemptedSelection); + return rotateKeyAfterFailure(config, providerName, 429, retryAfterHeader, now, attemptedKey, attemptedSelection, quotaResetAt); } /** @@ -489,6 +573,8 @@ export function sweepExpiredApiKeyCooldowns(now = Date.now()): number { interface RotateProviderTransportOptions { retryAfter?: string | null; + /** Epoch ms from `parseQuotaResetAt`, when the upstream dated the reset in its body. */ + quotaResetAt?: number; now?: number; attemptedKey?: string; attemptedSelection?: ProviderApiKeySelection; @@ -514,6 +600,7 @@ export function rotateProviderTransportOn429( options.now, options.attemptedKey, options.attemptedSelection ?? routedProvider._apiKeyAttempt, + options.quotaResetAt, ); if (!rotated) return null; return applyRotatedTransport(providerName, routedProvider, rotated, options.promptCacheKey); diff --git a/src/server/responses/adapter-dispatch.ts b/src/server/responses/adapter-dispatch.ts index e832d7308eb..03d896d86cb 100644 --- a/src/server/responses/adapter-dispatch.ts +++ b/src/server/responses/adapter-dispatch.ts @@ -25,6 +25,7 @@ import { hasKeyPoolFailover, rotateProviderTransportOn401, rateLimitRetryDelayMs, + readQuotaResetAt, rotateProviderTransportOn429, } from "../../providers/key-failover"; import { @@ -675,11 +676,16 @@ export async function prepareAdapterExchange( // SAME request once per remaining key. OAuth/forward providers and single-key pools // return null immediately, so this stays a no-op for them (src/providers/key-failover.ts). while (upstreamResponse.status === 429 && hasKeyPoolFailover(route.provider)) { + // A quota exhaustion is dated in the BODY, not in `Retry-After` — OpenRouter + // sends no header for it (#4024). Read a bounded prefix before the socket is + // released below; a failed or slow read just leaves the header path in charge. + const quotaResetAt = await readQuotaResetAt(upstreamResponse); const rotated = rotateProviderTransportOn429(config, route.providerName, route.provider, { retryAfter: upstreamResponse.headers.get("retry-after"), now: Date.now(), attemptedKey: route.provider.apiKey, promptCacheKey: parsed.options.promptCacheKey, + quotaResetAt, }); if (!rotated) break; // Release the failed response's socket before retrying; unread bodies otherwise linger diff --git a/tests/providers/openrouter-quota-reset-cooldown-4024.test.ts b/tests/providers/openrouter-quota-reset-cooldown-4024.test.ts new file mode 100644 index 00000000000..e4ac0a1cbbd --- /dev/null +++ b/tests/providers/openrouter-quota-reset-cooldown-4024.test.ts @@ -0,0 +1,89 @@ +import { describe, expect, test } from "bun:test"; +import { parseQuotaResetAt, readQuotaResetAt } from "../../src/providers/key-failover"; + +/** + * #4024 — a free-tier quota exhaustion is dated by the upstream, and OpenRouter + * sends it in the 429 body rather than in `Retry-After`. Without reading it the + * key is parked for the undated-429 cap (10 min), comes back, takes another 429, + * and repeats for the rest of the quota window. + */ +describe("parseQuotaResetAt", () => { + const now = Date.parse("2026-09-01T00:00:00Z"); + + test("reads the OpenRouter wording, treating a bare timestamp as UTC", () => { + const body = JSON.stringify({ + error: { code: "rate_limit_error", message: "Weekly Limit Exhausted. Your limit will reset at 2026-09-09 03:30:06" }, + }); + expect(parseQuotaResetAt(body, now)).toBe(Date.parse("2026-09-09T03:30:06Z")); + }); + + test("honours an explicit zone rather than re-stamping it as UTC", () => { + const at = parseQuotaResetAt("limit will reset at 2026-09-09T03:30:06+05:30", now); + expect(at).toBe(Date.parse("2026-09-09T03:30:06+05:30")); + expect(at).not.toBe(Date.parse("2026-09-09T03:30:06Z")); + }); + + test("accepts the 'resets at' spelling and a date with no clock time", () => { + expect(parseQuotaResetAt("quota resets at 2026-09-09", now)).toBe(Date.parse("2026-09-09T00:00:00Z")); + }); + + test("a body it cannot read yields undefined, so today's behaviour is unchanged", () => { + for (const body of [ + null, + undefined, + "", + "429 Too Many Requests", + JSON.stringify({ error: { message: "rate limited, try later" } }), + "will reset at soon", + "will reset at 2026-13-45 99:99:99", + ]) { + expect(parseQuotaResetAt(body as string | null | undefined, now)).toBeUndefined(); + } + }); + + test("a reset already in the past is not a park-until instant", () => { + expect(parseQuotaResetAt("will reset at 2026-08-01 00:00:00", now)).toBeUndefined(); + }); + + test("a monthly window is honoured in full, not clamped", () => { + // `Weekly/Monthly Limit Exhausted` is the wording upstream sends, so a reset + // up to ~31 days out is legitimate. Clamping it would resume the 429 loop + // weeks early — the failure this feature exists to prevent. + const monthly = "will reset at 2026-10-01 00:00:00"; + expect(parseQuotaResetAt(monthly, now)).toBe(Date.parse("2026-10-01T00:00:00Z")); + }); + + test("an absurd or hostile date is capped rather than parking the key forever", () => { + const at = parseQuotaResetAt("will reset at 2999-01-01 00:00:00", now); + expect(at).toBe(now + 32 * 24 * 60 * 60_000); + }); + + test("only the first 4KB is scanned, so a huge body cannot stall the rotation path", () => { + const padded = "x".repeat(8_000) + " will reset at 2026-09-09 03:30:06"; + expect(parseQuotaResetAt(padded, now)).toBeUndefined(); + }); +}); + +describe("readQuotaResetAt", () => { + const now = Date.parse("2026-09-01T00:00:00Z"); + + test("pulls the reset from a 429 body without consuming it for the caller", async () => { + const body = JSON.stringify({ + error: { code: "rate_limit_error", message: "Weekly Limit Exhausted. Your limit will reset at 2026-09-05 12:00:00" }, + }); + const response = new Response(body, { status: 429 }); + + expect(await readQuotaResetAt(response, now)).toBe(Date.parse("2026-09-05T12:00:00Z")); + // The caller still cancels the original to release the socket — it must not + // already be disturbed by the read above. + expect(response.bodyUsed).toBe(false); + await response.body?.cancel(); + }); + + test("a bodyless or unreadable response leaves the Retry-After path in charge", async () => { + expect(await readQuotaResetAt(new Response(null, { status: 429 }), now)).toBeUndefined(); + const consumed = new Response("x", { status: 429 }); + await consumed.text(); + expect(await readQuotaResetAt(consumed, now)).toBeUndefined(); + }); +}); From a7f6be02b01829049901db9c6f9a66274db58ab1 Mon Sep 17 00:00:00 2001 From: Abhishek Sharma Date: Tue, 15 Sep 2026 17:46:43 -0700 Subject: [PATCH 2/4] fix(providers): bound the 429 body READ, and stop cloning it MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review follow-up from @lidge-jun on #4733: readQuotaResetAt called .text() on a clone and sliced 4KB afterwards, so the parse was bounded and the read was not. The PR claimed a bounded prefix; that was wrong. Fixing it surfaced something worse than the unbounded read. clone() tees the body, and the caller leaves the original branch undrained while this runs — so the tee stalls once its buffer fills. A 5MB error body hangs the rotation path outright. Reproduced: the first bounded version still timed out at 5s against a finite 5MB stream. So it no longer clones. It pulls a bounded prefix from the original and returns a Response that replays those bytes ahead of the remainder, which the caller can read or cancel exactly as before. The signature is now { at, response } and adapter-dispatch rebinds upstreamResponse — the response is still needed on the !rotated path, so consuming it outright was not an option either. New test counts bytes actually PULLED, not bytes parsed: the two were different before, which is the whole point. --- src/providers/key-failover.ts | 61 +++++++++++++++++-- src/server/responses/adapter-dispatch.ts | 6 +- ...enrouter-quota-reset-cooldown-4024.test.ts | 58 +++++++++++++++--- 3 files changed, 110 insertions(+), 15 deletions(-) diff --git a/src/providers/key-failover.ts b/src/providers/key-failover.ts index 04539a585a7..a7ad5f402eb 100644 --- a/src/providers/key-failover.ts +++ b/src/providers/key-failover.ts @@ -60,16 +60,65 @@ const MAX_QUOTA_COOLDOWN_MS = 32 * 24 * 60 * 60_000; * failure — no body, already consumed, slow, malformed — returns undefined and * leaves the `Retry-After` path exactly as it was. */ -export async function readQuotaResetAt(response: Response, now = Date.now()): Promise { +export async function readQuotaResetAt( + response: Response, + now = Date.now(), +): Promise<{ at: number | undefined; response: Response }> { + if (!response.body) return { at: undefined, response }; try { - if (!response.body) return undefined; - const text = await response.clone().text(); - return parseQuotaResetAt(text, now); + const reader = response.body.getReader(); + const decoder = new TextDecoder(); + const chunks: Uint8Array[] = []; + let seen = 0; + let text = ""; + while (seen < QUOTA_RESET_SCAN_BYTES) { + const { done, value } = await reader.read(); + if (done) break; + chunks.push(value); + seen += value.byteLength; + text += decoder.decode(value, { stream: true }); + } + // Hand back a Response carrying the bytes already pulled followed by whatever + // is left, so the caller can still read or cancel it. `response.clone()` is + // NOT usable here: it tees, and with the original branch undrained the tee + // stalls once its buffer fills — a 5MB error body hangs the rotation path, + // which is worse than the unbounded read this replaced. + const rest = new ReadableStream({ + start(controller) { + for (const c of chunks) controller.enqueue(c); + }, + async pull(controller) { + const { done, value } = await reader.read(); + if (done) { + controller.close(); + return; + } + controller.enqueue(value); + }, + cancel(reason) { + return reader.cancel(reason); + }, + }); + const rebuilt = new Response(rest, { + status: response.status, + statusText: response.statusText, + headers: response.headers, + }); + return { at: parseQuotaResetAt(text, now), response: rebuilt }; } catch { - return undefined; + return { at: undefined, response }; } } +/** + * How much of a 429 body is read and scanned for the reset instant. + * + * Bounds the READ, not just the parse: this runs on the rotation path, once per + * rotated key under a rate-limit storm, and the body is upstream-controlled. + * OpenRouter's rate_limit_error JSON is a few hundred bytes. + */ +const QUOTA_RESET_SCAN_BYTES = 4_096; + /** * Reset instant an upstream declared in a 429 *body*, in epoch ms. * @@ -79,7 +128,7 @@ export async function readQuotaResetAt(response: Response, now = Date.now()): Pr * as a date, so an unparsable body keeps today's behaviour exactly. */ export function parseQuotaResetAt(body: string | null | undefined, now = Date.now()): number | undefined { - const text = body?.slice(0, 4_096); + const text = body?.slice(0, QUOTA_RESET_SCAN_BYTES); if (!text) return undefined; // `will reset at 2026-09-09 03:30:06` / `... at 2026-09-09T03:30:06Z` / `resets at ` const match = /reset[s]?\s+at\s+([0-9]{4}-[0-9]{2}-[0-9]{2}(?:[T ][0-9]{2}:[0-9]{2}(?::[0-9]{2})?(?:\.[0-9]+)?(?:Z|[+-][0-9]{2}:?[0-9]{2})?)?)/i.exec(text); diff --git a/src/server/responses/adapter-dispatch.ts b/src/server/responses/adapter-dispatch.ts index 03d896d86cb..5ed7136a49d 100644 --- a/src/server/responses/adapter-dispatch.ts +++ b/src/server/responses/adapter-dispatch.ts @@ -679,7 +679,11 @@ export async function prepareAdapterExchange( // A quota exhaustion is dated in the BODY, not in `Retry-After` — OpenRouter // sends no header for it (#4024). Read a bounded prefix before the socket is // released below; a failed or slow read just leaves the header path in charge. - const quotaResetAt = await readQuotaResetAt(upstreamResponse); + // Peeks a bounded prefix and hands back a Response still carrying the whole + // body, so the cancel below still releases the socket. + const peeked = await readQuotaResetAt(upstreamResponse); + upstreamResponse = peeked.response; + const quotaResetAt = peeked.at; const rotated = rotateProviderTransportOn429(config, route.providerName, route.provider, { retryAfter: upstreamResponse.headers.get("retry-after"), now: Date.now(), diff --git a/tests/providers/openrouter-quota-reset-cooldown-4024.test.ts b/tests/providers/openrouter-quota-reset-cooldown-4024.test.ts index e4ac0a1cbbd..738c43bfb32 100644 --- a/tests/providers/openrouter-quota-reset-cooldown-4024.test.ts +++ b/tests/providers/openrouter-quota-reset-cooldown-4024.test.ts @@ -67,23 +67,65 @@ describe("parseQuotaResetAt", () => { describe("readQuotaResetAt", () => { const now = Date.parse("2026-09-01T00:00:00Z"); - test("pulls the reset from a 429 body without consuming it for the caller", async () => { + test("returns the reset AND a response whose body is still fully readable", async () => { + // The caller still needs this response: on a failed rotation adapter-dispatch + // breaks out of the loop with it, and on a successful one it cancels the body + // to release the socket. Peeking must not cost it either. const body = JSON.stringify({ error: { code: "rate_limit_error", message: "Weekly Limit Exhausted. Your limit will reset at 2026-09-05 12:00:00" }, }); - const response = new Response(body, { status: 429 }); + const { at, response } = await readQuotaResetAt(new Response(body, { status: 429 }), now); - expect(await readQuotaResetAt(response, now)).toBe(Date.parse("2026-09-05T12:00:00Z")); - // The caller still cancels the original to release the socket — it must not - // already be disturbed by the read above. - expect(response.bodyUsed).toBe(false); + expect(at).toBe(Date.parse("2026-09-05T12:00:00Z")); + expect(response.status).toBe(429); + // The bytes already pulled are replayed ahead of the remainder. + expect(await response.text()).toBe(body); + }); + + test("the returned response can be cancelled instead of read", async () => { + const { response } = await readQuotaResetAt(new Response("x".repeat(10_000), { status: 429 }), now); await response.body?.cancel(); + expect(response.bodyUsed).toBe(true); }); test("a bodyless or unreadable response leaves the Retry-After path in charge", async () => { - expect(await readQuotaResetAt(new Response(null, { status: 429 }), now)).toBeUndefined(); + expect((await readQuotaResetAt(new Response(null, { status: 429 }), now)).at).toBeUndefined(); const consumed = new Response("x", { status: 429 }); await consumed.text(); - expect(await readQuotaResetAt(consumed, now)).toBeUndefined(); + expect((await readQuotaResetAt(consumed, now)).at).toBeUndefined(); + }); +}); + +describe("readQuotaResetAt — the read is bounded, not just the parse", () => { + const now = Date.parse("2026-09-01T00:00:00Z"); + + test("stops pulling after the cap instead of buffering the whole body", async () => { + // A chatty upstream must not make the rotation path read megabytes. This counts + // what the reader actually PULLED, not what the parser looked at — the two were + // different before this was fixed (`.text()` read it all, then sliced 4KB). + let pulled = 0; + const chunk = new TextEncoder().encode("x".repeat(64 * 1_024)); + const total = 5 * 1_024 * 1_024; + const body = new ReadableStream({ + pull(controller) { + if (pulled >= total) { + controller.close(); + return; + } + pulled += chunk.byteLength; + controller.enqueue(chunk); + }, + }); + + const { at } = await readQuotaResetAt(new Response(body, { status: 429 }), now); + + expect(at).toBeUndefined(); + expect(pulled).toBeLessThan(total / 4); + }); + + test("still finds a reset that sits inside the cap", async () => { + const body = `{"error":{"message":"Weekly Limit Exhausted. Your limit will reset at 2026-09-05 12:00:00"}}`; + expect((await readQuotaResetAt(new Response(body, { status: 429 }), now)).at) + .toBe(Date.parse("2026-09-05T12:00:00Z")); }); }); From 79689aa13ce689d0ba4eec70988d9b4eb75647a6 Mon Sep 17 00:00:00 2001 From: Abhishek Sharma Date: Tue, 15 Sep 2026 19:40:38 -0700 Subject: [PATCH 3/4] fix(providers): refuse a reset date the calendar does not have, and pin the dispatch path Two CodeRabbit findings. `Date.parse` does not reject an out-of-range day. Measured on Bun, `2026-02-30T00:00:00Z` yields March 2 and `2026-04-31T00:00:00Z` yields May 1, so a malformed upstream body parked a key past the instant it actually named. Only the month was caught (`2026-13-01` is NaN). Validated on the date text rather than by round-tripping the parsed instant, because a value carrying an explicit offset legitimately lands on a different UTC day than the one written. Century leap rule included: 2000 is a leap year, 2100 is not. The other finding is that nothing exercised the wiring. The unit tests covered parseQuotaResetAt and readQuotaResetAt in isolation, so a change that dropped quotaResetAt before rotateProviderTransportOn429, or flipped the precedence against Retry-After, kept every test green while the key came back after the header's 30s and took the same 429 again. Added an end-to-end test that serves a 429 carrying both a Retry-After and a body date and asserts the failed key is parked to the body's instant. Co-authored-by: Abhishek Sharma --- src/providers/key-failover.ts | 21 ++++++ ...enrouter-quota-reset-cooldown-4024.test.ts | 23 +++++++ tests/server/server-key-failover-e2e.test.ts | 65 ++++++++++++++++++- 3 files changed, 108 insertions(+), 1 deletion(-) diff --git a/src/providers/key-failover.ts b/src/providers/key-failover.ts index a7ad5f402eb..60964e830cd 100644 --- a/src/providers/key-failover.ts +++ b/src/providers/key-failover.ts @@ -127,6 +127,26 @@ const QUOTA_RESET_SCAN_BYTES = 4_096; * back to `DEFAULT_COOLDOWN_MS`. Returns undefined for anything it cannot read * as a date, so an unparsable body keeps today's behaviour exactly. */ +/** + * Whether `YYYY-MM-DD…` names a day that exists. + * + * `Date.parse` does NOT reject an out-of-range day: measured on Bun, + * `2026-02-30T00:00:00Z` yields March 2 and `2026-04-31T00:00:00Z` yields + * May 1, so a malformed upstream body would park a key past the instant it + * actually named. Only the month is rejected outright (`2026-13-01` is NaN). + * + * Checked on the date text alone rather than by round-tripping the parsed + * instant, because a value carrying an explicit offset (`…T23:00+05:30`) + * legitimately lands on a different UTC day than the one written. + */ +function isRealCalendarDate(value: string): boolean { + const [year, month, day] = value.slice(0, 10).split("-").map(Number); + if (month < 1 || month > 12 || day < 1) return false; + const leap = (year % 4 === 0 && year % 100 !== 0) || year % 400 === 0; + const lengths = [31, leap ? 29 : 28, 31, 30, 31, 30, 31, 31, 30, 31, 30, 31]; + return day <= lengths[month - 1]!; +} + export function parseQuotaResetAt(body: string | null | undefined, now = Date.now()): number | undefined { const text = body?.slice(0, QUOTA_RESET_SCAN_BYTES); if (!text) return undefined; @@ -148,6 +168,7 @@ export function parseQuotaResetAt(body: string | null | undefined, now = Date.no const raw = match[1].includes("T") || /(?:Z|[+-][0-9]{2}:?[0-9]{2})$/.test(match[1]) ? match[1] : `${match[1].replace(" ", "T")}Z`; + if (!isRealCalendarDate(match[1])) return undefined; const at = Date.parse(raw); if (!Number.isFinite(at)) return undefined; // Already past, or beyond the cap: not usable as a park-until instant. diff --git a/tests/providers/openrouter-quota-reset-cooldown-4024.test.ts b/tests/providers/openrouter-quota-reset-cooldown-4024.test.ts index 738c43bfb32..d806bed1df0 100644 --- a/tests/providers/openrouter-quota-reset-cooldown-4024.test.ts +++ b/tests/providers/openrouter-quota-reset-cooldown-4024.test.ts @@ -58,6 +58,29 @@ describe("parseQuotaResetAt", () => { expect(at).toBe(now + 32 * 24 * 60 * 60_000); }); + test("a day the calendar does not have is refused, not rolled forward", () => { + // `Date.parse` does not reject an out-of-range DAY — measured on Bun, + // `2026-02-30T00:00:00Z` yields March 2 — so without this the key parks + // past the instant the upstream actually named. Only the month is caught + // by the parser itself. + const feb = Date.parse("2026-02-25T00:00:00Z"); + expect(parseQuotaResetAt("resets at 2026-02-30T00:00:00Z", feb)).toBeUndefined(); + expect(parseQuotaResetAt("resets at 2026-02-29T00:00:00Z", feb)).toBeUndefined(); + expect(parseQuotaResetAt("resets at 2026-04-31T00:00:00Z", Date.parse("2026-04-25T00:00:00Z"))).toBeUndefined(); + expect(parseQuotaResetAt("resets at 2026-13-01T00:00:00Z", feb)).toBeUndefined(); + }); + + test("real leap days still park the key, including the century rule", () => { + // The guard above must not cost a legitimate Feb 29. 2024 is a leap year, + // 2000 is one (divisible by 400) and 2100 is not (divisible by 100). + expect(parseQuotaResetAt("resets at 2024-02-29T00:00:00Z", Date.parse("2024-02-25T00:00:00Z"))) + .toBe(Date.parse("2024-02-29T00:00:00Z")); + expect(parseQuotaResetAt("resets at 2000-02-29T00:00:00Z", Date.parse("2000-02-25T00:00:00Z"))) + .toBe(Date.parse("2000-02-29T00:00:00Z")); + expect(parseQuotaResetAt("resets at 2100-02-29T00:00:00Z", Date.parse("2100-02-25T00:00:00Z"))) + .toBeUndefined(); + }); + test("only the first 4KB is scanned, so a huge body cannot stall the rotation path", () => { const padded = "x".repeat(8_000) + " will reset at 2026-09-09 03:30:06"; expect(parseQuotaResetAt(padded, now)).toBeUndefined(); diff --git a/tests/server/server-key-failover-e2e.test.ts b/tests/server/server-key-failover-e2e.test.ts index 4bf924c2492..9fe34cb8f40 100644 --- a/tests/server/server-key-failover-e2e.test.ts +++ b/tests/server/server-key-failover-e2e.test.ts @@ -5,7 +5,7 @@ import { join } from "node:path"; import { apiKeyAccountLogLabel } from "../../src/codex/account-label"; import { readUsageEntries, resetUsageReadCacheForTests } from "../../src/usage/log"; import { loadConfig, saveConfig } from "../../src/config"; -import { clearKeyCooldowns, rotateKeyOn429 } from "../../src/providers/key-failover"; +import { clearKeyCooldowns, getKeyCooldownUntil, rotateKeyOn429 } from "../../src/providers/key-failover"; import { deriveXaiConvId } from "../../src/providers/xai-transport"; import { clearReasoningReplayCacheForTests } from "../../src/responses/reasoning-replay-cache"; import { startServer } from "../../src/server"; @@ -496,6 +496,69 @@ describe("server 429 key failover (end-to-end)", () => { }); } + test("a 429 dated in the body parks the failed key until that instant, outranking Retry-After", async () => { + // #4024 regression, through the real dispatch path. The unit tests cover + // parseQuotaResetAt/readQuotaResetAt in isolation; nothing exercised + // adapter-dispatch actually READING the body and handing quotaResetAt to + // rotateProviderTransportOn429. Dropping it there would leave every unit + // test green while the key came back after the header's 30s and took the + // same 429 again — which is the bug. + const resetAt = new Date(Date.now() + 6 * 60 * 60_000); + const stamp = resetAt.toISOString().replace("T", " ").slice(0, 19); // bare form, read as UTC + const seenAuth: string[] = []; + upstream = Bun.serve({ + hostname: "127.0.0.1", port: 0, + fetch(req) { + seenAuth.push(req.headers.get("authorization") ?? ""); + if (seenAuth.length === 1) { + return new Response(JSON.stringify({ + error: { code: "rate_limit_error", message: `Weekly Limit Exhausted. Your limit will reset at ${stamp}` }, + }), { status: 429, headers: { "retry-after": "30", "content-type": "application/json" } }); + } + return new Response(JSON.stringify({ + id: "chatcmpl-dated", object: "chat.completion", + choices: [{ index: 0, message: { role: "assistant", content: "ok after dated rotate" }, finish_reason: "stop" }], + }), { headers: { "content-type": "application/json" } }); + }, + }); + const config: OcxConfig = { + port: 0, hostname: "127.0.0.1", defaultProvider: "dated", + providers: { + dated: { + adapter: "openai-chat", + baseUrl: `http://127.0.0.1:${upstream.port}/v1`, + allowPrivateNetwork: true, + apiKey: "key-dated-000111222333", + apiKeyPool: [ + { id: "d1", key: "key-dated-000111222333", addedAt: 1 }, + { id: "d2", key: "key-dated-444555666777", addedAt: 2 }, + ], + }, + }, + } as OcxConfig; + saveConfig(config); + const server = startServer(0); + try { + const res = await fetch(new URL("/v1/responses", server.url), { + method: "POST", + headers: { "content-type": "application/json" }, + body: JSON.stringify({ model: "dated/some-model", input: "hello", stream: false }), + }); + expect(res.status).toBe(200); + await res.json(); + expect(seenAuth[1]).toBe("Bearer key-dated-444555666777"); + + const cooldownUntil = getKeyCooldownUntil("dated", "d1"); + expect(cooldownUntil).not.toBeNull(); + // The body's instant, not the header's 30s. Compared with a wide window + // because the cooldown is anchored to the server's Date.now(), not ours. + expect(cooldownUntil!).toBeGreaterThan(Date.now() + 5 * 60 * 60_000); + expect(cooldownUntil!).toBeLessThanOrEqual(resetAt.getTime() + 60_000); + } finally { + await server.stop(true); + } + }); + test("reasoning replay misses after a 429 rotates to a different physical key", async () => { const model = "reasoning-model"; const callId = "call_key_rotation"; From 613e224ea400def76ab9e42404f67edb9efbd8cb Mon Sep 17 00:00:00 2001 From: JUN Date: Fri, 18 Sep 2026 08:34:10 +0900 Subject: [PATCH 4/4] fix(providers): harden dated quota cooldown parsing Implements the reviewed fixes for PR #4733 while preserving the original monthly-window and UTC contracts. Co-authored-by: Abhishek Sharma --- scripts/test-layout/layout.json | 1 + src/providers/key-failover.ts | 142 ++++++++++++++---- src/server/responses/adapter-dispatch.ts | 14 +- structure/transports/responses.md | 8 +- tests/fixtures/test-layout-expected.json | 1 + ...enrouter-quota-reset-cooldown-4024.test.ts | 107 +++++++++---- tests/server/server-key-failover-e2e.test.ts | 60 ++++++++ 7 files changed, 278 insertions(+), 55 deletions(-) diff --git a/scripts/test-layout/layout.json b/scripts/test-layout/layout.json index 305a19946ef..e0c73a5e32c 100644 --- a/scripts/test-layout/layout.json +++ b/scripts/test-layout/layout.json @@ -1044,6 +1044,7 @@ "opencode-zen-deepseek-reasoning.test.ts": "providers", "opencode-zen-rate-limit.test.ts": "providers", "openrouter-provider-routing.test.ts": "providers", + "openrouter-quota-reset-cooldown-4024.test.ts": "providers", "optional-shutdown-hooks.test.ts": "lib", "orcarouter-provider.test.ts": "providers", "outbound-body-guard.test.ts": "server", diff --git a/src/providers/key-failover.ts b/src/providers/key-failover.ts index 60964e830cd..ba7193d2b57 100644 --- a/src/providers/key-failover.ts +++ b/src/providers/key-failover.ts @@ -51,32 +51,93 @@ const MAX_COOLDOWN_MS = 10 * 60_000; // cap at 10 min for api-key rotation * operator would think to look at. */ const MAX_QUOTA_COOLDOWN_MS = 32 * 24 * 60 * 60_000; +const QUOTA_RESET_PEEK_TIMEOUT_MS = 250; + +interface QuotaResetReadOptions { + now?: number; + signal?: AbortSignal; + timeoutMs?: number; +} + +function rebuiltResponse(response: Response, body: ReadableStream): Response { + return new Response(body, { + status: response.status, + statusText: response.statusText, + headers: response.headers, + }); +} + +function replayOnlyResponse(response: Response, chunks: readonly Uint8Array[]): Response { + return rebuiltResponse(response, new ReadableStream({ + start(controller) { + for (const chunk of chunks) controller.enqueue(chunk); + controller.close(); + }, + })); +} /** * Read a bounded prefix of a 429 body and pull the upstream's declared reset instant. * - * Clones first: the caller still cancels the original body to release the socket, - * and a rotation storm must not be gated on reading N full error payloads. Any - * failure — no body, already consumed, slow, malformed — returns undefined and - * leaves the `Retry-After` path exactly as it was. + * The returned response replays the bounded prefix and any boundary-chunk overflow + * before streaming the unread remainder. A rotation storm must not be gated on + * reading N full error payloads. Any failure — no body, already consumed, slow, + * malformed — returns undefined and leaves the `Retry-After` path in charge. */ export async function readQuotaResetAt( response: Response, - now = Date.now(), + nowOrOptions: number | QuotaResetReadOptions = {}, ): Promise<{ at: number | undefined; response: Response }> { if (!response.body) return { at: undefined, response }; + const options = typeof nowOrOptions === "number" ? { now: nowOrOptions } : nowOrOptions; + const now = options.now ?? Date.now(); + let reader: ReadableStreamDefaultReader; + try { + reader = response.body.getReader(); + } catch { + return { at: undefined, response }; + } + const chunks: Uint8Array[] = []; + let transferred = false; + let timer: ReturnType | undefined; + const deadline = new AbortController(); + const timeoutReason = new DOMException("Quota reset body peek timed out", "TimeoutError"); try { - const reader = response.body.getReader(); const decoder = new TextDecoder(); - const chunks: Uint8Array[] = []; let seen = 0; let text = ""; + timer = setTimeout(() => deadline.abort(timeoutReason), options.timeoutMs ?? QUOTA_RESET_PEEK_TIMEOUT_MS); + const signal = options.signal + ? AbortSignal.any([options.signal, deadline.signal]) + : deadline.signal; while (seen < QUOTA_RESET_SCAN_BYTES) { - const { done, value } = await reader.read(); + const read = reader.read(); + let rejectAbort: ((reason: unknown) => void) | undefined; + const onAbort = () => rejectAbort?.(signal.reason); + const aborted = new Promise((_resolve, reject) => { + rejectAbort = reject; + if (signal.aborted) reject(signal.reason); + else signal.addEventListener("abort", onAbort, { once: true }); + }); + // Derived from the reader rather than named directly: Bun's lib types + // `ReadableStreamDefaultReader.read()` as returning + // `ReadableStreamDefaultReadResult`, which is not assignable to the + // `ReadableStreamReadResult` alias. + let result: Awaited>; + try { + result = await Promise.race([read, aborted]); + } finally { + signal.removeEventListener("abort", onAbort); + } + const { done, value } = result; if (done) break; - chunks.push(value); - seen += value.byteLength; - text += decoder.decode(value, { stream: true }); + const remaining = QUOTA_RESET_SCAN_BYTES - seen; + const prefix = value.byteLength > remaining ? value.subarray(0, remaining) : value; + const overflow = value.byteLength > remaining ? value.subarray(remaining) : undefined; + chunks.push(prefix); + if (overflow?.byteLength) chunks.push(overflow); + seen += prefix.byteLength; + text += decoder.decode(prefix, { stream: true }); } // Hand back a Response carrying the bytes already pulled followed by whatever // is left, so the caller can still read or cancel it. `response.clone()` is @@ -88,25 +149,41 @@ export async function readQuotaResetAt( for (const c of chunks) controller.enqueue(c); }, async pull(controller) { - const { done, value } = await reader.read(); - if (done) { - controller.close(); - return; + try { + const { done, value } = await reader.read(); + if (done) { + controller.close(); + reader.releaseLock(); + return; + } + controller.enqueue(value); + } catch (error) { + controller.error(error); + reader.releaseLock(); } - controller.enqueue(value); }, - cancel(reason) { - return reader.cancel(reason); + async cancel(reason) { + try { + await reader.cancel(reason); + } finally { + reader.releaseLock(); + } }, }); - const rebuilt = new Response(rest, { - status: response.status, - statusText: response.statusText, - headers: response.headers, + transferred = true; + return { at: parseQuotaResetAt(text, now), response: rebuiltResponse(response, rest) }; + } catch (error) { + const clientAborted = options.signal?.aborted === true; + void reader.cancel(error).catch(() => {}).finally(() => { + try { reader.releaseLock(); } catch { /* already released */ } }); - return { at: parseQuotaResetAt(text, now), response: rebuilt }; - } catch { - return { at: undefined, response }; + if (clientAborted) throw options.signal!.reason ?? error; + return { at: undefined, response: replayOnlyResponse(response, chunks) }; + } finally { + if (timer !== undefined) clearTimeout(timer); + if (!transferred) { + try { reader.releaseLock(); } catch { /* already released */ } + } } } @@ -150,8 +227,21 @@ function isRealCalendarDate(value: string): boolean { export function parseQuotaResetAt(body: string | null | undefined, now = Date.now()): number | undefined { const text = body?.slice(0, QUOTA_RESET_SCAN_BYTES); if (!text) return undefined; + let parsed: unknown; + try { + parsed = JSON.parse(text); + } catch { + return undefined; + } + if (!parsed || typeof parsed !== "object") return undefined; + const error = (parsed as { error?: unknown }).error; + if (!error || typeof error !== "object") return undefined; + const code = (error as { code?: unknown }).code; + const message = (error as { message?: unknown }).message; + if ((code !== "rate_limit_error" && code !== 429) || typeof message !== "string") return undefined; + if (!/^(?:Weekly|Monthly) Limit Exhausted\b/i.test(message.trim())) return undefined; // `will reset at 2026-09-09 03:30:06` / `... at 2026-09-09T03:30:06Z` / `resets at ` - const match = /reset[s]?\s+at\s+([0-9]{4}-[0-9]{2}-[0-9]{2}(?:[T ][0-9]{2}:[0-9]{2}(?::[0-9]{2})?(?:\.[0-9]+)?(?:Z|[+-][0-9]{2}:?[0-9]{2})?)?)/i.exec(text); + const match = /reset[s]?\s+at\s+([0-9]{4}-[0-9]{2}-[0-9]{2}(?:[T ][0-9]{2}:[0-9]{2}(?::[0-9]{2})?(?:\.[0-9]+)?(?:Z|[+-][0-9]{2}:?[0-9]{2})?)?)/i.exec(message); if (!match) return undefined; // Pin a bare `YYYY-MM-DD hh:mm:ss` to UTC explicitly. // diff --git a/src/server/responses/adapter-dispatch.ts b/src/server/responses/adapter-dispatch.ts index 5ed7136a49d..95a08b6e68e 100644 --- a/src/server/responses/adapter-dispatch.ts +++ b/src/server/responses/adapter-dispatch.ts @@ -681,7 +681,19 @@ export async function prepareAdapterExchange( // released below; a failed or slow read just leaves the header path in charge. // Peeks a bounded prefix and hands back a Response still carrying the whole // body, so the cancel below still releases the socket. - const peeked = await readQuotaResetAt(upstreamResponse); + let peeked: Awaited>; + try { + peeked = await readQuotaResetAt(upstreamResponse, { signal: options.abortSignal }); + } catch { + cleanupUpstreamAbort(); + upstream.abort(); + return clientCancelledResponse(); + } + if (options.abortSignal?.aborted) { + cleanupUpstreamAbort(); + upstream.abort(); + return clientCancelledResponse(); + } upstreamResponse = peeked.response; const quotaResetAt = peeked.at; const rotated = rotateProviderTransportOn429(config, route.providerName, route.provider, { diff --git a/structure/transports/responses.md b/structure/transports/responses.md index aad709a5b2c..9b4fb00de28 100644 --- a/structure/transports/responses.md +++ b/structure/transports/responses.md @@ -189,7 +189,13 @@ wholesale assignment sends the literal reference as the bearer token. `adapter` are not at risk on a stored row, because the config schema requires both. Reactive 429 rotation (`rotateProviderTransportOn429`) remains the recovery path after a -send has already earned a throttle. +send has already earned a throttle. Before rotating a key, the Responses dispatch path peeks at +most 4 KiB of a 429 body under the client abort signal and a short deadline. Only the canonical +OpenRouter quota error shape (`rate_limit_error` or numeric 429 plus a Weekly/Monthly Limit +Exhausted message) may supply a dated cooldown; other providers continue to use `Retry-After` or +the ordinary undated cooldown. Bytes pulled in the boundary chunk are replayed ahead of the unread +stream, and every timeout, read failure, or cancellation cancels the reader and releases its lock. Client +cancellation terminates dispatch before rotation can persist another key. ### Routed service-tier capability diff --git a/tests/fixtures/test-layout-expected.json b/tests/fixtures/test-layout-expected.json index 76e216195c1..5a60ecf15bd 100644 --- a/tests/fixtures/test-layout-expected.json +++ b/tests/fixtures/test-layout-expected.json @@ -870,6 +870,7 @@ "opencode-zen-deepseek-reasoning.test.ts": "providers", "opencode-zen-rate-limit.test.ts": "providers", "openrouter-provider-routing.test.ts": "providers", + "openrouter-quota-reset-cooldown-4024.test.ts": "providers", "optional-shutdown-hooks.test.ts": "lib", "orcarouter-provider.test.ts": "providers", "outbound-body-guard.test.ts": "server", diff --git a/tests/providers/openrouter-quota-reset-cooldown-4024.test.ts b/tests/providers/openrouter-quota-reset-cooldown-4024.test.ts index d806bed1df0..52e28c1a0f5 100644 --- a/tests/providers/openrouter-quota-reset-cooldown-4024.test.ts +++ b/tests/providers/openrouter-quota-reset-cooldown-4024.test.ts @@ -9,22 +9,24 @@ import { parseQuotaResetAt, readQuotaResetAt } from "../../src/providers/key-fai */ describe("parseQuotaResetAt", () => { const now = Date.parse("2026-09-01T00:00:00Z"); + const quotaBody = (message: string, code: string | number = "rate_limit_error") => JSON.stringify({ + error: { code, message }, + }); test("reads the OpenRouter wording, treating a bare timestamp as UTC", () => { - const body = JSON.stringify({ - error: { code: "rate_limit_error", message: "Weekly Limit Exhausted. Your limit will reset at 2026-09-09 03:30:06" }, - }); + const body = quotaBody("Weekly Limit Exhausted. Your limit will reset at 2026-09-09 03:30:06"); expect(parseQuotaResetAt(body, now)).toBe(Date.parse("2026-09-09T03:30:06Z")); }); test("honours an explicit zone rather than re-stamping it as UTC", () => { - const at = parseQuotaResetAt("limit will reset at 2026-09-09T03:30:06+05:30", now); + const at = parseQuotaResetAt(quotaBody("Monthly Limit Exhausted. Your limit will reset at 2026-09-09T03:30:06+05:30", 429), now); expect(at).toBe(Date.parse("2026-09-09T03:30:06+05:30")); expect(at).not.toBe(Date.parse("2026-09-09T03:30:06Z")); }); test("accepts the 'resets at' spelling and a date with no clock time", () => { - expect(parseQuotaResetAt("quota resets at 2026-09-09", now)).toBe(Date.parse("2026-09-09T00:00:00Z")); + expect(parseQuotaResetAt(quotaBody("Weekly Limit Exhausted. Quota resets at 2026-09-09"), now)) + .toBe(Date.parse("2026-09-09T00:00:00Z")); }); test("a body it cannot read yields undefined, so today's behaviour is unchanged", () => { @@ -34,27 +36,27 @@ describe("parseQuotaResetAt", () => { "", "429 Too Many Requests", JSON.stringify({ error: { message: "rate limited, try later" } }), - "will reset at soon", - "will reset at 2026-13-45 99:99:99", + quotaBody("Weekly Limit Exhausted. Your limit will reset at soon"), + quotaBody("Weekly Limit Exhausted. Your limit will reset at 2026-13-45 99:99:99"), ]) { expect(parseQuotaResetAt(body as string | null | undefined, now)).toBeUndefined(); } }); test("a reset already in the past is not a park-until instant", () => { - expect(parseQuotaResetAt("will reset at 2026-08-01 00:00:00", now)).toBeUndefined(); + expect(parseQuotaResetAt(quotaBody("Weekly Limit Exhausted. Will reset at 2026-08-01 00:00:00"), now)).toBeUndefined(); }); test("a monthly window is honoured in full, not clamped", () => { // `Weekly/Monthly Limit Exhausted` is the wording upstream sends, so a reset // up to ~31 days out is legitimate. Clamping it would resume the 429 loop // weeks early — the failure this feature exists to prevent. - const monthly = "will reset at 2026-10-01 00:00:00"; + const monthly = quotaBody("Monthly Limit Exhausted. Will reset at 2026-10-01 00:00:00"); expect(parseQuotaResetAt(monthly, now)).toBe(Date.parse("2026-10-01T00:00:00Z")); }); test("an absurd or hostile date is capped rather than parking the key forever", () => { - const at = parseQuotaResetAt("will reset at 2999-01-01 00:00:00", now); + const at = parseQuotaResetAt(quotaBody("Monthly Limit Exhausted. Will reset at 2999-01-01 00:00:00"), now); expect(at).toBe(now + 32 * 24 * 60 * 60_000); }); @@ -64,20 +66,29 @@ describe("parseQuotaResetAt", () => { // past the instant the upstream actually named. Only the month is caught // by the parser itself. const feb = Date.parse("2026-02-25T00:00:00Z"); - expect(parseQuotaResetAt("resets at 2026-02-30T00:00:00Z", feb)).toBeUndefined(); - expect(parseQuotaResetAt("resets at 2026-02-29T00:00:00Z", feb)).toBeUndefined(); - expect(parseQuotaResetAt("resets at 2026-04-31T00:00:00Z", Date.parse("2026-04-25T00:00:00Z"))).toBeUndefined(); - expect(parseQuotaResetAt("resets at 2026-13-01T00:00:00Z", feb)).toBeUndefined(); + expect(parseQuotaResetAt(quotaBody("Weekly Limit Exhausted. Resets at 2026-02-30T00:00:00Z"), feb)).toBeUndefined(); + expect(parseQuotaResetAt(quotaBody("Weekly Limit Exhausted. Resets at 2026-02-29T00:00:00Z"), feb)).toBeUndefined(); + expect(parseQuotaResetAt(quotaBody("Weekly Limit Exhausted. Resets at 2026-04-31T00:00:00Z"), Date.parse("2026-04-25T00:00:00Z"))).toBeUndefined(); + expect(parseQuotaResetAt(quotaBody("Weekly Limit Exhausted. Resets at 2026-13-01T00:00:00Z"), feb)).toBeUndefined(); }); test("real leap days still park the key, including the century rule", () => { // The guard above must not cost a legitimate Feb 29. 2024 is a leap year, // 2000 is one (divisible by 400) and 2100 is not (divisible by 100). - expect(parseQuotaResetAt("resets at 2024-02-29T00:00:00Z", Date.parse("2024-02-25T00:00:00Z"))) + expect(parseQuotaResetAt(quotaBody("Weekly Limit Exhausted. Resets at 2024-02-29T00:00:00Z"), Date.parse("2024-02-25T00:00:00Z"))) .toBe(Date.parse("2024-02-29T00:00:00Z")); - expect(parseQuotaResetAt("resets at 2000-02-29T00:00:00Z", Date.parse("2000-02-25T00:00:00Z"))) + expect(parseQuotaResetAt(quotaBody("Weekly Limit Exhausted. Resets at 2000-02-29T00:00:00Z"), Date.parse("2000-02-25T00:00:00Z"))) .toBe(Date.parse("2000-02-29T00:00:00Z")); - expect(parseQuotaResetAt("resets at 2100-02-29T00:00:00Z", Date.parse("2100-02-25T00:00:00Z"))) + expect(parseQuotaResetAt(quotaBody("Weekly Limit Exhausted. Resets at 2100-02-29T00:00:00Z"), Date.parse("2100-02-25T00:00:00Z"))) + .toBeUndefined(); + }); + + test("rejects unrelated 429 prose and lookalike JSON from other providers", () => { + const date = "2026-09-09 03:30:06"; + expect(parseQuotaResetAt(`service resets at ${date}`, now)).toBeUndefined(); + expect(parseQuotaResetAt(JSON.stringify({ error: { code: "rate_limit_error", message: `service resets at ${date}` } }), now)) + .toBeUndefined(); + expect(parseQuotaResetAt(JSON.stringify({ error: { code: "other_provider", message: `Weekly Limit Exhausted. Resets at ${date}` } }), now)) .toBeUndefined(); }); @@ -122,12 +133,16 @@ describe("readQuotaResetAt", () => { describe("readQuotaResetAt — the read is bounded, not just the parse", () => { const now = Date.parse("2026-09-01T00:00:00Z"); - test("stops pulling after the cap instead of buffering the whole body", async () => { - // A chatty upstream must not make the rotation path read megabytes. This counts - // what the reader actually PULLED, not what the parser looked at — the two were - // different before this was fixed (`.text()` read it all, then sliced 4KB). + test("scans exactly the cap and replays overflow from a larger boundary chunk", async () => { + // A stream read is chunk-atomic, so the first pull may exceed 4 KiB. The parser + // receives exactly 4 KiB while the boundary overflow is replayed before later + // chunks; reading the rebuilt response must reproduce every byte in order. let pulled = 0; - const chunk = new TextEncoder().encode("x".repeat(64 * 1_024)); + const reset = JSON.stringify({ + error: { code: "rate_limit_error", message: "Weekly Limit Exhausted. Your limit will reset at 2026-09-05 12:00:00" }, + }); + const first = reset + " ".repeat(4_096 - reset.length) + "overflow-must-not-enter-the-parser"; + const chunk = new TextEncoder().encode(first.padEnd(64 * 1_024, "x")); const total = 5 * 1_024 * 1_024; const body = new ReadableStream({ pull(controller) { @@ -138,17 +153,55 @@ describe("readQuotaResetAt — the read is bounded, not just the parse", () => { pulled += chunk.byteLength; controller.enqueue(chunk); }, - }); + }, { highWaterMark: 0 }); - const { at } = await readQuotaResetAt(new Response(body, { status: 429 }), now); + const { at, response } = await readQuotaResetAt(new Response(body, { status: 429 }), now); - expect(at).toBeUndefined(); - expect(pulled).toBeLessThan(total / 4); + expect(at).toBe(Date.parse("2026-09-05T12:00:00Z")); + expect(pulled).toBe(chunk.byteLength); + expect(await response.text()).toBe(new TextDecoder().decode(chunk).repeat(total / chunk.byteLength)); }); test("still finds a reset that sits inside the cap", async () => { - const body = `{"error":{"message":"Weekly Limit Exhausted. Your limit will reset at 2026-09-05 12:00:00"}}`; + const body = `{"error":{"code":"rate_limit_error","message":"Weekly Limit Exhausted. Your limit will reset at 2026-09-05 12:00:00"}}`; expect((await readQuotaResetAt(new Response(body, { status: 429 }), now)).at) .toBe(Date.parse("2026-09-05T12:00:00Z")); }); + + test("a stalled peek times out, cancels its reader, and returns an unlocked response", async () => { + let cancelled = false; + const body = new ReadableStream({ + pull() { + return new Promise(() => {}); + }, + cancel() { + cancelled = true; + }, + }); + + const result = await readQuotaResetAt(new Response(body, { status: 429 }), { now, timeoutMs: 1 }); + + expect(result.at).toBeUndefined(); + expect(cancelled).toBe(true); + expect(result.response.body?.locked).toBe(false); + expect(await result.response.text()).toBe(""); + }); + + test("client cancellation rejects instead of producing a cooldown candidate", async () => { + const abort = new AbortController(); + const reason = new DOMException("client closed", "AbortError"); + const body = new ReadableStream({ + pull() { + abort.abort(reason); + return new Promise(() => {}); + }, + }); + + await expect(readQuotaResetAt(new Response(body, { status: 429 }), { + now, + signal: abort.signal, + timeoutMs: 1_000, + })).rejects.toMatchObject({ name: "AbortError", message: "client closed" }); + expect(body.locked).toBe(false); + }); }); diff --git a/tests/server/server-key-failover-e2e.test.ts b/tests/server/server-key-failover-e2e.test.ts index 9fe34cb8f40..a9753c6f27e 100644 --- a/tests/server/server-key-failover-e2e.test.ts +++ b/tests/server/server-key-failover-e2e.test.ts @@ -9,6 +9,7 @@ import { clearKeyCooldowns, getKeyCooldownUntil, rotateKeyOn429 } from "../../sr import { deriveXaiConvId } from "../../src/providers/xai-transport"; import { clearReasoningReplayCacheForTests } from "../../src/responses/reasoning-replay-cache"; import { startServer } from "../../src/server"; +import { handleResponses } from "../../src/server/responses"; import type { OcxConfig } from "../../src/types"; import { installIsolatedCodexHome, type IsolatedCodexHome } from "../helpers/isolated-codex-home"; import { removeTreeWithRetry } from "../helpers/remove-tree"; @@ -559,6 +560,65 @@ describe("server 429 key failover (end-to-end)", () => { } }); + test("client cancellation during the 429 body peek does not rotate or cool the key", async () => { + const bodyRead = Promise.withResolvers(); + const bodyCancelled = Promise.withResolvers(); + const seenAuth: string[] = []; + upstream = Bun.serve({ + hostname: "127.0.0.1", port: 0, + fetch(req) { + seenAuth.push(req.headers.get("authorization") ?? ""); + let pulls = 0; + return new Response(new ReadableStream({ + pull(controller) { + pulls += 1; + if (pulls === 1) { + controller.enqueue(new TextEncoder().encode('{"error":{"code":"rate_limit_error","message":"Weekly Limit Exhausted.')); + return; + } + bodyRead.resolve(); + return new Promise(() => {}); + }, + cancel() { + bodyCancelled.resolve(); + }, + }), { status: 429, headers: { "content-type": "application/json" } }); + }, + }); + const config = { + port: 0, hostname: "127.0.0.1", defaultProvider: "cancelled", + providers: { + cancelled: { + adapter: "openai-chat", authMode: "key", + baseUrl: `http://127.0.0.1:${upstream.port}/v1`, allowPrivateNetwork: true, + apiKey: "key-canc-000111222333", + apiKeyPool: [ + { id: "c1", key: "key-canc-000111222333", addedAt: 1 }, + { id: "c2", key: "key-cancelled-444555666777", addedAt: 2 }, + ], + }, + }, + } as OcxConfig; + saveConfig(config); + const abort = new AbortController(); + const pending = handleResponses(new Request("http://localhost/v1/responses", { + method: "POST", + headers: { "content-type": "application/json" }, + body: JSON.stringify({ model: "cancelled/some-model", input: "hello", stream: false }), + }), config, { model: "", provider: "" }, { abortSignal: abort.signal }); + + await bodyRead.promise; + abort.abort(new DOMException("client closed", "AbortError")); + const response = await pending; + + expect(response.status).toBe(499); + expect(await response.json()).toMatchObject({ error: { code: "client_cancelled" } }); + await bodyCancelled.promise; + expect(seenAuth).toEqual(["Bearer key-canc-000111222333"]); + expect(getKeyCooldownUntil("cancelled", "c1")).toBeNull(); + expect(loadConfig().providers.cancelled?.apiKey).toBe("key-canc-000111222333"); + }); + test("reasoning replay misses after a 429 rotates to a different physical key", async () => { const model = "reasoning-model"; const callId = "call_key_rotation";