From c4a993184e2ee47107126684dfa1f34fbcd10ef5 Mon Sep 17 00:00:00 2001 From: diegosouzapw Date: Tue, 2 Jun 2026 08:47:51 -0300 Subject: [PATCH] fix(quota): never flag rotating-refresh providers expired from the quota sync MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The quota-sync path deliberately reuses a rotating-refresh provider's (Codex/ OpenAI/Claude — see refreshSerializer ROTATION_LOCK_GROUP) access_token WITHOUT proactively refreshing it (#3019, to avoid the Auth0 family-revocation cascade). When that token is expired the codex usage fetch returns "token expired", and syncExpiredStatusIfNeeded then flagged the connection testStatus="expired" — a false-negative: the credential is still valid (expires_at in the future) and the reactive serialized 401 path refreshes the access_token on next use. Symptom: freshly-added Codex accounts showed "expired" with no quota on the quota page, while a providers-page refresh turned them green. They never lost access — only the quota sync mislabeled them. Fix: extract the decision into the pure, exported `quotaPathShouldMarkExpired()` and skip rotating providers (rotationGroupFor !== null). Their status is owned by the reactive path / connection test, never the quota sync. Adds unit coverage. --- src/lib/usage/providerLimits.ts | 44 ++++++++++++++--- ...ider-limits-rotating-expired-guard.test.ts | 49 +++++++++++++++++++ 2 files changed, 85 insertions(+), 8 deletions(-) create mode 100644 tests/unit/provider-limits-rotating-expired-guard.test.ts diff --git a/src/lib/usage/providerLimits.ts b/src/lib/usage/providerLimits.ts index 0c29f5988c..61d3fd7096 100644 --- a/src/lib/usage/providerLimits.ts +++ b/src/lib/usage/providerLimits.ts @@ -213,15 +213,43 @@ function shouldFailClosedForProviderLimitsProxy( return connection.authType === "oauth" && isAccountScopedProxyResolution(proxyInfo); } -async function syncExpiredStatusIfNeeded(connection: ProviderConnectionLike, usage: JsonRecord) { - const errorMessage = typeof usage.message === "string" ? usage.message.toLowerCase() : ""; - const isAuthError = - errorMessage.includes("token expired") || - errorMessage.includes("access denied") || - errorMessage.includes("re-authenticate") || - errorMessage.includes("unauthorized"); +/** + * Decide whether the quota-sync path should flag a connection `expired` from an + * auth-style usage error. Exported for unit testing. + * + * Rotating-refresh providers (Codex/OpenAI/Claude/etc. — see refreshSerializer's + * ROTATION_LOCK_GROUP) have their access_token deliberately NOT proactively + * refreshed in this quota path (#3019, to avoid the Auth0 family-revocation + * cascade). So a "token expired" from the quota fetch is a recoverable + * false-negative: the credential is still valid (its `expires_at` is in the + * future) and the reactive, serialized 401 path refreshes the access_token on + * next use. Flagging it `expired` hides a healthy account from the quota page + * (observed: freshly-added Codex accounts flagged expired while a providers-page + * refresh turns them green). So never mark a rotating provider expired from the + * quota sync — leave its status to the reactive path / connection test. + */ +export function quotaPathShouldMarkExpired( + provider: string, + usageMessage: unknown, + currentTestStatus: string | null | undefined +): boolean { + if (currentTestStatus === "expired") return false; - if (!isAuthError || connection.testStatus === "expired") { + const message = typeof usageMessage === "string" ? usageMessage.toLowerCase() : ""; + const isAuthError = + message.includes("token expired") || + message.includes("access denied") || + message.includes("re-authenticate") || + message.includes("unauthorized"); + if (!isAuthError) return false; + + if (rotationGroupFor(provider) !== null) return false; + + return true; +} + +async function syncExpiredStatusIfNeeded(connection: ProviderConnectionLike, usage: JsonRecord) { + if (!quotaPathShouldMarkExpired(connection.provider, usage.message, connection.testStatus)) { return; } diff --git a/tests/unit/provider-limits-rotating-expired-guard.test.ts b/tests/unit/provider-limits-rotating-expired-guard.test.ts new file mode 100644 index 0000000000..ecbe258be6 --- /dev/null +++ b/tests/unit/provider-limits-rotating-expired-guard.test.ts @@ -0,0 +1,49 @@ +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"; + +// providerLimits.ts touches the DB singleton at import time; give it a scratch dir. +const TEST_DATA_DIR = fs.mkdtempSync(path.join(os.tmpdir(), "omniroute-rotating-expired-guard-")); +process.env.DATA_DIR = TEST_DATA_DIR; +process.env.API_KEY_SECRET = process.env.API_KEY_SECRET || "rotating-expired-guard-secret"; + +const { quotaPathShouldMarkExpired } = await import("../../src/lib/usage/providerLimits.ts"); + +test.after(() => { + fs.rmSync(TEST_DATA_DIR, { recursive: true, force: true }); +}); + +// Regression: the quota sync reuses a rotating provider's (possibly expired) +// access_token without refreshing it (#3019, to avoid the Auth0 family-revocation +// cascade). A "token expired" from that fetch is recoverable, NOT a real expiry — +// flagging it expired hid freshly-added Codex accounts from the quota page even +// though a providers-page refresh turned them green. +test("rotating providers are NEVER flagged expired from the quota path", () => { + for (const provider of ["codex", "openai", "claude", "kiro", "qwen", "gitlab-duo"]) { + assert.equal( + quotaPathShouldMarkExpired(provider, "Token expired, please re-authenticate", "active"), + false, + `${provider} (rotating) must not be marked expired by the quota sync` + ); + } +}); + +test("non-rotating OAuth providers are still flagged expired on a genuine auth error", () => { + assert.equal(quotaPathShouldMarkExpired("github", "token expired", "active"), true); + assert.equal(quotaPathShouldMarkExpired("github", "Unauthorized", "active"), true); + assert.equal(quotaPathShouldMarkExpired("cursor", "Access denied", "active"), true); +}); + +test("non-auth usage messages never trigger an expired flag", () => { + assert.equal(quotaPathShouldMarkExpired("github", "rate limit exceeded", "active"), false); + assert.equal(quotaPathShouldMarkExpired("github", "", "active"), false); + assert.equal(quotaPathShouldMarkExpired("github", undefined, "active"), false); + assert.equal(quotaPathShouldMarkExpired("github", { nested: true }, "active"), false); +}); + +test("an already-expired connection is left untouched (no redundant write)", () => { + assert.equal(quotaPathShouldMarkExpired("github", "token expired", "expired"), false); + assert.equal(quotaPathShouldMarkExpired("codex", "token expired", "expired"), false); +});