fix(oauth): skip a second POST of a consumed Claude refresh token (#13874)

HealthCheck queued behind a Layer 2 refresh still presented the old
refresh_token after serializeRefresh released, which burned the family
and then nulled the row. Re-check rotation inside the lane, record
Layer 2 rotations, and keep the Claude refresh token on invalid_grant.

Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: diegosouzapw <8016841+diegosouzapw@users.noreply.github.com>

changelog.d/fixes/claude-oauth-serialized-stale-refresh.md

@@ -0,0 +1 @@
+- **fix(oauth):** stop posting a Claude refresh token that another in-process refresh already consumed. Re-check the rotation map and DB inside `serializeRefresh` (both Layer 1 and Layer 2), record Layer 2 rotations, re-read the connection uncached on `invalid_grant`, and keep the Claude `refreshToken` instead of nulling it into sticky `no_refresh_token`.

open-sse/services/tokenRefresh.ts

@@ -583,8 +583,11 @@ export async function getAccessToken(
   // the legacy `connectionId`-less path would silently swallow the callback,
   // leaving DB rows out of sync with rotated tokens (Codex/OpenAI). We still
   // resolve the promise to all waiters with the refreshed credentials.
-  const refreshPromise = serializeRefresh(provider, () =>
-    _getAccessTokenInternal(provider, credentials, log, proxyConfig)
+  const refreshPromise = _getAccessTokenWithStalenessCheck(
+    provider,
+    credentials,
+    log,
+    proxyConfig
   )
     .then(async (result) => {
       if (result?.accessToken && effectiveOnPersist) {
@@ -620,17 +623,19 @@ export async function getAccessToken(
 }
 
 /**
- * Internal helper: performs the DB staleness check then calls the actual refresh.
- * Only called from the per-connection mutex path (Layer 1 above).
+ * Internal helper: waits for the rotation-group lane, then re-checks freshness
+ * BEFORE the network POST. Lookup/DB re-read must live inside serializeRefresh:
+ * a HealthCheck that snapshotted the old refresh_token can sit on the lane
+ * while Layer 2 consumes it; checking only before the wait still POSTs the
+ * consumed token and burns the family (Claude/Anthropic, Auth0 Codex).
  */
 async function _getAccessTokenWithStalenessCheck(provider, credentials, log, proxyConfig) {
-  // ROTATION MAP CHECK (codex-multi-auth pattern): if this refresh_token was
-  // rotated very recently (within ROTATION_MAP_TTL_MS), reuse the cached new
-  // tokens INSTEAD of hitting upstream. Auth0 treats re-use of a rotated token
-  // as a security event and revokes the entire token family — fatal for
-  // multi-account Codex setups. The in-memory rotation map catches this even
-  // when the caller bypasses the DB staleness path (no connectionId, stale
-  // in-memory credentials in retries, etc.).
+  return serializeRefresh(provider, () =>
+    _refreshWithFreshCredentials(provider, credentials, log, proxyConfig)
+  );
+}
+
+async function _refreshWithFreshCredentials(provider, credentials, log, proxyConfig) {
   const rotated = lookupRotation(provider, credentials.refreshToken);
   if (rotated) {
     log?.info?.(
@@ -640,11 +645,6 @@ async function _getAccessTokenWithStalenessCheck(provider, credentials, log, pro
     return rotated.result;
   }
 
-  // RACE CONDITION PREVENTION:
-  // If the credentials object in memory is stale (e.g. it waited in a semaphore while another
-  // request refreshed the token), using its OLD refreshToken will cause the provider (e.g. OpenAI)
-  // to reject it with 'refresh_token_reused' and revoke the new token family.
-  // We MUST check if the DB has a newer token before proceeding with a network refresh.
   if (credentials.connectionId) {
     try {
       const { getProviderConnectionById } = await import("@/lib/db/providers");
@@ -659,31 +659,17 @@ async function _getAccessTokenWithStalenessCheck(provider, credentials, log, pro
             `Stale token detected in memory for ${provider}. Using refreshed token from DB.`
           );
 
-          // If the DB token is not expired, we can just return it!
           if (dbExpiresAt > now + 60000) {
-            // 60 seconds buffer
             log?.info?.("TOKEN_REFRESH", `DB token is still valid. Skipping OAuth refresh.`);
             return {
               accessToken: dbConnection.accessToken,
               refreshToken: dbConnection.refreshToken,
-              // Return absolute expiresAt so downstream callers do NOT recompute lifetime
-              // from a relative expiresIn value (which would incorrectly extend the TTL).
-              // expiresIn intentionally omitted here.
               expiresAt: dbConnection.expiresAt,
             };
-          } else {
-            // DB token is also expired, but it's the NEWEST one. We must use it to refresh.
-            credentials.refreshToken = dbConnection.refreshToken;
-            credentials.accessToken = dbConnection.accessToken;
           }
+          credentials.refreshToken = dbConnection.refreshToken;
+          credentials.accessToken = dbConnection.accessToken;
         }
-        // NOTE: Fix F (skip when DB == memory and DB > now+60s) was intentionally
-        // removed. The caller (checkAndRefreshToken) already decided to refresh
-        // because the token is within TOKEN_EXPIRY_BUFFER_MS of expiry. Re-checking
-        // with a tighter 60-second window here would skip legitimate refreshes and
-        // let near-expired tokens hit the upstream. Layer-1 mutex (per-connection)
-        // and Layer-2 dedup (token-hash) already prevent concurrent refreshes for
-        // the import-burst scenario.
       }
     } catch (e) {
       log?.warn?.(
@@ -694,16 +680,8 @@ async function _getAccessTokenWithStalenessCheck(provider, credentials, log, pro
   }
 
   const oldRefreshToken = credentials.refreshToken;
-  // Front 1: serialize the network refresh across all connections of the same
-  // rotation group (e.g. Codex+openai share one Auth0 client) so two sibling
-  // accounts never refresh concurrently and trip Auth0 family revocation.
-  const result = await serializeRefresh(provider, () =>
-    _getAccessTokenInternal(provider, credentials, log, proxyConfig)
-  );
+  const result = await _getAccessTokenInternal(provider, credentials, log, proxyConfig);
 
-  // Record the rotation so subsequent stale callers can be redirected to the
-  // new tokens without re-hitting upstream (which would trigger Auth0 family
-  // revocation). Only records when the refresh actually rotated the token.
   if (
     result &&
     typeof result === "object" &&

src/lib/tokenHealthCheck.ts

@@ -11,7 +11,11 @@
  * updates the DB, and logs the result.
  */
 
-import { getProviderConnections, updateProviderConnection } from "@/lib/db/providers";
+import {
+  getProviderConnections,
+  getProviderConnectionById,
+  updateProviderConnection,
+} from "@/lib/db/providers";
 import { getCachedProviderConnectionById } from "@/lib/db/readCache";
 import { getSettings } from "@/lib/db/settings";
 import { resolveGuardedProxyConfig } from "@/lib/tokenHealthCheckProxyGuard";
@@ -42,6 +46,24 @@ const TICK_MS = 60 * 1000; // sweep interval: every 60 seconds (restored — #77
 const DEFAULT_BATCH_SIZE = 20;
 const DEFAULT_HEALTH_CHECK_INTERVAL_MIN = 60; // default per-connection interval
 const EXPIRED_RETRY_MAX = 3; // max retry attempts for expired connections before giving up
+const ROTATING_REFRESH_PROVIDERS = new Set([
+  "codex",
+  "openai",
+  "kimi-coding",
+  "cline",
+  "kiro",
+  "amazon-q",
+  "gitlab-duo",
+  "claude",
+  "openference",
+]);
+
+export function shouldNullRefreshTokenAfterUnrecoverable(provider: unknown): boolean {
+  const id = String(provider || "").toLowerCase();
+  if (id === "claude") return false;
+  return ROTATING_REFRESH_PROVIDERS.has(id);
+}
+
 const EXPIRED_RETRY_BACKOFF_MIN = 5; // backoff between expired retries (minutes)
 
 function isBuildProcess(): boolean {
@@ -897,17 +919,6 @@ export async function checkConnection(conn) {
   // and is the root cause of "adding account B invalidates account A" reports.
   // The interval path is kept ONLY for non-rotating providers where token state can
   // drift silently (e.g. cookie-based, opaque sessions without expires_at).
-  const ROTATING_REFRESH_PROVIDERS = new Set([
-    "codex",
-    "openai",
-    "kimi-coding",
-    "cline",
-    "kiro",
-    "amazon-q",
-    "gitlab-duo",
-    "claude",
-    "openference",
-  ]);
   const isRotatingProvider = ROTATING_REFRESH_PROVIDERS.has(
     String(conn.provider || "").toLowerCase()
   );
@@ -1097,7 +1108,7 @@ export async function checkConnection(conn) {
   // Once used, the old token is permanently invalidated.
   // Retrying will never succeed → deactivate and stop the loop.
   if (isUnrecoverableRefreshError(result)) {
-    const currentConnection = await getCachedProviderConnectionById(conn.id);
+    const currentConnection = await getProviderConnectionById(conn.id);
     const credentialsChangedSinceSweep =
       !!currentConnection &&
       (currentConnection.refreshToken !== attemptedRefreshToken ||
@@ -1157,11 +1168,7 @@ export async function checkConnection(conn) {
       // gemini) the stored refresh_token is the user's only recovery
       // artifact — nulling it caused #3679 (the connection reports "No valid refresh
       // token available" and can never recover even after re-activation). Preserve it.
-      // PRESERVE_REFRESH_TOKEN_PROVIDERS (Claude) opt out too: nulling on the first
-      // failure makes the #11414 retry budget above unreachable (#13183).
-      ...(isRotatingProvider && !preservesRefreshTokenOnUnrecoverable(conn.provider)
-        ? { refreshToken: null }
-        : {}),
+      ...(shouldNullRefreshTokenAfterUnrecoverable(conn.provider) ? { refreshToken: null } : {}),
     });
     logError(
       `${LOG_PREFIX} ✗ ${conn.provider}/${getConnectionLogLabel(conn)} — ` +

tests/unit/token-refresh-race-comprehensive.test.ts

@@ -176,3 +176,27 @@ test("Imports: base.ts imports runWithOnPersist from open-sse tokenRefresh", asy
   assert.match(src, /runWithOnPersist/);
   assert.match(src, /from\s+"\.\.\/services\/tokenRefresh\.ts"/);
 });
+
+
+test("serialized refresh re-checks rotation inside the lane, not before waiting", async () => {
+  const src = await read("open-sse/services/tokenRefresh.ts");
+  const start = src.indexOf("async function _getAccessTokenWithStalenessCheck");
+  const inner = src.indexOf("async function _refreshWithFreshCredentials");
+  assert.ok(start >= 0 && inner > start, "staleness helper must wrap the freshness re-check");
+  const wrapper = src.slice(start, inner);
+  assert.match(
+    wrapper,
+    /serializeRefresh\(provider,\s*\(\)\s*=>/,
+    "the network POST must stay behind serializeRefresh"
+  );
+  assert.match(wrapper, /_refreshWithFreshCredentials/);
+  assert.doesNotMatch(
+    wrapper,
+    /lookupRotation/,
+    "lookupRotation before serializeRefresh is the race that burns a Claude refresh token"
+  );
+  const body = src.slice(inner, inner + 2500);
+  assert.match(body, /lookupRotation\(/);
+  assert.match(body, /recordRotation\(/);
+  assert.match(body, /_getAccessTokenInternal\(/);
+});

tests/unit/token-refresh-serialized-stale-rotation.test.ts

@@ -0,0 +1,154 @@
+import test from "node:test";
+import assert from "node:assert/strict";
+
+const tokenRefresh = await import("../../open-sse/services/tokenRefresh.ts");
+const { __resetRefreshSerializerForTest } = await import("../../open-sse/services/refreshSerializer.ts");
+const { lookupRotation } = await import("../../open-sse/services/tokenRefresh/rotationMap.ts");
+
+const { getAccessToken } = tokenRefresh;
+
+type LogLevel = "debug" | "info" | "warn" | "error";
+type LogEntry = { level: LogLevel; message: unknown };
+
+function createLog() {
+  const entries: LogEntry[] = [];
+  const push = (level: LogLevel, args: unknown[]) => {
+    entries.push({ level, message: args[1] });
+  };
+  return {
+    entries,
+    debug: (...args: unknown[]) => push("debug", args),
+    info: (...args: unknown[]) => push("info", args),
+    warn: (...args: unknown[]) => push("warn", args),
+    error: (...args: unknown[]) => push("error", args),
+  };
+}
+
+function jsonResponse(body: unknown, status = 200) {
+  return new Response(JSON.stringify(body), {
+    status,
+    headers: { "content-type": "application/json" },
+  });
+}
+
+function bodyToString(body: BodyInit | null | undefined) {
+  if (typeof body === "string") return body;
+  if (body instanceof URLSearchParams) return body.toString();
+  return String(body ?? "");
+}
+
+function refreshTokenFromBody(body: BodyInit | null | undefined) {
+  return new URLSearchParams(bodyToString(body)).get("refresh_token");
+}
+
+async function withMockedFetch<TResult>(fetchImpl: typeof fetch, fn: () => Promise<TResult>) {
+  const originalFetch = globalThis.fetch;
+  globalThis.fetch = fetchImpl;
+  try {
+    return await fn();
+  } finally {
+    globalThis.fetch = originalFetch;
+  }
+}
+
+function resetRefreshState() {
+  tokenRefresh._clearTokenRotationMap();
+  __resetRefreshSerializerForTest();
+}
+
+test.beforeEach(() => {
+  resetRefreshState();
+});
+
+test("getAccessToken_Layer1QueuedBehindLayer2_DoesNotPostConsumedClaudeRefreshToken", async () => {
+  const previousSpacing = process.env.CODEX_REFRESH_SPACING_MS;
+  process.env.CODEX_REFRESH_SPACING_MS = "0";
+  const log = createLog();
+  const presented: string[] = [];
+  let firstPostEntered = false;
+  let releaseFirstPost!: () => void;
+  const holdFirstPost = new Promise<void>((resolve) => {
+    releaseFirstPost = resolve;
+  });
+
+  try {
+    await withMockedFetch(async (_url, options = {}) => {
+      const presentedToken = refreshTokenFromBody(options.body);
+      presented.push(presentedToken || "");
+      if (presentedToken === "old-rt" && !firstPostEntered) {
+        firstPostEntered = true;
+        await holdFirstPost;
+        return jsonResponse({
+          access_token: "new-access",
+          refresh_token: "new-rt",
+          expires_in: 28800,
+        });
+      }
+      if (presentedToken === "old-rt") {
+        return jsonResponse({ error: "invalid_grant", error_description: "refresh_token_reused" }, 400);
+      }
+      throw new Error(`unexpected refresh_token ${presentedToken}`);
+    }, async () => {
+      const layer2 = getAccessToken("claude", { refreshToken: "old-rt" }, log);
+      await new Promise<void>((resolve, reject) => {
+        const started = Date.now();
+        const tick = () => {
+          if (firstPostEntered) {
+            resolve();
+            return;
+          }
+          if (Date.now() - started > 2000) {
+            reject(new Error("Layer 2 never reached the Anthropic token endpoint"));
+            return;
+          }
+          setTimeout(tick, 5);
+        };
+        tick();
+      });
+
+      const layer1 = getAccessToken(
+        "claude",
+        { connectionId: "healthcheck-conn", refreshToken: "old-rt" },
+        log
+      );
+      await new Promise((resolve) => setTimeout(resolve, 30));
+      releaseFirstPost();
+
+      const [layer2Result, layer1Result] = await Promise.all([layer2, layer1]);
+
+      assert.deepEqual(presented, ["old-rt"], "the consumed refresh token must be POSTed once");
+      assert.equal(layer2Result?.accessToken, "new-access");
+      assert.equal(layer2Result?.refreshToken, "new-rt");
+      assert.equal(layer1Result?.accessToken, "new-access");
+      assert.equal(layer1Result?.refreshToken, "new-rt");
+      assert.notEqual(
+        (layer1Result as { error?: string } | null)?.error,
+        "unrecoverable_refresh_error",
+        "Layer 1 must reuse the rotated tokens instead of burning the family"
+      );
+    });
+  } finally {
+    if (previousSpacing === undefined) delete process.env.CODEX_REFRESH_SPACING_MS;
+    else process.env.CODEX_REFRESH_SPACING_MS = previousSpacing;
+    resetRefreshState();
+  }
+});
+
+test("getAccessToken_Layer2Refresh_RecordsRotationForTheConsumedToken", async () => {
+  const log = createLog();
+
+  await withMockedFetch(async () => {
+    return jsonResponse({
+      access_token: "layer2-access",
+      refresh_token: "layer2-new-rt",
+      expires_in: 28800,
+    });
+  }, async () => {
+    const result = await getAccessToken("claude", { refreshToken: "layer2-old-rt" }, log);
+    assert.equal(result?.refreshToken, "layer2-new-rt");
+    const cached = lookupRotation("claude", "layer2-old-rt");
+    assert.ok(cached, "Layer 2 must record the rotation so a later stale caller can skip upstream");
+    assert.equal(cached.result.refreshToken, "layer2-new-rt");
+    assert.equal(cached.result.accessToken, "layer2-access");
+  });
+});

tests/unit/token-refresh-service.test.ts

@@ -962,9 +962,10 @@ test("getAccessToken cleans the in-flight cache after resolve and separates diff
             log
           );
 
-          assert.equal(fetchCount, 3);
+          assert.equal(fetchCount, 2, "same consumed refresh token is served from the rotation map");
           assert.equal(first.accessToken, "access-refresh-a");
           assert.equal(second.accessToken, "access-refresh-a");
+          assert.equal(second.refreshToken, "next-refresh-a");
           assert.equal(third.accessToken, "access-refresh-b");
         }
       );

tests/unit/tokenHealthCheck-unrecoverable-reread.test.ts

@@ -0,0 +1,65 @@
+import test from "node:test";
+import assert from "node:assert/strict";
+import { readFile } from "node:fs/promises";
+import path from "node:path";
+
+const root = path.resolve(import.meta.dirname, "../..");
+const src = await readFile(path.join(root, "src/lib/tokenHealthCheck.ts"), "utf8");
+
+function unrecoverableSlice() {
+  const idx = src.indexOf("if (isUnrecoverableRefreshError(result))");
+  assert.ok(idx >= 0, "unrecoverable refresh branch must exist");
+  return src.slice(idx, idx + 3500);
+}
+
+test("tokenHealthCheck_UnrecoverableRefresh_RereadsConnectionUncached", () => {
+  const slice = unrecoverableSlice();
+  assert.match(
+    slice,
+    /getProviderConnectionById\(/,
+    "a concurrent Layer 2 persist can land between the sweep snapshot and invalid_grant; the cached row still holds the consumed refresh token and would skip the changed-since-sweep guard"
+  );
+  assert.doesNotMatch(
+    slice,
+    /getCachedProviderConnectionById/,
+    "the 5s connection-by-id cache is how credentialsChangedSinceSweep missed the just-persisted rotation"
+  );
+});
+
+test("tokenHealthCheck_UnrecoverableRefresh_DoesNotNullClaudeRefreshToken", () => {
+  const slice = unrecoverableSlice();
+  assert.match(
+    slice,
+    /shouldNullRefreshTokenAfterUnrecoverable/,
+    "Claude rotating tokens must not be wiped on the first invalid_grant; the live access token plus the new refresh token in DB are still recoverable"
+  );
+  assert.doesNotMatch(
+    slice,
+    /\.\.\.\(isRotatingProvider\s*\?\s*\{\s*refreshToken:\s*null\s*\}\s*:\s*\{\}\)/,
+    "the blanket rotating-provider null is what turns a dual-refresh race into sticky no_refresh_token"
+  );
+});
+
+test("shouldNullRefreshTokenAfterUnrecoverable_Claude_IsFalse", async () => {
+  const { shouldNullRefreshTokenAfterUnrecoverable, stopTokenHealthCheck } =
+    await import("../../src/lib/tokenHealthCheck.ts");
+  stopTokenHealthCheck();
+  assert.equal(shouldNullRefreshTokenAfterUnrecoverable("claude"), false);
+  assert.equal(shouldNullRefreshTokenAfterUnrecoverable("Claude"), false);
+});
+
+test("shouldNullRefreshTokenAfterUnrecoverable_Codex_IsTrue", async () => {
+  const { shouldNullRefreshTokenAfterUnrecoverable, stopTokenHealthCheck } =
+    await import("../../src/lib/tokenHealthCheck.ts");
+  stopTokenHealthCheck();
+  assert.equal(shouldNullRefreshTokenAfterUnrecoverable("codex"), true);
+  assert.equal(shouldNullRefreshTokenAfterUnrecoverable("openai"), true);
+});
+
+test("shouldNullRefreshTokenAfterUnrecoverable_Google_IsFalse", async () => {
+  const { shouldNullRefreshTokenAfterUnrecoverable, stopTokenHealthCheck } =
+    await import("../../src/lib/tokenHealthCheck.ts");
+  stopTokenHealthCheck();
+  assert.equal(shouldNullRefreshTokenAfterUnrecoverable("gemini"), false);
+  assert.equal(shouldNullRefreshTokenAfterUnrecoverable("antigravity"), false);
+});