Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
loopover-orb merged 1 commit into from
Jul 20, 2026
+196 −36
Merged

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
127 changes: 91 additions & 36 deletions src/github/backfill.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -270,6 +273,7 @@ type GitHubPullRequestDetailsResponse = {
} | null> | null;
} | null;
reviews?: {
pageInfo?: GitHubGraphQlPageInfo;
nodes?: Array<{
databaseId?: number | null;
author?: { login?: string | null } | null;
Expand Down Expand Up @@ -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,
Expand All @@ -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 */
}
Expand Down
105 changes: 105 additions & 0 deletions test/unit/backfill.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down