diff --git a/src/codex/quota.ts b/src/codex/quota.ts index ae9152c0ed..3248186375 100644 --- a/src/codex/quota.ts +++ b/src/codex/quota.ts @@ -95,6 +95,9 @@ const MONTHLY_WINDOW_MIN_SECONDS = 28 * 24 * 60 * 60; */ const WEEKLY_WINDOW_MIN_SECONDS = 24 * 60 * 60; const MONTHLY_WINDOW_MIN_MINUTES = MONTHLY_WINDOW_MIN_SECONDS / 60; +// Derived, never written as a literal: the header parser and the WHAM parser must not be able +// to drift to different thresholds, which is the class of defect this pair exists to prevent. +const WEEKLY_WINDOW_MIN_MINUTES = WEEKLY_WINDOW_MIN_SECONDS / 60; const accountQuota = new Map(); let lastReconciledGeneration = 0; @@ -219,14 +222,24 @@ function isExplicitMonthlyWindow(window: WhamUsageWindow | null | undefined): bo } function isExplicitMonthlyWindowMinutes(windowMinutes: unknown): boolean { - const minutes = typeof windowMinutes === "number" - ? windowMinutes - : typeof windowMinutes === "string" && windowMinutes.trim() !== "" - ? Number(windowMinutes) + const minutes = windowMinutes_(windowMinutes); + return minutes !== undefined && minutes >= MONTHLY_WINDOW_MIN_MINUTES; +} + +/** The header wire reports a window duration in MINUTES; WHAM reports it in seconds. */ +function windowMinutes_(value: unknown): number | undefined { + const minutes = typeof value === "number" + ? value + : typeof value === "string" && value.trim() !== "" + ? Number(value) : undefined; - return typeof minutes === "number" - && Number.isFinite(minutes) - && minutes >= MONTHLY_WINDOW_MIN_MINUTES; + return typeof minutes === "number" && Number.isFinite(minutes) ? minutes : undefined; +} + +/** Minutes-domain twin of isExplicitShortWindow. Same strict `<`, same 24h discriminator. */ +function isExplicitShortWindowMinutes(value: unknown): boolean { + const minutes = windowMinutes_(value); + return minutes !== undefined && minutes > 0 && minutes < WEEKLY_WINDOW_MIN_MINUTES; } @@ -342,6 +355,12 @@ export function parseUpstreamQuotaHeaders(headers: Headers): Omit typeof value === "number" && Number.isFinite(value)); const maxPercent = percents.length > 0 ? Math.max(...percents) : undefined; // Credits-only snapshots prove neither usage nor exhaustion. Unknown must not @@ -49,6 +55,10 @@ function codexAccountQuotaEvidence(accountId: string, plan?: string): RouteQuota const resets = [ ...(monthly ? [] : [quota.weeklyResetAt]), quota.monthlyResetAt, + // Pair the reset with the window that can actually gate the next request: a burst-limited + // account recovers in hours, and reporting a distant weekly reset would defer a retry that + // is already safe. + quota.shortResetAt, ].filter((value): value is number => typeof value === "number" && Number.isFinite(value)) .filter(value => value > Date.now()); return { diff --git a/tests/codex-quota-parser-parity.test.ts b/tests/codex-quota-parser-parity.test.ts new file mode 100644 index 0000000000..1f909f4f4e --- /dev/null +++ b/tests/codex-quota-parser-parity.test.ts @@ -0,0 +1,112 @@ +import { describe, expect, it } from "bun:test"; +import { + clearAccountQuota, + parseUpstreamQuotaHeaders, + parseUsageQuota, + setAccountQuotaFromParsed, +} from "../src/codex/quota"; +import { codexPoolQuotaEvidence } from "../src/routing/quota"; + +/** + * The two quota parsers, pinned against each other. + * + * Codex reports the same account state twice: as response headers on every request, and as a + * WHAM usage payload on refresh. `parseUsageQuota` classified windows by DURATION from the + * start; `parseUpstreamQuotaHeaders` only knew "explicitly monthly, or else weekly". While + * Codex had no 5-hour window that difference was invisible. When the window came back for Plus + * and Team, the header path started filing a 5h reading as the weekly one, discarding the real + * weekly value and leaving the account exhausted long after the burst reset. + * + * This is the assertion that would have caught it: the parsers must agree about WHICH WINDOW a + * number belongs to, whichever wire it arrived on. It compares window assignment rather than + * whole objects, because the WHAM payload also carries provenance and Spark windows the header + * wire does not. + */ +describe("quota parser parity: headers and WHAM agree on window assignment", () => { + const cases = [ + { name: "Plus/Team 5h burst + 7-day weekly", minutes: 300, seconds: 18_000, primary: 97, secondary: 12 }, + { name: "Pro weekly-only", minutes: 10_080, seconds: 604_800, primary: 80, secondary: undefined }, + { name: "monthly plan with a weekly secondary", minutes: 43_800, seconds: 2_628_000, primary: 100, secondary: 22 }, + { name: "sub-hour burst", minutes: 15, seconds: 900, primary: 40, secondary: 5 }, + ] as const; + + /** Which field each percent landed in — the only thing both wires can be compared on. */ + function assignment(quota: Record | null): Record { + return { + shortPercent: quota?.shortPercent, + weeklyPercent: quota?.weeklyPercent, + monthlyPercent: quota?.monthlyPercent, + }; + } + + for (const testCase of cases) { + it(`agrees on ${testCase.name}`, () => { + const headers = new Headers({ + "x-codex-primary-used-percent": String(testCase.primary), + "x-codex-primary-window-minutes": String(testCase.minutes), + ...(testCase.secondary !== undefined + ? { + "x-codex-secondary-used-percent": String(testCase.secondary), + "x-codex-secondary-window-minutes": "10080", + } + : {}), + }); + const wham = parseUsageQuota({ + plan_type: "plus", + rate_limit: { + primary_window: { used_percent: testCase.primary, limit_window_seconds: testCase.seconds }, + ...(testCase.secondary !== undefined + ? { secondary_window: { used_percent: testCase.secondary, limit_window_seconds: 604_800 } } + : {}), + }, + }); + + expect(assignment(parseUpstreamQuotaHeaders(headers) as Record)) + .toEqual(assignment(wham as Record)); + }); + } + + it("the burst duration survives the header round trip in seconds", () => { + // The header wire speaks minutes and the stored field is seconds; a unit slip here would be + // silent, since both numbers are plausible durations. + const quota = parseUpstreamQuotaHeaders(new Headers({ + "x-codex-primary-used-percent": "50", + "x-codex-primary-window-minutes": "300", + })); + expect(quota?.shortWindowSeconds).toBe(18_000); + }); +}); + +/** + * The regression the parser fix would otherwise have introduced. + * + * `codexAccountQuotaEvidence` scored headroom from weekly and monthly only. That was survivable + * while the broken parser wrote 5h readings into `weeklyPercent` — routing saw the burst by + * accident. Correcting the parser without this fold would take a 5h-exhausted account from 3% + * headroom to 88% and route straight into a 429. + */ +describe("routing headroom accounts for the burst window", () => { + it("a 5h-exhausted account keeps low headroom despite a healthy weekly", () => { + clearAccountQuota(); + setAccountQuotaFromParsed("burst-acct", { shortPercent: 97, weeklyPercent: 12 }); + const evidence = codexPoolQuotaEvidence([{ accountId: "burst-acct", plan: "plus" }]); + expect(evidence.known).toBe(true); + expect(evidence.headroom).toBeLessThanOrEqual(0.05); + }); + + it("a fully exhausted burst window reports exhausted", () => { + clearAccountQuota(); + setAccountQuotaFromParsed("burst-dead", { shortPercent: 100, weeklyPercent: 8 }); + const evidence = codexPoolQuotaEvidence([{ accountId: "burst-dead", plan: "plus" }]); + expect(evidence.exhausted).toBe(true); + }); + + it("a healthy burst window does not suppress a real weekly limit", () => { + // The fold must not invert: the maximum still governs, so a near-full weekly still bites. + clearAccountQuota(); + setAccountQuotaFromParsed("weekly-bound", { shortPercent: 3, weeklyPercent: 96 }); + const evidence = codexPoolQuotaEvidence([{ accountId: "weekly-bound", plan: "plus" }]); + expect(evidence.headroom).toBeLessThanOrEqual(0.05); + }); +}); + diff --git a/tests/rate-limit-reset-credits.test.ts b/tests/rate-limit-reset-credits.test.ts index b775700c8d..45b9950fdf 100644 --- a/tests/rate-limit-reset-credits.test.ts +++ b/tests/rate-limit-reset-credits.test.ts @@ -481,4 +481,96 @@ describe("rate-limit reset credits", () => { }); }); }); + + /** + * Codex removed the 5-hour window and restored it for Plus and Team (Pro stays weekly-only). + * The header parser classified every non-monthly primary as weekly, so a 5h reading landed in + * weeklyPercent, the real weekly value was discarded, and the account stayed "exhausted" long + * after the burst window reset. + */ + describe("header window duration classification (5h restoration)", () => { + it("files a 5h primary as the burst window and the 7-day secondary as weekly", () => { + clearAccountQuota(); + const headers = new Headers({ + "x-codex-primary-used-percent": "97", + "x-codex-primary-window-minutes": "300", + "x-codex-primary-reset-at": "1787401330", + "x-codex-secondary-used-percent": "12", + "x-codex-secondary-window-minutes": "10080", + "x-codex-secondary-reset-at": "1788000000", + }); + applyAccountQuotaFromUpstreamHeaders("burst-A", headers); + expect(getAccountQuota("burst-A")).toEqual({ + shortPercent: 97, + shortResetAt: 1787401330, + shortWindowSeconds: 18000, + weeklyPercent: 12, + weeklyResetAt: 1788000000, + updatedAt: expect.any(Number), + }); + }); + + it("an exhausted burst window does not poison the weekly reading", () => { + // The damaging half of the bug: weeklyPercent=100 survives the 5h reset and keeps the + // account out of the pool until an unrelated WHAM refresh overwrites it. + clearAccountQuota(); + applyAccountQuotaFromUpstreamHeaders("burst-B", new Headers({ + "x-codex-primary-used-percent": "100", + "x-codex-primary-window-minutes": "300", + "x-codex-secondary-used-percent": "8", + "x-codex-secondary-window-minutes": "10080", + })); + const quota = getAccountQuota("burst-B"); + expect(quota?.shortPercent).toBe(100); + expect(quota?.weeklyPercent).toBe(8); + }); + + it("a sub-day window at 1439 minutes is short; 1440 minutes is not", () => { + // Strict `<` against the 24h discriminator, matching isExplicitShortWindow exactly. + clearAccountQuota(); + applyAccountQuotaFromUpstreamHeaders("edge-below", new Headers({ + "x-codex-primary-used-percent": "60", + "x-codex-primary-window-minutes": "1439", + })); + expect(getAccountQuota("edge-below")?.shortPercent).toBe(60); + expect(getAccountQuota("edge-below")?.weeklyPercent).toBeUndefined(); + + clearAccountQuota(); + applyAccountQuotaFromUpstreamHeaders("edge-at", new Headers({ + "x-codex-primary-used-percent": "60", + "x-codex-primary-window-minutes": "1440", + })); + expect(getAccountQuota("edge-at")?.weeklyPercent).toBe(60); + expect(getAccountQuota("edge-at")?.shortPercent).toBeUndefined(); + }); + + it("a primary with no declared duration stays weekly", () => { + // Duration classification is opt-in on a DECLARED duration. Legacy payloads omit the + // header, and guessing there would reclassify every account that predates the field. + clearAccountQuota(); + applyAccountQuotaFromUpstreamHeaders("legacy-A", new Headers({ + "x-codex-primary-used-percent": "80", + "x-codex-primary-reset-at": "1787000000", + })); + expect(getAccountQuota("legacy-A")).toEqual({ + weeklyPercent: 80, + weeklyResetAt: 1787000000, + updatedAt: expect.any(Number), + }); + }); + + it("the reset instant travels with its own window", () => { + clearAccountQuota(); + applyAccountQuotaFromUpstreamHeaders("reset-A", new Headers({ + "x-codex-primary-used-percent": "50", + "x-codex-primary-window-minutes": "300", + "x-codex-primary-reset-at": "1787401330", + })); + const quota = getAccountQuota("reset-A"); + expect(quota?.shortResetAt).toBe(1787401330); + // The primary reset belongs to the burst window; it must not be reported as the weekly one. + expect(quota?.weeklyResetAt).toBeUndefined(); + }); + }); + });