fix(github): paginate GraphQL PR-detail fallback past 100 nodes (#7493)

galuis116 · web-flow · commit 3ee9e4ef0100 · 2026-07-20T13:12:24.000Z
fetchPullRequestDetailsFromGraphQl's files/reviews connections requested only first: 100 with no pageInfo, so a PR with more than 100 changed files or more than 100 reviews silently truncated when the REST detail fetch fell back to GraphQL -- with no way to even detect it. Add pageInfo { hasNextPage endCursor } to both connections and walk each independently with cursor pagination, bounded by a new PR_DETAIL_GRAPHQL_MAX_PAGES constant mirroring the REST path's PR_DETAIL_MAX_PAGES. A later-page failure keeps the items already fetched instead of discarding them, matching githubPaginatedList's partial-result semantics; a page-1 failure still surfaces the same way to callers as before. Closes #7453
diff --git a/src/github/backfill.ts b/src/github/backfill.ts
@@ -257,11 +257,14 @@ type GitHubOpenPullRequestsResponse = {
   errors?: Array<{ message?: string }>;
 };
 
+type GitHubGraphQlPageInfo = { hasNextPage?: boolean | null; endCursor?: string | null } | null;
+
 type GitHubPullRequestDetailsResponse = {
   data?: {
     repository?: {
       pullRequest?: {
         files?: {
+          pageInfo?: GitHubGraphQlPageInfo;
           nodes?: Array<{
             path?: string | null;
             additions?: number | null;
@@ -270,6 +273,7 @@ type GitHubPullRequestDetailsResponse = {
           } | null> | null;
         } | null;
         reviews?: {
+          pageInfo?: GitHubGraphQlPageInfo;
           nodes?: Array<{
             databaseId?: number | null;
             author?: { login?: string | null } | null;
@@ -4137,6 +4141,12 @@ export async function fetchLinkedIssueFacts(
   };
 }
 
+// GraphQL's `files`/`reviews` connections default to first: 100 with no follow-up, so a PR with more than
+// 100 changed files or more than 100 reviews silently truncates here too -- mirror PR_DETAIL_MAX_PAGES's
+// REST bound with the same 10-page-of-100 (1,000 item) ceiling, walked independently per connection via
+// `pageInfo { hasNextPage endCursor }` instead of a Link header.
+const PR_DETAIL_GRAPHQL_MAX_PAGES = 10;
+
 async function fetchPullRequestDetailsFromGraphQl(
   env: Env,
   repoFullName: string,
@@ -4146,48 +4156,93 @@ async function fetchPullRequestDetailsFromGraphQl(
 ): Promise<{ files: GitHubFilePayload[]; reviews: GitHubReviewPayload[] }> {
   /* v8 ignore start -- GitHub detail GraphQL sparse-node fallbacks are exercised through PR detail hydration tests. */
   const { owner, name } = repoParts(repoFullName);
-  const query = `query LoopOverPullRequestDetails {
-    repository(owner: ${JSON.stringify(owner)}, name: ${JSON.stringify(name)}) {
-      pullRequest(number: ${pullNumber}) {
-        files(first: 100) {
+  const files: GitHubFilePayload[] = [];
+  const reviews: GitHubReviewPayload[] = [];
+  let filesCursor: string | undefined;
+  let reviewsCursor: string | undefined;
+  let filesHasNextPage = true;
+  let reviewsHasNextPage = true;
+
+  for (let page = 1; page <= PR_DETAIL_GRAPHQL_MAX_PAGES && (filesHasNextPage || reviewsHasNextPage); page += 1) {
+    // Only request a connection that still has more pages -- once one connection finishes, the other keeps
+    // paginating alone rather than re-fetching data already fully collected.
+    const filesSelection = filesHasNextPage
+      ? `files(first: 100${filesCursor ? `, after: ${JSON.stringify(filesCursor)}` : ""}) {
+          pageInfo { hasNextPage endCursor }
           nodes { path additions deletions changeType }
-        }
-        reviews(first: 100) {
+        }`
+      : "";
+    const reviewsSelection = reviewsHasNextPage
+      ? `reviews(first: 100${reviewsCursor ? `, after: ${JSON.stringify(reviewsCursor)}` : ""}) {
+          pageInfo { hasNextPage endCursor }
           nodes { databaseId author { login } state authorAssociation submittedAt }
-        }
+        }`
+      : "";
+    const query = `query LoopOverPullRequestDetails {
+    repository(owner: ${JSON.stringify(owner)}, name: ${JSON.stringify(name)}) {
+      pullRequest(number: ${pullNumber}) {
+        ${filesSelection}
+        ${reviewsSelection}
       }
     }
     rateLimit { remaining resetAt }
   }`;
-  const response = await githubGraphQl<GitHubPullRequestDetailsResponse>(env, query, token, admissionKey);
-  const pullRequest = response.data?.repository?.pullRequest;
-  if (!pullRequest) throw new GitHubApiError(`GitHub GraphQL failed for ${repoFullName} pull request #${pullNumber}: pull request not found`, 404, null, null, null, "");
-  const files: GitHubFilePayload[] = (pullRequest.files?.nodes ?? []).flatMap((file) => {
-    if (!file?.path) return [];
-    const additions = Number(file.additions ?? 0);
-    const deletions = Number(file.deletions ?? 0);
-    return [
-      {
-        filename: file.path,
-        status: String(file.changeType ?? "modified").toLowerCase(),
-        additions,
-        deletions,
-        changes: additions + deletions,
-      },
-    ];
-  });
-  const reviews: GitHubReviewPayload[] = (pullRequest.reviews?.nodes ?? []).flatMap((review) => {
-    if (!review?.databaseId) return [];
-    return [
-      {
-        id: review.databaseId,
-        ...(review.author?.login ? { user: { login: review.author.login } } : {}),
-        ...(review.state ? { state: review.state } : {}),
-        ...(review.authorAssociation ? { author_association: review.authorAssociation } : {}),
-        ...(review.submittedAt === undefined ? {} : { submitted_at: review.submittedAt }),
-      },
-    ];
-  });
+    // A page-1 failure surfaces the same way the un-paginated call always did (the caller's `.catch(() =>
+    // undefined)` falls through to the REST+GraphQL-failed warning). A LATER page failing must not discard
+    // the pages already collected -- same "keep a successful partial result" semantics as githubPaginatedList.
+    let response: GitHubPullRequestDetailsResponse | undefined;
+    try {
+      response = await githubGraphQl<GitHubPullRequestDetailsResponse>(env, query, token, admissionKey);
+    } catch (error) {
+      if (page === 1) throw error;
+      break;
+    }
+    const pullRequest = response.data?.repository?.pullRequest;
+    if (!pullRequest) {
+      if (page === 1) throw new GitHubApiError(`GitHub GraphQL failed for ${repoFullName} pull request #${pullNumber}: pull request not found`, 404, null, null, null, "");
+      break;
+    }
+    if (filesHasNextPage) {
+      files.push(
+        ...(pullRequest.files?.nodes ?? []).flatMap((file) => {
+          if (!file?.path) return [];
+          const additions = Number(file.additions ?? 0);
+          const deletions = Number(file.deletions ?? 0);
+          return [
+            {
+              filename: file.path,
+              status: String(file.changeType ?? "modified").toLowerCase(),
+              additions,
+              deletions,
+              changes: additions + deletions,
+            },
+          ];
+        }),
+      );
+      const pageInfo = pullRequest.files?.pageInfo;
+      if (pageInfo?.hasNextPage && pageInfo.endCursor) filesCursor = pageInfo.endCursor;
+      else filesHasNextPage = false;
+    }
+    if (reviewsHasNextPage) {
+      reviews.push(
+        ...(pullRequest.reviews?.nodes ?? []).flatMap((review) => {
+          if (!review?.databaseId) return [];
+          return [
+            {
+              id: review.databaseId,
+              ...(review.author?.login ? { user: { login: review.author.login } } : {}),
+              ...(review.state ? { state: review.state } : {}),
+              ...(review.authorAssociation ? { author_association: review.authorAssociation } : {}),
+              ...(review.submittedAt === undefined ? {} : { submitted_at: review.submittedAt }),
+            },
+          ];
+        }),
+      );
+      const pageInfo = pullRequest.reviews?.pageInfo;
+      if (pageInfo?.hasNextPage && pageInfo.endCursor) reviewsCursor = pageInfo.endCursor;
+      else reviewsHasNextPage = false;
+    }
+  }
   return { files, reviews };
   /* v8 ignore stop */
 }
diff --git a/test/unit/backfill.test.ts b/test/unit/backfill.test.ts
@@ -3663,6 +3663,111 @@ describe("GitHub backfill", () => {
     );
   });
 
+  it("paginates the GraphQL PR-detail fallback past 100 files instead of truncating at the first page (#7453)", async () => {
+    const env = createTestEnv({ GITHUB_PUBLIC_TOKEN: "public-token" });
+    await seedRegisteredRepo(env);
+    await upsertPullRequestFromGitHub(env, "JSONbored/gittensory", {
+      number: 30,
+      title: "PR with more than 100 changed files",
+      state: "open",
+      user: { login: "oktofeesh1" },
+      head: { sha: "hugesha" },
+      labels: [],
+      body: "",
+    });
+    const firstPageFiles = Array.from({ length: 100 }, (_, index) => ({ path: `src/file-${index}.ts`, additions: 1, deletions: 0, changeType: "MODIFIED" }));
+    vi.stubGlobal("fetch", async (input: RequestInfo | URL, init?: RequestInit) => {
+      const url = input.toString();
+      if (url === "https://api.github.com/graphql") {
+        const query = JSON.parse(String(init?.body ?? "{}")).query as string;
+        if (query.includes("LoopOverPullRequestDetails")) {
+          // A second-page request carries the `after` cursor the first page's pageInfo.endCursor handed back.
+          // The un-paginated fallback never sends this, so this branch only fires against the fixed code.
+          if (query.includes('after: "files-page-2"')) {
+            return Response.json({
+              data: {
+                repository: {
+                  pullRequest: {
+                    files: { pageInfo: { hasNextPage: false, endCursor: null }, nodes: [{ path: "src/file-100.ts", additions: 1, deletions: 0, changeType: "MODIFIED" }] },
+                  },
+                },
+                rateLimit: { remaining: 4998, resetAt: "2026-05-25T16:00:00Z" },
+              },
+            });
+          }
+          return Response.json({
+            data: {
+              repository: {
+                pullRequest: {
+                  files: { pageInfo: { hasNextPage: true, endCursor: "files-page-2" }, nodes: firstPageFiles },
+                  reviews: { pageInfo: { hasNextPage: false, endCursor: null }, nodes: [{ databaseId: 1, author: { login: "rev1" }, state: "APPROVED", authorAssociation: "MEMBER", submittedAt: "2026-05-25T00:00:00Z" }] },
+                },
+              },
+              rateLimit: { remaining: 4999, resetAt: "2026-05-25T16:00:00Z" },
+            },
+          });
+        }
+      }
+      if (url.includes("/pulls/30/files") || url.includes("/pulls/30/reviews")) return new Response("", { status: 404 });
+      if (url.includes("/commits/hugesha/check-runs")) return Response.json({});
+      return Response.json([]);
+    });
+
+    const result = await backfillOpenPullRequestDetails(env, { repoFullName: "JSONbored/gittensory", mode: "full", cursor: 0 });
+
+    expect(result).toMatchObject({ status: "complete", processed: 1, warnings: [] });
+    // 101 files total (100 on page 1 + 1 on page 2) must all be persisted -- truncating at page 1 would leave 100.
+    expect(await listPullRequestFiles(env, "JSONbored/gittensory", 30)).toHaveLength(101);
+    expect(await listPullRequestReviews(env, "JSONbored/gittensory", 30)).toEqual([expect.objectContaining({ id: "JSONbored/gittensory#30#1", reviewerLogin: "rev1" })]);
+  });
+
+  it("does not send a second GraphQL PR-detail page when the fallback's files/reviews already fit on page one", async () => {
+    const env = createTestEnv({ GITHUB_PUBLIC_TOKEN: "public-token" });
+    await seedRegisteredRepo(env);
+    await upsertPullRequestFromGitHub(env, "JSONbored/gittensory", {
+      number: 31,
+      title: "Single-page PR",
+      state: "open",
+      user: { login: "oktofeesh1" },
+      head: { sha: "singlesha" },
+      labels: [],
+      body: "",
+    });
+    let graphQlCalls = 0;
+    vi.stubGlobal("fetch", async (input: RequestInfo | URL, init?: RequestInit) => {
+      const url = input.toString();
+      if (url === "https://api.github.com/graphql") {
+        const query = JSON.parse(String(init?.body ?? "{}")).query as string;
+        if (query.includes("LoopOverPullRequestDetails")) {
+          graphQlCalls += 1;
+          return Response.json({
+            data: {
+              repository: {
+                pullRequest: {
+                  files: { pageInfo: { hasNextPage: false, endCursor: null }, nodes: [{ path: "src/only.ts", additions: 2, deletions: 1, changeType: "MODIFIED" }] },
+                  reviews: { pageInfo: { hasNextPage: false, endCursor: null }, nodes: [{ databaseId: 60, author: { login: "solo-reviewer" }, state: "APPROVED", authorAssociation: "MEMBER", submittedAt: "2026-05-25T00:00:00Z" }] },
+                },
+              },
+              rateLimit: { remaining: 4999, resetAt: "2026-05-25T16:00:00Z" },
+            },
+          });
+        }
+      }
+      if (url.includes("/pulls/31/files") || url.includes("/pulls/31/reviews")) return new Response("", { status: 404 });
+      if (url.includes("/commits/singlesha/check-runs")) return Response.json({});
+      return Response.json([]);
+    });
+
+    const result = await backfillOpenPullRequestDetails(env, { repoFullName: "JSONbored/gittensory", mode: "full", cursor: 0 });
+
+    expect(result).toMatchObject({ status: "complete", processed: 1, warnings: [] });
+    expect(await listPullRequestFiles(env, "JSONbored/gittensory", 31)).toEqual([expect.objectContaining({ path: "src/only.ts", changes: 3 })]);
+    expect(await listPullRequestReviews(env, "JSONbored/gittensory", 31)).toEqual([expect.objectContaining({ id: "JSONbored/gittensory#31#60", reviewerLogin: "solo-reviewer" })]);
+    // fetchPullRequestFiles and fetchPullRequestReviews each independently fall back to GraphQL (one call
+    // apiece), but neither should send a page-2 request when pageInfo.hasNextPage is false on page one.
+    expect(graphQlCalls).toBe(2);
+  });
+
   it("refreshes one pull request's files before gate evaluation and drops stale cached paths", async () => {
     const env = createTestEnv({ GITHUB_PUBLIC_TOKEN: "public-token" });
     await seedRegisteredRepo(env);
