fix(github): key the response cache by installation identity, not the raw token

src/github/client.ts

@@ -60,7 +60,9 @@ type EnvLookup = Record<string, string | undefined>;
 export type GitHubTimeoutFetchInit = RequestInit & {
   /** Opt in to using this response's REST bucket headers for self-host queue admission control. */
   githubRateLimitAdmission?: boolean;
-  /** Stable actor key for admission control. Installation-token reads should use the installation id. */
+  /** Stable actor key for admission control AND (independent of githubRateLimitAdmission) the response-cache key —
+   *  present whenever it, so a cacheable GET's cache key stays stable across token rotation instead of being
+   *  derived from the raw token. Installation-token reads should use the installation id. */
   githubRateLimitAdmissionKey?: string;
   /** Consulted ONLY when this GET is about to make a NETWORK read (a cache hit is always served first, for free).
    *  Return true to skip the network read — timeoutFetch then resolves to a synthetic 503 so a best-effort caller
@@ -236,11 +238,19 @@ async function sha256Short(value: string): Promise<string> {
   return Array.from(new Uint8Array(digest), (byte) => byte.toString(16).padStart(2, "0")).join("").slice(0, 16);
 }
 
-async function responseCacheKey(url: string, headers: Headers): Promise<string> {
-  const authHash = await sha256Short(headers.get("authorization") || "");
+// Prefer the SAME stable per-installation/public-token identity already used for rate-limit admission scoping
+// (githubRateLimitAdmissionKeyForToken) over hashing the raw Authorization header. An installation token rotates
+// roughly hourly by design (plus on auth failure, plus on every redeploy if not persisted); hashing it means EVERY
+// rotation invalidates the ENTIRE cached-response namespace for that installation across all cache classes at once
+// — not just entries that are actually stale — even though every entry is still within its own TTL. Keying by the
+// stable identity instead means a token rotation no longer touches the cache at all. A caller that doesn't thread
+// an admission key falls back to the previous token-hash behavior: still correctly isolated, just without the
+// cross-rotation benefit (mirrors the App-JWT-reuse fix for the same class of problem, #1940). (#2538)
+async function responseCacheKey(url: string, headers: Headers, admissionKey: GitHubRateLimitAdmissionKey | null): Promise<string> {
+  const authIdentity = admissionKey ? `key:${admissionKey}` : `auth:${await sha256Short(headers.get("authorization") || "")}`;
   const accept = encodeURIComponent(headers.get("accept") || "");
   const apiVersion = encodeURIComponent(headers.get("x-github-api-version") || "");
-  return `v2:${authHash}:${accept}:${apiVersion}:${url}`;
+  return `v3:${authIdentity}:${accept}:${apiVersion}:${url}`;
 }
 
 type VolatileSingleFlightScope = { requestKey: string; authorization: string };
@@ -286,6 +296,14 @@ function rateLimitAdmissionKey(init: GitHubTimeoutFetchInit | undefined): GitHub
   return key ? key : null;
 }
 
+// Deliberately NOT gated on githubRateLimitAdmission (unlike rateLimitAdmissionKey above): a caller may know its
+// stable identity and want it used for cache keying without opting into local rate-limit observation for this
+// particular call. A blank/whitespace-only key is treated as absent, same as rateLimitAdmissionKey. (#2538)
+function cacheKeyAdmissionIdentity(init: GitHubTimeoutFetchInit | undefined): GitHubRateLimitAdmissionKey | null {
+  const key = init?.githubRateLimitAdmissionKey?.trim();
+  return key ? key : null;
+}
+
 function requestInitForFetch(init: GitHubTimeoutFetchInit | undefined): RequestInit | undefined {
   if (!init || (!("githubRateLimitAdmission" in init) && !("githubRateLimitAdmissionKey" in init))) return init;
   const { githubRateLimitAdmission: _omitted, githubRateLimitAdmissionKey: _omittedKey, ...rest } = init;
@@ -493,7 +511,7 @@ export async function timeoutFetch(input: RequestInfo | URL, init?: GitHubTimeou
     return fetchWithGitHubRetry(input, init);
   }
 
-  const cacheKey = await responseCacheKey(url, headers);
+  const cacheKey = await responseCacheKey(url, headers, cacheKeyAdmissionIdentity(init));
   let hit: CachedGitHubResponse | null = null;
   try {
     hit = await responseCache!.get(cacheKey);

src/upstream/commit.ts

@@ -1,4 +1,4 @@
-import { timeoutFetch } from "../github/client";
+import { githubRateLimitAdmissionKeyForPublicToken, timeoutFetch } from "../github/client";
 import { shouldWaitForGitHubRateLimit } from "../github/rate-limit";
 
 function upstreamCommitHeaders(token: string | undefined): Record<string, string> {
@@ -38,6 +38,9 @@ export async function resolveUpstreamCommitSha(
         // read is skipped (→ synthetic non-OK → null → caller falls back to the mutable ref) when the REST budget is
         // at/below the low-water floor. This callback runs ONLY on a cache miss, so it never suppresses a cache hit.
         githubSkipNetworkWhen: () => shouldWaitForGitHubRateLimit(env).then(Boolean),
+        // Give this bare-commit read the same stable, distinctly-scoped cache-key identity every installation-token
+        // read gets, so the shared public token rotating doesn't reset this cacheable "commit" class either (#2538).
+        githubRateLimitAdmissionKey: githubRateLimitAdmissionKeyForPublicToken(),
       },
     );
     if (!response.ok) return null;

test/unit/github-client.test.ts

@@ -443,6 +443,108 @@ describe("timeoutFetch", () => {
     expect([...store.keys()].filter((key) => key.includes(url))).toHaveLength(2);
   });
 
+  it("keys a cacheable GET by the stable rate-limit admission identity, surviving a token rotation (#2538)", async () => {
+    const store = installMemoryResponseCache();
+    let getFetches = 0;
+    vi.stubGlobal("fetch", async () => {
+      getFetches += 1;
+      return Response.json({ fetches: getFetches });
+    });
+
+    const url = "https://api.github.com/repos/o/r";
+    const admissionKey = githubRateLimitAdmissionKeyForInstallation(42);
+    const beforeRotation = await timeoutFetch(url, {
+      headers: { authorization: "Bearer old-token", accept: "application/vnd.github+json" },
+      githubRateLimitAdmissionKey: admissionKey,
+    });
+    // A minted replacement installation token for the SAME installation -- rotation must not evict the entry
+    // beforeRotation just set, since it's still within its own TTL.
+    const afterRotation = await timeoutFetch(url, {
+      headers: { authorization: "Bearer new-token-after-rotation", accept: "application/vnd.github+json" },
+      githubRateLimitAdmissionKey: admissionKey,
+    });
+
+    expect(await beforeRotation.json()).toEqual({ fetches: 1 });
+    expect(await afterRotation.json()).toEqual({ fetches: 1 });
+    expect(afterRotation.headers.get(GITHUB_RESPONSE_CACHE_REPLAY_HEADER)).toBe("hit");
+    expect(getFetches).toBe(1);
+    expect([...store.keys()].filter((key) => key.includes(url))).toHaveLength(1);
+    expect([...store.keys()].some((key) => key.includes("old-token") || key.includes("new-token-after-rotation"))).toBe(false);
+  });
+
+  it("keeps two installations' admission-keyed cache entries isolated even if they briefly share raw token bytes (#2538)", async () => {
+    const store = installMemoryResponseCache();
+    let getFetches = 0;
+    vi.stubGlobal("fetch", async () => {
+      getFetches += 1;
+      return Response.json({ fetches: getFetches });
+    });
+
+    const url = "https://api.github.com/repos/o/r";
+    const first = await timeoutFetch(url, {
+      headers: { authorization: "Bearer shared-token", accept: "application/vnd.github+json" },
+      githubRateLimitAdmissionKey: githubRateLimitAdmissionKeyForInstallation(1),
+    });
+    const second = await timeoutFetch(url, {
+      headers: { authorization: "Bearer shared-token", accept: "application/vnd.github+json" },
+      githubRateLimitAdmissionKey: githubRateLimitAdmissionKeyForInstallation(2),
+    });
+
+    expect(await first.json()).toEqual({ fetches: 1 });
+    expect(await second.json()).toEqual({ fetches: 2 });
+    expect(getFetches).toBe(2);
+    expect([...store.keys()].filter((key) => key.includes(url))).toHaveLength(2);
+  });
+
+  it("scopes the public-token admission key distinctly from an installation-scoped key for the same URL (#2538)", async () => {
+    installMemoryResponseCache();
+    let getFetches = 0;
+    vi.stubGlobal("fetch", async () => {
+      getFetches += 1;
+      return Response.json({ fetches: getFetches });
+    });
+
+    // The bare-commit "commit" cache class -- the exact shape resolveUpstreamCommitSha reads with the shared
+    // public token, distinct from an installation-token read of the same URL.
+    const url = "https://api.github.com/repos/o/r/commits/main";
+    const viaInstallation = await timeoutFetch(url, {
+      headers: { authorization: "Bearer tok", accept: "application/vnd.github+json" },
+      githubRateLimitAdmissionKey: githubRateLimitAdmissionKeyForInstallation(7),
+    });
+    const viaPublicToken = await timeoutFetch(url, {
+      headers: { authorization: "Bearer tok", accept: "application/vnd.github+json" },
+      githubRateLimitAdmissionKey: githubRateLimitAdmissionKeyForPublicToken(),
+    });
+
+    expect(await viaInstallation.json()).toEqual({ fetches: 1 });
+    expect(await viaPublicToken.json()).toEqual({ fetches: 2 });
+    expect(getFetches).toBe(2);
+  });
+
+  it("treats a blank admission key as absent, keeping the auth-hash fallback for keying (#2538)", async () => {
+    installMemoryResponseCache();
+    let getFetches = 0;
+    vi.stubGlobal("fetch", async () => {
+      getFetches += 1;
+      return Response.json({ fetches: getFetches });
+    });
+
+    const url = "https://api.github.com/repos/o/r";
+    const first = await timeoutFetch(url, {
+      headers: { authorization: "Bearer token-a", accept: "application/vnd.github+json" },
+      githubRateLimitAdmissionKey: "   ",
+    });
+    const second = await timeoutFetch(url, {
+      headers: { authorization: "Bearer token-b", accept: "application/vnd.github+json" },
+      githubRateLimitAdmissionKey: "   ",
+    });
+
+    expect(await first.json()).toEqual({ fetches: 1 });
+    // A different token with only a blank admission key falls back to auth-hash keying -- still a MISS.
+    expect(await second.json()).toEqual({ fetches: 2 });
+    expect(getFetches).toBe(2);
+  });
+
   it("replays pagination and validator headers while dropping rate-limit headers", async () => {
     installMemoryResponseCache();
     let getFetches = 0;

test/unit/upstream-commit.test.ts

@@ -81,6 +81,23 @@ describe("resolveUpstreamCommitSha — one shared, cached, budget-gated upstream
     expect(fetchSpy).not.toHaveBeenCalled();
   });
 
+  it("keys its cache entry by the stable public-token admission identity, surviving a GITHUB_PUBLIC_TOKEN change (#2538)", async () => {
+    rateLimitMock.shouldWaitForGitHubRateLimit.mockResolvedValue(undefined);
+    const store = new Map<string, CachedGitHubResponse>();
+    setGitHubResponseCache({ get: async (k) => store.get(k) ?? null, set: async (k, v) => void store.set(k, v) });
+    let fetches = 0;
+    vi.stubGlobal("fetch", async () => {
+      fetches += 1;
+      return Response.json({ sha: "stable-across-rotation" });
+    });
+    expect(await resolveUpstreamCommitSha(env, config)).toBe("stable-across-rotation");
+    // Simulate an operator rotating the shared public token (e.g. a redeploy with a new value) -- the entry the
+    // first call just set is still within its TTL and must still be served, not re-fetched.
+    const rotatedEnv = { GITHUB_PUBLIC_TOKEN: "pub-tok-rotated" } as unknown as Env;
+    expect(await resolveUpstreamCommitSha(rotatedEnv, config)).toBe("stable-across-rotation");
+    expect(fetches).toBe(1);
+  });
+
   it("fails open (null) on a non-OK status, a missing/empty sha, or a thrown fetch", async () => {
     rateLimitMock.shouldWaitForGitHubRateLimit.mockResolvedValue(undefined);
     vi.stubGlobal("fetch", async () => new Response("nope", { status: 404 }));