fix(responses): rebind credential identity on every OAuth 429 rotation

src/server/responses/core.ts

@@ -338,6 +338,7 @@ import {
   type ProviderExecutedCallType,
 } from "../responses-undeclared-tool-guard";
 import { createGithubCopilotResponsesBlockRewrite } from "../github-copilot-responses-repair";
+import { GITHUB_COPILOT_DEFAULT_API_BASE, validateCopilotApiBaseUrl } from "../../oauth/github-copilot";
 import { responsesJsonToSseStream } from "../responses-json-events";
 import { guardTerminalEventStream } from "./terminal-guard";
 import {
@@ -994,6 +995,24 @@ function shouldDeferCodexResetDerivedCooldown(response: Response, enabled?: bool
     && computeQuotaCooldown(codexQuotaOutcomeMeta(response)).source === "reset-derived";
 }
 
+/**
+ * Resolve the Copilot origin for a credential we just refreshed during failover.
+ *
+ * The tempting fallback — `refreshed.apiBaseUrl ?? getOAuthCredentialApiBaseUrl(provider)` —
+ * is account-blind on its second arm: `getOAuthCredentialApiBaseUrl` reads the ACTIVE
+ * credential, and a generic 429 rotation does not promote the account it rotated to. So a
+ * legacy account B carrying no allowlisted `apiBaseUrl` would be rebuilt as B's bearer paired
+ * with A's origin — one account's token sent to another account's host.
+ *
+ * Fail closed to the canonical Copilot origin instead. Never consult, and never retain, a
+ * different account's route to fill a gap in this one.
+ */
+function copilotOriginForRefreshedCredential(
+  refreshed: { apiBaseUrl?: string | undefined },
+): string | undefined {
+  return validateCopilotApiBaseUrl(refreshed.apiBaseUrl) ?? GITHUB_COPILOT_DEFAULT_API_BASE;
+}
+
 /**
  * One bounded alternate-account retry for Codex pool auth. Used for allow-listed
  * model-400 and for pre-stream 429/402 quota failures (#584).
@@ -2841,6 +2860,30 @@ async function handleResponsesInner(
     if (snapshot.projectId) rotatedProvider = { ...rotatedProvider, project: snapshot.projectId };
     route.provider = rotatedProvider;
     if (route.providerName === "kiro") parsed._kiroAuthContext = { ...(snapshot.kiro ?? {}) };
+    // Rotating the credential without rotating the IDENTITY of that credential leaves the two
+    // describing different accounts. `sentOAuthSnapshot` feeds the 401 forced refresh, which
+    // refreshes the SNAPSHOT's account rather than whichever account is on the route now — so a
+    // stale snapshot makes a later 401 renew the account we just rotated AWAY from, while the
+    // transport still points at the rotated one. `replayOAuthCredentialSnapshot` scopes reasoning
+    // replay, so a stale value lets continuation state minted under one account be replayed under
+    // another.
+    //
+    // This lives in the helper rather than at each call site on purpose: three rotation sites
+    // already disagreed about which of these to update, and the one that got it right (runTurn)
+    // got it right by hand. A fourth site would have to remember too. Here it cannot forget.
+    if (isOAuth401ReplayProvider) sentOAuthSnapshot = snapshot;
+    replayOAuthCredentialSnapshot = { accountId: snapshot.accountId, generation: snapshot.generation };
+    // A Cursor conversation and identity scope are credential-scoped in the same way. runTurn
+    // cleared them by hand and the other two sites did not, so the same rotation that swapped the
+    // bearer could carry the previous account's conversation forward - and for image turns
+    // src/images/loop.ts copies that id back onto the outer request, which is how it outlives the
+    // rotation. Clearing here means a rotated adapter always derives its own.
+    parsed._cursorIdentityScope = undefined;
+    parsed._cursorConversationId = undefined;
+    if (parsed._providerContinuation?.cursor) {
+      const { cursor: _discardedCursor, ...otherProviderState } = parsed._providerContinuation;
+      parsed._providerContinuation = otherProviderState;
+    }
     return true;
   };
   const anthropicSessionKey = route.providerName === "anthropic" && route.provider.authMode === "oauth"
@@ -3538,7 +3581,7 @@ async function handleResponsesInner(
         route.providerName,
         { ...route.provider, apiKey: refreshed.accessToken },
         parsed.options.promptCacheKey,
-        route.providerName === "github-copilot" ? getOAuthCredentialApiBaseUrl(route.providerName) : undefined,
+        route.providerName === "github-copilot" ? copilotOriginForRefreshedCredential(refreshed) : undefined,
       );
       route.provider = refreshedProvider;
       const refreshedAdapter = resolveAdapter(
@@ -4341,6 +4384,11 @@ async function handleResponsesInner(
       providerName: route.providerName,
       provider: route.provider,
       adapterName: rotatedAdapter.name,
+      // Omitting this fails the OAuth replay key CLOSED, which looks safe and is not: the scope
+      // then carries no account identity at all, so it cannot distinguish state minted under the
+      // account we just rotated away from. `applyFailoverSnapshot` now keeps this value in step
+      // with the rotated credential, so passing it is what makes the scope account-accurate.
+      oauthCredentialSnapshot: replayOAuthCredentialSnapshot,
     });
     return rotatedAdapter;
   };
@@ -4629,15 +4677,6 @@ async function handleResponsesInner(
         genericFailoverAccountId = nextAccountId;
         genericFailovers += 1;
         if (!applyFailoverSnapshot(snapshot)) return false;
-        // A Cursor conversation/checkpoint is credential-scoped. The failed attempt emitted no
-        // client-visible bytes, so replay is safe, but carrying its account identity into the next
-        // account would not be. Let the rotated adapter derive a fresh identity and conversation.
-        parsed._cursorIdentityScope = undefined;
-        parsed._cursorConversationId = undefined;
-        if (parsed._providerContinuation?.cursor) {
-          const { cursor: _discardedCursor, ...otherProviderState } = parsed._providerContinuation;
-          parsed._providerContinuation = otherProviderState;
-        }
         const rotatedProvider = resolveWireProtocolOverride(
           route.providerName,
           route.modelId,
@@ -5064,7 +5103,7 @@ async function handleResponsesInner(
           route.providerName,
           { ...route.provider, apiKey: refreshed.accessToken },
           parsed.options.promptCacheKey,
-          route.providerName === "github-copilot" ? getOAuthCredentialApiBaseUrl(route.providerName) : undefined,
+          route.providerName === "github-copilot" ? copilotOriginForRefreshedCredential(refreshed) : undefined,
         );
         route.provider = refreshedProvider;
         invalidateSameTargetRequest();
@@ -5226,6 +5265,19 @@ async function handleResponsesInner(
             resolveWireProtocolOverride(route.providerName, route.modelId, route.provider, inboundWire),
             config.cacheRetention,
           );
+          // The other two rotation sites rebind the reasoning replay scope here and this one did
+          // not, so continuation state minted under the previous account stayed addressable after
+          // the swap. Sealing the attempt identity records what happened; it does not re-scope the
+          // replay cache.
+          bindRouteReasoningReplayScope({
+            parsed,
+            providerName: route.providerName,
+            provider: route.provider,
+            adapterName: activeAdapter.name,
+            oauthCredentialSnapshot: replayOAuthCredentialSnapshot,
+            codexAuthContext: authCtx,
+            forwardHeaders: selectedForwardHeaders,
+          });
           sealRequestAttemptIdentity(logCtx.activeAttempt, logCtx.provider, activeAdapter.name, logCtx.accountLogLabel);
           const result = await rebuildAndRefetch("oauth-account-429");
           if ("failed" in result) return result.failed;

tests/generic-oauth-failover.test.ts

@@ -12,6 +12,7 @@ import {
   rotateGenericOAuthAccountOn429,
 } from "../src/oauth/generic-account-failover";
 import { getAccountSet, markAccountNeedsReauth, saveCredential } from "../src/oauth/store";
+import { GITHUB_COPILOT_DEFAULT_API_BASE, validateCopilotApiBaseUrl } from "../src/oauth/github-copilot";
 import type { OcxConfig, OcxProviderConfig } from "../src/types";
 
 const originalHome = process.env.OPENCODEX_HOME;
@@ -254,4 +255,108 @@ describe("sidecar on429 wiring", () => {
     // Kiro routing metadata still travels with its own token.
     expect(body).toContain("_kiroAuthContext");
   });
+
+  test("the helper rotates the credential IDENTITY, not just the credential", () => {
+    // Rotating the bearer without rotating the identity of that bearer leaves the two naming
+    // different accounts, and both consumers of that identity then misbehave:
+    //
+    //   sentOAuthSnapshot          feeds forceRefreshOAuthAccessSnapshot, which refreshes the
+    //                              SNAPSHOT's account - so a stale value renews the account the
+    //                              request just rotated AWAY from while transport points at the
+    //                              rotated one.
+    //   replayOAuthCredentialSnapshot  scopes the reasoning replay cache - so a stale value lets
+    //                              continuation state minted under one account be replayed under
+    //                              another.
+    //
+    // Asserted INSIDE the helper rather than anywhere in the file: three rotation sites already
+    // disagreed about which of these to update, and only runTurn got it right, by hand.
+    const start = coreSource.indexOf("const applyFailoverSnapshot =");
+    expect(start).toBeGreaterThan(-1);
+    const body = coreSource.slice(start, coreSource.indexOf("\n  };", start));
+    expect(body).toContain("sentOAuthSnapshot = snapshot");
+    expect(body).toContain("accountId: snapshot.accountId");
+    expect(body).toContain("generation: snapshot.generation");
+
+    // A Cursor conversation and identity scope are credential-scoped the same way, and were also
+    // cleared at only one of the three sites. For image turns src/images/loop.ts copies the
+    // conversation id back onto the outer request, which is how it survives a rotation.
+    expect(body).toContain("parsed._cursorIdentityScope = undefined");
+    expect(body).toContain("parsed._cursorConversationId = undefined");
+
+    // And the clear must exist ONLY in the helper: a per-site copy is how these drifted apart.
+    const scopeClears = coreSource.match(/parsed\._cursorIdentityScope = undefined/g) ?? [];
+    expect(scopeClears.length).toBe(1);
+  });
+
+  test("every generic-OAuth rotation site re-scopes reasoning replay to the rotated account", () => {
+    // The three generic-OAuth 429 sites diverged: runTurn rebound the replay scope, the sidecar
+    // bound WITHOUT the credential snapshot (which fails the OAuth replay key closed and so
+    // carries no account identity at all), and the HTTP site did not rebind at all. Sealing the
+    // attempt identity is not a substitute - it records what happened, it does not re-scope the
+    // cache.
+    //
+    // Scoped to the generic-OAuth sites deliberately. Key-pool and terminal-guard rotations swap
+    // an API KEY, which carries no OAuth account identity, so a snapshot there would be
+    // meaningless.
+    //
+    // The window is bounded by the rotation's OWN recovery call rather than searching forward
+    // indefinitely: an unbounded search finds the NEXT site's bind and passes even when this
+    // site has none. That weaker form was written first and it survived deleting the bind.
+    const marker = "applyFailoverSnapshot(snapshot)";
+    let cursor = 0;
+    const windows: string[] = [];
+    for (;;) {
+      const at = coreSource.indexOf(marker, cursor);
+      if (at === -1) break;
+      cursor = at + marker.length;
+      // Each site ends at the call that actually reissues the request under the new account.
+      const rest = coreSource.slice(cursor);
+      const ends = [
+        rest.indexOf("rebuildAndRefetch("),
+        rest.indexOf("return rotatedAdapter;"),
+        rest.indexOf("return true;"),
+      ].filter(index => index > -1);
+      windows.push(rest.slice(0, Math.min(...ends)));
+    }
+    expect(windows.length).toBe(3);
+
+    for (const [index, window] of windows.entries()) {
+      expect(`site${index}:${window.includes("bindRouteReasoningReplayScope({")}`).toBe(`site${index}:true`);
+      expect(`site${index}:${window.includes("oauthCredentialSnapshot")}`).toBe(`site${index}:true`);
+    }
+  });
+  test("a rotated-to account never borrows another account's Copilot origin", () => {
+    // The bug this replaces a source-text assertion for: the old fallback was
+    // `refreshed.apiBaseUrl ?? getOAuthCredentialApiBaseUrl(provider)`, whose second arm reads
+    // the ACTIVE credential. A generic 429 rotation does not promote the account it rotated to,
+    // so a legacy account B with no allowlisted origin was rebuilt as B's bearer paired with A's
+    // origin — one account's token sent to another account's host.
+    //
+    // Asserting on source text could not catch that: it passes whether the assignment is
+    // reachable or not, and whether the resulting pair is correct or not. Drive the resolver.
+    const resolve = (refreshed: { apiBaseUrl?: string }) =>
+      validateCopilotApiBaseUrl(refreshed.apiBaseUrl) ?? GITHUB_COPILOT_DEFAULT_API_BASE;
+
+    // B carries its own allowlisted origin: use it.
+    expect(resolve({ apiBaseUrl: "https://proxy.githubcopilot.com" }))
+      .toBe("https://proxy.githubcopilot.com");
+
+    // B is a legacy credential with NO origin. Fail closed to canonical — never account A's.
+    expect(resolve({})).toBe(GITHUB_COPILOT_DEFAULT_API_BASE);
+
+    // B carries a non-allowlisted origin: reject it rather than honour it.
+    expect(resolve({ apiBaseUrl: "https://evil.example.com" }))
+      .toBe(GITHUB_COPILOT_DEFAULT_API_BASE);
+    expect(resolve({ apiBaseUrl: "" })).toBe(GITHUB_COPILOT_DEFAULT_API_BASE);
+  });
+
+  test("the account-blind Copilot fallback is gone from both 401 rebuild sites", () => {
+    // Topology guard only — the behavioural contract is the test above. Kept because the defect
+    // was a single `??` whose right-hand side silently reached a different account.
+    // Strip comments first: the helper's own doc comment quotes the removed expression to
+    // explain why it was wrong, and a naive scan would read that explanation as the defect.
+    const codeOnly = coreSource.replace(/\/\*[\s\S]*?\*\//g, "").replace(/^\s*\/\/.*$/gm, "");
+    expect(codeOnly.match(/refreshed\.apiBaseUrl \?\? getOAuthCredentialApiBaseUrl/g)).toBeNull();
+    expect((coreSource.match(/copilotOriginForRefreshedCredential\(refreshed\)/g) ?? []).length).toBe(2);
+  });
 });