From add8bdaf5cd641cc9bed44d3e93b1f0fa19b1d87 Mon Sep 17 00:00:00 2001 From: Andriy Polanski Date: Sun, 26 Jul 2026 14:13:33 +0000 Subject: [PATCH] fix(github): cap unbounded backfill list pagination at 10 pages (#8890) Bound syncLabels and both GraphQL open-issue/PR undercount supplements so a pathological repo cannot turn one sync into an unbounded fetch loop. Co-authored-by: Cursor --- src/github/backfill.ts | 86 +++++++++++++++++++++------ test/unit/backfill.test.ts | 116 +++++++++++++++++++++++++++++++++++++ 2 files changed, 184 insertions(+), 18 deletions(-) diff --git a/src/github/backfill.ts b/src/github/backfill.ts index 5a57c54b34..7875028ac8 100644 --- a/src/github/backfill.ts +++ b/src/github/backfill.ts @@ -1476,7 +1476,12 @@ async function backfillOpenIssuesSegment( { countPersisted: () => countOpenIssues(env, repo.fullName), reconcileOnComplete: (scanStartedAt) => markUnseenOpenIssuesClosed(env, repo.fullName, scanStartedAt), - ...(token ? { supplementOnUnderCount: (scanStartedAt: string) => supplementOpenIssuesFromGraphQl(env, repo, token, scanStartedAt) } : {}), + ...(token + ? { + supplementOnUnderCount: (scanStartedAt: string, supplementWarnings: string[]) => + supplementOpenIssuesFromGraphQl(env, repo, token, scanStartedAt, supplementWarnings), + } + : {}), }, ); return result; @@ -1508,7 +1513,13 @@ async function backfillOpenPullRequestsSegment( { countPersisted: () => countOpenPullRequests(env, repo.fullName), reconcileOnComplete: (scanStartedAt) => markUnseenOpenPullRequestsClosed(env, repo.fullName, scanStartedAt), - ...(token ? { supplementOnUnderCount: (scanStartedAt: string) => supplementOpenPullRequestsFromGraphQl(env, repo, token, scanStartedAt), supplementDescription: "open pull request row(s)" } : {}), + ...(token + ? { + supplementOnUnderCount: (scanStartedAt: string, supplementWarnings: string[]) => + supplementOpenPullRequestsFromGraphQl(env, repo, token, scanStartedAt, supplementWarnings), + supplementDescription: "open pull request row(s)", + } + : {}), }, ); } @@ -1621,7 +1632,7 @@ async function fetchPagedSegment( progressiveHistory?: boolean; countPersisted?: () => Promise; reconcileOnComplete?: (scanStartedAt: string) => Promise; - supplementOnUnderCount?: (scanStartedAt: string) => Promise; + supplementOnUnderCount?: (scanStartedAt: string, warnings: string[]) => Promise; supplementDescription?: string; } = {}, ): Promise<{ status: RepoSyncSegmentRecord["status"]; segment: RepoSyncSegmentRecord }> { @@ -1760,7 +1771,7 @@ async function fetchPagedSegment( async function supplementUnderCountIfNeeded( options: { countPersisted?: () => Promise; - supplementOnUnderCount?: (scanStartedAt: string) => Promise; + supplementOnUnderCount?: (scanStartedAt: string, warnings: string[]) => Promise; supplementDescription?: string; }, scanStartedAt: string, @@ -1770,7 +1781,7 @@ async function supplementUnderCountIfNeeded( ): Promise { if (expectedCount === undefined || fetchedCount >= expectedCount || !options.supplementOnUnderCount) return fetchedCount; try { - const supplemented = await options.supplementOnUnderCount(scanStartedAt); + const supplemented = await options.supplementOnUnderCount(scanStartedAt, warnings); if (supplemented > 0) warnings.push(`Supplemented ${supplemented} ${options.supplementDescription ?? "open issue row(s)"} from GitHub GraphQL because REST pagination undercounted the authoritative total.`); /* v8 ignore next -- Under-count supplements normally re-count persisted rows; arithmetic fallback protects custom callers. */ return options.countPersisted ? await options.countPersisted() : fetchedCount + supplemented; @@ -1780,14 +1791,19 @@ async function supplementUnderCountIfNeeded( } } -async function supplementOpenIssuesFromGraphQl(env: Env, repo: RepositoryRecord, token: string, seenOpenAt: string): Promise { - /* v8 ignore start -- Defensive GitHub GraphQL payload normalization is covered by sparse-payload backfill tests. */ +async function supplementOpenIssuesFromGraphQl( + env: Env, + repo: RepositoryRecord, + token: string, + seenOpenAt: string, + warnings: string[], +): Promise { const existingNumbers = new Set(await listOpenIssueNumbers(env, repo.fullName)); const { owner, name } = repoParts(repo.fullName); const admissionKey = repoAdmissionKeyForToken(env, repo, token); let after = ""; let supplemented = 0; - for (;;) { + for (let page = 1; page <= GITHUB_LIST_MAX_PAGES; page += 1) { const query = `query LoopOverOpenIssuesSupplement { repository(owner: ${JSON.stringify(owner)}, name: ${JSON.stringify(name)}) { issues(states: OPEN, first: 100${after}) { @@ -1829,20 +1845,30 @@ async function supplementOpenIssuesFromGraphQl(env: Env, repo: RepositoryRecord, supplemented += 1; } if (!issues?.pageInfo?.hasNextPage || !issues.pageInfo.endCursor) break; + if (page === GITHUB_LIST_MAX_PAGES) { + warnings.push( + `GitHub GraphQL open-issues supplement reached local page cap of ${GITHUB_LIST_MAX_PAGES}; stopping with ${supplemented} supplemented row(s).`, + ); + break; + } after = `, after: ${JSON.stringify(issues.pageInfo.endCursor)}`; } return supplemented; - /* v8 ignore stop */ } -async function supplementOpenPullRequestsFromGraphQl(env: Env, repo: RepositoryRecord, token: string, seenOpenAt: string): Promise { - /* v8 ignore start -- Defensive GitHub GraphQL payload normalization is covered by sparse-payload backfill tests. */ +async function supplementOpenPullRequestsFromGraphQl( + env: Env, + repo: RepositoryRecord, + token: string, + seenOpenAt: string, + warnings: string[], +): Promise { const existingNumbers = new Set((await listOpenPullRequests(env, repo.fullName)).map((pr) => pr.number)); const { owner, name } = repoParts(repo.fullName); const admissionKey = repoAdmissionKeyForToken(env, repo, token); let after = ""; let supplemented = 0; - for (;;) { + for (let page = 1; page <= GITHUB_LIST_MAX_PAGES; page += 1) { const query = `query LoopOverOpenPullRequestsSupplement { repository(owner: ${JSON.stringify(owner)}, name: ${JSON.stringify(name)}) { pullRequests(states: OPEN, first: 100${after}, orderBy: { field: CREATED_AT, direction: ASC }) { @@ -1895,10 +1921,15 @@ async function supplementOpenPullRequestsFromGraphQl(env: Env, repo: RepositoryR supplemented += 1; } if (!pullRequests?.pageInfo?.hasNextPage || !pullRequests.pageInfo.endCursor) break; + if (page === GITHUB_LIST_MAX_PAGES) { + warnings.push( + `GitHub GraphQL open-pull-requests supplement reached local page cap of ${GITHUB_LIST_MAX_PAGES}; stopping with ${supplemented} supplemented row(s).`, + ); + break; + } after = `, after: ${JSON.stringify(pullRequests.pageInfo.endCursor)}`; } return supplemented; - /* v8 ignore stop */ } // Terminal segment states that count as synced. `sampled` is the terminal state of the @@ -2363,6 +2394,10 @@ async function fetchAndStorePullRequestDetails( // undefined (the caller can fall back to GraphQL); a later-page failure keeps the pages already fetched // rather than dropping a successful first page. const PR_DETAIL_MAX_PAGES = 10; +// Shared bound for syncLabels + GraphQL open-issue/PR undercount supplements (#8890): every other pagination +// walk in this file already caps at 10 pages so a pathological repo can't turn one sync into an unbounded +// fetch loop. These three loops were the remaining unbounded `for (;;)` / `for (page = 1; ;)` outliers. +const GITHUB_LIST_MAX_PAGES = 10; async function githubPaginatedList( env: Env, @@ -4401,20 +4436,35 @@ async function syncLabels( const startedAt = nowIso(); await markSegmentRunning(env, repo, "labels", sourceKind, mode, startedAt); const items: GitHubLabelPayload[] = []; + const warnings: string[] = []; const admissionKey = repoAdmissionKeyForToken(env, repo, token); try { - for (let page = 1; ; page += 1) { + let status: RepoSyncSegmentRecord["status"] = "complete"; + let nextCursor: string | undefined; + let pageCount = 0; + for (let page = 1; page <= GITHUB_LIST_MAX_PAGES; page += 1) { const result = await githubJsonWithHeaders(env, repo.fullName, `/labels?per_page=100&page=${page}`, token, githubRateLimitOptions(admissionKey)); items.push(...result.data); + pageCount = page; if (!hasNextPage(result.link)) break; + if (page === GITHUB_LIST_MAX_PAGES) { + status = "capped"; + nextCursor = String(page + 1); + warnings.push( + `Label sync reached local page cap of ${GITHUB_LIST_MAX_PAGES} (fetched ${items.length}); next page cursor is ${nextCursor}.`, + ); + break; + } } const segment = await completeSegment(env, repo, "labels", sourceKind, mode, startedAt, { - status: "complete", + status, fetchedCount: items.length, - expectedCount: items.length, - warnings: [], + expectedCount: status === "complete" ? items.length : undefined, + pageCount, + nextCursor, + warnings, }); - return { items, warnings: [], segment }; + return { items, warnings, segment }; } catch (error) { const warning = `Label sync failed: ${errorMessage(error)}`; const segment = await completeSegment(env, repo, "labels", sourceKind, mode, startedAt, { diff --git a/test/unit/backfill.test.ts b/test/unit/backfill.test.ts index 5d921bc67b..c712d341fa 100644 --- a/test/unit/backfill.test.ts +++ b/test/unit/backfill.test.ts @@ -3311,6 +3311,122 @@ describe("GitHub backfill", () => { expect(result.warnings).toEqual(expect.arrayContaining([expect.stringContaining("met the expected total after a late page error")])); }); + it("REGRESSION (#8890): syncLabels stops at the 10-page cap and surfaces a capped segment/warning", async () => { + // Asserts termination at the local page cap (not a mocked hasNextPage:false). syncLabels lives on the + // monolithic backfillRegisteredRepositories path, not backfillRepositorySegment's githubPaged walker. + const env = createTestEnv({ GITHUB_PUBLIC_TOKEN: "public-token" }); + await seedInstalledAndRegisteredRepo(env); + const labelPages: number[] = []; + vi.stubGlobal("fetch", async (input: RequestInfo | URL) => { + const url = input.toString(); + if (url.includes("/access_tokens")) return Response.json({ token: "installation-token" }); + if (url.endsWith("/repos/JSONbored/gittensory")) { + return Response.json({ + name: "gittensory", + full_name: "JSONbored/gittensory", + private: false, + default_branch: "main", + language: "TypeScript", + owner: { login: "JSONbored" }, + }); + } + if (url === "https://api.github.com/graphql") { + return githubTotalsResponse({ openIssues: 0, openPullRequests: 0, mergedPullRequests: 0, closedPullRequests: 0, labels: 2000 }); + } + if (url.includes("/labels?")) { + const page = Number(new URL(url).searchParams.get("page") ?? "1"); + labelPages.push(page); + return Response.json([{ name: `label-${page}`, color: "cc0000", description: `Label ${page}` }], { + headers: { link: `; rel="next"` }, + }); + } + if (url.includes("/issues?") || url.includes("/pulls?")) return Response.json([]); + return new Response(`unexpected ${url}`, { status: 404 }); + }); + + const result = await backfillRegisteredRepositories(env, { + force: true, + limits: { issues: 0, pullRequests: 0, recentMergedPullRequests: 0, pullRequestDetails: 0 }, + }); + + expect(labelPages).toEqual([1, 2, 3, 4, 5, 6, 7, 8, 9, 10]); + expect(result.repos[0]?.dataQuality).toMatchObject({ capped: true }); + expect(result.repos[0]?.warnings.join("\n")).toMatch(/Label sync reached local page cap of 10/); + expect(await listRepoSyncSegments(env, "JSONbored/gittensory")).toEqual( + expect.arrayContaining([ + expect.objectContaining({ segment: "labels", status: "capped", fetchedCount: 10, nextCursor: "11", pageCount: 10 }), + ]), + ); + }); + + it("REGRESSION (#8890): GraphQL open-issues/PR supplements stop at the 10-page cap with a capped warning", async () => { + const env = createTestEnv({ GITHUB_PUBLIC_TOKEN: "public-token" }); + await seedRegisteredRepo(env); + let issueSupplementPages = 0; + let prSupplementPages = 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("LoopOverRepoTotals")) { + return githubTotalsResponse({ openIssues: 2000, openPullRequests: 2000, mergedPullRequests: 0, closedPullRequests: 0, labels: 0 }); + } + if (query.includes("LoopOverOpenIssuesSupplement")) { + issueSupplementPages += 1; + return Response.json({ + data: { + repository: { + issues: { + pageInfo: { hasNextPage: true, endCursor: `issue-cursor-${issueSupplementPages}` }, + nodes: [{ number: 1000 + issueSupplementPages, title: `Issue ${issueSupplementPages}`, state: "OPEN", labels: { nodes: [] } }], + }, + }, + }, + }); + } + if (query.includes("LoopOverOpenPullRequestsSupplement")) { + prSupplementPages += 1; + return Response.json({ + data: { + repository: { + pullRequests: { + pageInfo: { hasNextPage: true, endCursor: `pr-cursor-${prSupplementPages}` }, + nodes: [ + { + number: 2000 + prSupplementPages, + title: `PR ${prSupplementPages}`, + state: "OPEN", + labels: { nodes: [] }, + head: {}, + base: {}, + }, + ], + }, + }, + }, + }); + } + } + if (url.includes("/issues?") || url.includes("/pulls?state=open")) return Response.json([]); + return Response.json([]); + }); + + const issuesResult = await backfillRepositorySegment(env, { repoFullName: "JSONbored/gittensory", segment: "open_issues", mode: "full", force: true }); + const pullRequestsResult = await backfillRepositorySegment(env, { + repoFullName: "JSONbored/gittensory", + segment: "open_pull_requests", + mode: "full", + force: true, + }); + + expect(issueSupplementPages).toBe(10); + expect(prSupplementPages).toBe(10); + expect(issuesResult.warnings.join("\n")).toMatch(/open-issues supplement reached local page cap of 10/); + expect(pullRequestsResult.warnings.join("\n")).toMatch(/open-pull-requests supplement reached local page cap of 10/); + expect(issuesResult.warnings.join("\n")).toMatch(/cap/i); + expect(pullRequestsResult.warnings.join("\n")).toMatch(/cap/i); + }); + it("marks a current open-data segment partial when reconciliation removes stale rows below expected totals", async () => { const env = createTestEnv({ GITHUB_PUBLIC_TOKEN: "public-token" }); await seedRegisteredRepo(env);