refactor(github): consolidate githubHeaders cluster and CI-cache params (#4610)

src/github/app.ts

@@ -7,6 +7,7 @@ import { recordGitHubRateLimitObservation, updateInstallationPermissions } from
 import { recordClockSkewFromResponse } from "../selfhost/clock-skew";
 import {
   clearGitHubResponseCacheForTest,
+  githubHeaders,
   githubRateLimitAdmissionKeyForInstallation,
   makeInstallationOctokit,
   timeoutFetch,
@@ -215,7 +216,7 @@ async function requestInstallationTokenWithJwt(
     `https://api.github.com/app/installations/${installationId}/access_tokens`,
     {
       method: "POST",
-      headers: githubHeaders(`Bearer ${jwt}`),
+      headers: githubHeaders({ token: jwt, json: true }),
     },
   );
   recordClockSkewFromResponse(response);
@@ -381,7 +382,7 @@ export async function getAppInstallation(
   // cache must stay keyed off the Authorization header (the no-admission-key fallback) to preserve per-App-identity
   // isolation (#1940). recordGitHubRateLimitObservation below records the SAME admissionKey directly instead.
   const response = await timeoutFetch(`https://api.github.com${path}`, {
-    headers: githubHeaders(`Bearer ${jwt}`),
+    headers: githubHeaders({ token: jwt, json: true }),
   });
   // #4506: refreshInstallationHealthRecords's per-installation loop (backfill.ts) makes one of these calls per
   // installation, but this call lives outside backfill.ts's module boundary -- and backfill.ts already imports
@@ -441,7 +442,7 @@ export async function getRepositoryCollaboratorPermission(
   const response = await timeoutFetch(
     `https://api.github.com/repos/${encodeURIComponent(owner)}/${encodeURIComponent(name)}/collaborators/${encodeURIComponent(login)}/permission`,
     {
-      headers: githubHeaders(`Bearer ${token}`),
+      headers: githubHeaders({ token, json: true }),
       githubRateLimitAdmission: true,
       githubRateLimitAdmissionKey: githubRateLimitAdmissionKeyForInstallation(installationId),
     },
@@ -475,7 +476,7 @@ export async function getGithubUserCreatedAt(
     const response = await timeoutFetch(
       `https://api.github.com/users/${encodeURIComponent(login)}`,
       {
-        headers: githubHeaders(`Bearer ${token}`),
+        headers: githubHeaders({ token, json: true }),
         githubRateLimitAdmission: true,
         githubRateLimitAdmissionKey: githubRateLimitAdmissionKeyForInstallation(installationId),
       },
@@ -615,7 +616,7 @@ export async function cancelInFlightWorkflowRunsForHeadSha(
   try {
     const token = await createInstallationToken(env, installationId);
     const fetchOptions: ActionsRunFetchOptions = {
-      headers: githubHeaders(`Bearer ${token}`),
+      headers: githubHeaders({ token, json: true }),
       githubRateLimitAdmission: true,
       githubRateLimitAdmissionKey: githubRateLimitAdmissionKeyForInstallation(installationId),
     };
@@ -1170,13 +1171,3 @@ export function getInstallationId(
 ): number | null {
   return payload.installation?.id ?? null;
 }
-
-function githubHeaders(authorization: string): HeadersInit {
-  return {
-    accept: "application/vnd.github+json",
-    authorization,
-    "content-type": "application/json",
-    "user-agent": "gittensory/0.1",
-    "x-github-api-version": "2022-11-28",
-  };
-}

src/github/client.ts

@@ -29,6 +29,32 @@ const DEFAULT_METADATA_TTL_SECONDS = 10 * 60;
 const DEFAULT_COMMIT_TTL_SECONDS = 15 * 60;
 export const GITHUB_RESPONSE_CACHE_REPLAY_HEADER = "x-gittensory-cache";
 
+/** The single shared GitHub REST header-builder for every raw-`fetch`/`timeoutFetch` call in `src/` (Octokit
+ *  calls set their own headers internally and don't need this). Consolidates four independent, drifted
+ *  `githubHeaders` copies that had each grown a different signature (#4610). `token` is optional — a public,
+ *  unauthenticated read omits it rather than sending an empty `authorization`. */
+export interface GitHubHeadersOptions {
+  /** Bearer token. Omitted (or empty) → no `authorization` header. */
+  token?: string | undefined;
+  /** `accept` header value. Defaults to the standard REST JSON media type. */
+  accept?: string | undefined;
+  /** Include `content-type: application/json`, for a request with a JSON body. Defaults to false. */
+  json?: boolean | undefined;
+  /** Include the pinned `x-github-api-version` header. Defaults to true. */
+  apiVersion?: boolean | undefined;
+}
+
+export function githubHeaders(options: GitHubHeadersOptions = {}): Record<string, string> {
+  const { token, accept = "application/vnd.github+json", json = false, apiVersion = true } = options;
+  return {
+    accept,
+    "user-agent": "gittensory/0.1",
+    ...(apiVersion ? { "x-github-api-version": "2022-11-28" } : {}),
+    ...(json ? { "content-type": "application/json" } : {}),
+    ...(token ? { authorization: `Bearer ${token}` } : {}),
+  };
+}
+
 /** A shared cache for safe GitHub GET responses (e.g. Redis on the self-host). Stores only status/body/
  *  content-type plus pagination/validator headers — never rate-limit or encoding headers. Set on the self-host;
  *  the Worker leaves it null. */

src/queue/processors.ts

@@ -792,124 +792,140 @@ function evictLiveFactOnReject<T>(
  */
 async function cachedFetchLiveCiAggregate(
   env: Env,
-  repoFullName: string,
-  prNumber: number,
-  headSha: string | null | undefined,
-  token: string | undefined,
-  requiredContexts: ReadonlySet<string> | null | undefined,
-  requiredContextsKey: string,
-  forceRefresh: boolean,
-  // False when the caller's own required-context lookup FAILED (not merely resolved to "none configured") --
-  // that fail-open aggregate must never be persisted under the normal key, or a transient lookup error would
-  // mask the repo's real required-context state for every other reader until the entry's TTL expires (#selfhost-
-  // ci-verification gate review finding). The live-fetched aggregate is still returned to THIS caller either way.
-  requiredContextsResolved: boolean,
-  admissionKey?: GitHubRateLimitAdmissionKey,
+  args: {
+    repoFullName: string;
+    prNumber: number;
+    headSha: string | null | undefined;
+    token: string | undefined;
+    requiredContexts: ReadonlySet<string> | null | undefined;
+    requiredContextsKey: string;
+    forceRefresh: boolean;
+    // False when the caller's own required-context lookup FAILED (not merely resolved to "none configured") --
+    // that fail-open aggregate must never be persisted under the normal key, or a transient lookup error would
+    // mask the repo's real required-context state for every other reader until the entry's TTL expires (#selfhost-
+    // ci-verification gate review finding). The live-fetched aggregate is still returned to THIS caller either way.
+    requiredContextsResolved: boolean;
+    admissionKey?: GitHubRateLimitAdmissionKey | undefined;
+  },
 ): Promise<LiveCiAggregate> {
-  const cached = await getPullRequestDetailSyncState(env, repoFullName, prNumber).catch(() => null);
-  if (!forceRefresh && cached && isCiStateCacheFresh(cached, headSha, requiredContextsKey)) {
+  const cached = await getPullRequestDetailSyncState(env, args.repoFullName, args.prNumber).catch(() => null);
+  if (!args.forceRefresh && cached && isCiStateCacheFresh(cached, args.headSha, args.requiredContextsKey)) {
     const deserialized = deserializeCachedCiAggregate(cached);
     if (deserialized) {
       incr(CI_STATE_CACHE_METRIC, { field: "aggregate", result: "hit" });
       return deserialized;
     }
   }
-  incr(CI_STATE_CACHE_METRIC, { field: "aggregate", result: forceRefresh ? "forced" : "miss" });
-  const live = await fetchLiveCiAggregatePreferGraphQl(env, repoFullName, headSha, token, requiredContexts, admissionKey);
-  if (requiredContextsResolved) {
-    await writeThroughCiStateCache(env, repoFullName, prNumber, cached, headSha, requiredContextsKey, live);
+  incr(CI_STATE_CACHE_METRIC, { field: "aggregate", result: args.forceRefresh ? "forced" : "miss" });
+  const live = await fetchLiveCiAggregatePreferGraphQl(env, args.repoFullName, args.headSha, args.token, args.requiredContexts, args.admissionKey);
+  if (args.requiredContextsResolved) {
+    await writeThroughCiStateCache(env, args.repoFullName, args.prNumber, cached, args.headSha, args.requiredContextsKey, live);
   }
   return live;
 }
 
 function fetchLiveCiAggregateWithRequiredContexts(
   env: Env,
-  repoFullName: string,
-  facts: LiveGithubFacts,
-  prNumber: number,
-  headSha: string | null | undefined,
-  baseRef: string | null | undefined,
-  token: string | undefined,
-  expectedCiContexts: ReadonlyArray<string> | null | undefined,
-  forceRefresh: boolean,
-  admissionKey?: GitHubRateLimitAdmissionKey,
+  args: {
+    repoFullName: string;
+    facts: LiveGithubFacts;
+    prNumber: number;
+    headSha: string | null | undefined;
+    baseRef: string | null | undefined;
+    token: string | undefined;
+    expectedCiContexts: ReadonlyArray<string> | null | undefined;
+    forceRefresh: boolean;
+    admissionKey?: GitHubRateLimitAdmissionKey | undefined;
+  },
 ): Promise<LiveCiAggregate> {
   // CI refresh callers need fresh check/status state; branch protection contexts move slowly enough to stay
   // request-cached. When the #1941 flag is on, fetchLiveCiAggregatePreferGraphQl collapses the check/status reads
   // into one GraphQL rollup (reusing these requiredContexts), else it uses the proven REST aggregate.
   // cachedFetchLiveCiAggregate (#selfhost-ci-verification) is the durable, cross-job snapshot cache sibling to
   // this request-scoped LiveGithubFacts memo -- it is only ever consulted here, on a LiveGithubFacts miss.
-  return cachedRequiredStatusContexts(env, repoFullName, facts, baseRef, token, expectedCiContexts, admissionKey)
+  return cachedRequiredStatusContexts(env, args.repoFullName, args.facts, args.baseRef, args.token, args.expectedCiContexts, args.admissionKey)
     .catch(() => ({ requiredContexts: null, resolved: false }))
     .then(({ requiredContexts, resolved }) =>
-      cachedFetchLiveCiAggregate(env, repoFullName, prNumber, headSha, token, requiredContexts, resolvedRequiredContextsKeyPart(requiredContexts), forceRefresh, resolved, admissionKey),
+      cachedFetchLiveCiAggregate(env, {
+        repoFullName: args.repoFullName,
+        prNumber: args.prNumber,
+        headSha: args.headSha,
+        token: args.token,
+        requiredContexts,
+        requiredContextsKey: resolvedRequiredContextsKeyPart(requiredContexts),
+        forceRefresh: args.forceRefresh,
+        requiredContextsResolved: resolved,
+        admissionKey: args.admissionKey,
+      }),
     );
 }
 
 function cachedLiveCiAggregate(
   env: Env,
-  repoFullName: string,
-  facts: LiveGithubFacts,
-  prNumber: number,
-  headSha: string | null | undefined,
-  baseRef: string | null | undefined,
-  token: string | undefined,
-  expectedCiContexts: ReadonlyArray<string> | null | undefined,
-  admissionKey?: GitHubRateLimitAdmissionKey,
+  args: {
+    repoFullName: string;
+    facts: LiveGithubFacts;
+    prNumber: number;
+    headSha: string | null | undefined;
+    baseRef: string | null | undefined;
+    token: string | undefined;
+    expectedCiContexts: ReadonlyArray<string> | null | undefined;
+    admissionKey?: GitHubRateLimitAdmissionKey | undefined;
+  },
 ): Promise<LiveCiAggregate> {
-  const key = liveFactKey(repoFullName, headSha, baseRef, liveFactTokenPart(token), expectedCiContextsKeyPart(expectedCiContexts));
-  const cached = facts.ciAggregates.get(key);
+  const key = liveFactKey(args.repoFullName, args.headSha, args.baseRef, liveFactTokenPart(args.token), expectedCiContextsKeyPart(args.expectedCiContexts));
+  const cached = args.facts.ciAggregates.get(key);
   if (cached) return cached;
   const next = evictLiveFactOnReject(
-    facts.ciAggregates,
+    args.facts.ciAggregates,
     key,
-    fetchLiveCiAggregateWithRequiredContexts(
-      env,
-      repoFullName,
-      facts,
-      prNumber,
-      headSha,
-      baseRef,
-      token,
-      expectedCiContexts,
-      false,
-      admissionKey,
-    ),
+    fetchLiveCiAggregateWithRequiredContexts(env, {
+      repoFullName: args.repoFullName,
+      facts: args.facts,
+      prNumber: args.prNumber,
+      headSha: args.headSha,
+      baseRef: args.baseRef,
+      token: args.token,
+      expectedCiContexts: args.expectedCiContexts,
+      forceRefresh: false,
+      admissionKey: args.admissionKey,
+    }),
   );
-  facts.ciAggregates.set(key, next);
+  args.facts.ciAggregates.set(key, next);
   return next;
 }
 
 function refreshLiveCiAggregate(
   env: Env,
-  repoFullName: string,
-  facts: LiveGithubFacts,
-  prNumber: number,
-  headSha: string | null | undefined,
-  baseRef: string | null | undefined,
-  token: string | undefined,
-  expectedCiContexts: ReadonlyArray<string> | null | undefined,
-  admissionKey?: GitHubRateLimitAdmissionKey,
+  args: {
+    repoFullName: string;
+    facts: LiveGithubFacts;
+    prNumber: number;
+    headSha: string | null | undefined;
+    baseRef: string | null | undefined;
+    token: string | undefined;
+    expectedCiContexts: ReadonlyArray<string> | null | undefined;
+    admissionKey?: GitHubRateLimitAdmissionKey | undefined;
+  },
 ): Promise<LiveCiAggregate> {
-  const key = liveFactKey(repoFullName, headSha, baseRef, liveFactTokenPart(token), expectedCiContextsKeyPart(expectedCiContexts));
+  const key = liveFactKey(args.repoFullName, args.headSha, args.baseRef, liveFactTokenPart(args.token), expectedCiContextsKeyPart(args.expectedCiContexts));
   const next = evictLiveFactOnReject(
-    facts.ciAggregates,
+    args.facts.ciAggregates,
     key,
-    fetchLiveCiAggregateWithRequiredContexts(
-      env,
-      repoFullName,
-      facts,
-      prNumber,
-      headSha,
-      baseRef,
-      token,
-      expectedCiContexts,
-      true,
-      admissionKey,
-    ),
+    fetchLiveCiAggregateWithRequiredContexts(env, {
+      repoFullName: args.repoFullName,
+      facts: args.facts,
+      prNumber: args.prNumber,
+      headSha: args.headSha,
+      baseRef: args.baseRef,
+      token: args.token,
+      expectedCiContexts: args.expectedCiContexts,
+      forceRefresh: true,
+      admissionKey: args.admissionKey,
+    }),
   );
-  facts.ciAggregates.set(key, next);
-  facts.forcedCiAggregateKeys.add(key);
+  args.facts.ciAggregates.set(key, next);
+  args.facts.forcedCiAggregateKeys.add(key);
   return next;
 }
 
@@ -1001,7 +1017,7 @@ function reuseOrRefreshLiveCiAggregate(
   const key = liveFactKey(repoFullName, headSha, baseRef, liveFactTokenPart(token), expectedCiContextsKeyPart(expectedCiContexts));
   const cached = facts.forcedCiAggregateKeys.has(key) ? facts.ciAggregates.get(key) : undefined;
   if (cached) return cached;
-  return refreshLiveCiAggregate(env, repoFullName, facts, prNumber, headSha, baseRef, token, expectedCiContexts, admissionKey);
+  return refreshLiveCiAggregate(env, { repoFullName, facts, prNumber, headSha, baseRef, token, expectedCiContexts, admissionKey });
 }
 
 /**
@@ -3819,7 +3835,16 @@ async function prReadyForReview(
   }
   // 2) wait for CI to finish before running the Gittensory review. Required contexts still define which failures
   // block/close, but hasPending tracks any visible non-bot CI that is not settled yet.
-  const ci = await cachedLiveCiAggregate(env, repoFullName, liveFacts, pr.number, pr.headSha, pr.baseRef, token, settings.expectedCiContexts, admissionKey).catch(() => undefined);
+  const ci = await cachedLiveCiAggregate(env, {
+    repoFullName,
+    facts: liveFacts,
+    prNumber: pr.number,
+    headSha: pr.headSha,
+    baseRef: pr.baseRef,
+    token,
+    expectedCiContexts: settings.expectedCiContexts,
+    admissionKey,
+  }).catch(() => undefined);
   if (ci?.hasPending) {
     // Staleness cap: inferred or unreadable pending CI can otherwise defer FOREVER (orphaned required context,
     // transiently unreadable pages, fork check that never reports). Past the cap we stop deferring and let the
@@ -8761,17 +8786,16 @@ async function resolveManifestPassedValidationCount(
   // fetchLiveCiAggregatePreferGraphQl, and the durable-cache read/write) is already fail-open at every
   // internal step (see their own doc comments), so it never rejects -- an extra catch here would just be
   // dead, uncoverable code.
-  const liveCi = await cachedLiveCiAggregate(
-    env,
-    args.repoFullName,
-    args.liveFacts,
-    args.prNumber,
-    args.headSha,
-    args.baseRef,
+  const liveCi = await cachedLiveCiAggregate(env, {
+    repoFullName: args.repoFullName,
+    facts: args.liveFacts,
+    prNumber: args.prNumber,
+    headSha: args.headSha,
+    baseRef: args.baseRef,
     token,
-    args.expectedCiContexts,
+    expectedCiContexts: args.expectedCiContexts,
     admissionKey,
-  );
+  });
   return liveCi.ciState === "passed" ? 1 : 0;
 }
 
@@ -11100,7 +11124,16 @@ async function maybePublishPrPublicSurface(
       const baseRef = pr.baseRef ?? repo?.defaultBranch;
       // Required contexts still detect missing/pending required CI, but every visible completed red check/status is
       // adverse and blocks the PR.
-      const liveCi = await refreshLiveCiAggregate(env, repoFullName, webhook.liveFacts, pr.number, pr.headSha, baseRef, token, settings.expectedCiContexts, admissionKey);
+      const liveCi = await refreshLiveCiAggregate(env, {
+        repoFullName,
+        facts: webhook.liveFacts,
+        prNumber: pr.number,
+        headSha: pr.headSha,
+        baseRef,
+        token,
+        expectedCiContexts: settings.expectedCiContexts,
+        admissionKey,
+      });
       // Live merge-state too — the SAME source the disposition uses (planAgentMaintenanceActions reads liveMergeState).
       // The stored pr.mergeableState lags GitHub's async recompute, and the gate's own check/review publication can
       // also advance mergeability after readiness ran, so refresh at this post-publish boundary.