From 6d7cbd5bb4995842fe1b28f44f8426c24ecbec39 Mon Sep 17 00:00:00 2001 From: andriypolanski Date: Fri, 31 Jul 2026 14:37:25 +0000 Subject: [PATCH] fix(miner): cap check-runs page follows so poll cannot spin forever (#10007) Add a normalized maxPages bound to fetchCheckRuns, mirroring opportunity-fanout, and throw a distinct page-cap error when a Link chain exceeds it. --- packages/loopover-miner/lib/ci-poller.ts | 14 ++- test/unit/miner-ci-poller.test.ts | 136 +++++++++++++++++++++++ 2 files changed, 149 insertions(+), 1 deletion(-) diff --git a/packages/loopover-miner/lib/ci-poller.ts b/packages/loopover-miner/lib/ci-poller.ts index 97641b535f..c6f866d535 100644 --- a/packages/loopover-miner/lib/ci-poller.ts +++ b/packages/loopover-miner/lib/ci-poller.ts @@ -5,6 +5,10 @@ const defaultMinIntervalMs = 60_000; const defaultMaxIntervalMs = 5 * 60_000; const defaultMaxAttempts = 1; const defaultRequestTimeoutMs = 10_000; +// Follow the check-runs Link header past the first page so a head with >100 checks isn't silently +// truncated; cap the follow loop so a pathological Link chain can't run away (#10007). Default 10 +// pages × per_page=100 = 1000 checks — well above any realistic PR check-run count. +const defaultMaxPages = 10; const githubApiVersion = "2022-11-28"; export type CheckRunConclusion = "pending" | "success" | "failure" | "neutral"; @@ -33,6 +37,8 @@ export type PollCheckRunsOptions = { minIntervalMs?: number; maxIntervalMs?: number; requestTimeoutMs?: number; + /** Cap on check-runs page follows (#10007); defaults to `defaultMaxPages`. */ + maxPages?: number; sleepFn?: (delayMs: number) => Promise; }; @@ -44,6 +50,7 @@ type NormalizedPollOptions = { minIntervalMs: number; maxIntervalMs: number; requestTimeoutMs: number; + maxPages: number; sleepFn: (delayMs: number) => Promise; }; @@ -81,6 +88,8 @@ function normalizeOptions(options: PollCheckRunsOptions = {}): NormalizedPollOpt minIntervalMs: normalizePositiveInt(options.minIntervalMs, defaultMinIntervalMs, 1, 60 * 60_000), maxIntervalMs: normalizePositiveInt(options.maxIntervalMs, defaultMaxIntervalMs, 1, 60 * 60_000), requestTimeoutMs: normalizePositiveInt(options.requestTimeoutMs, defaultRequestTimeoutMs, 1, 60_000), + // Same clamp shape as opportunity-fanout's maxPages (#4831 / #10007). + maxPages: normalizePositiveInt(options.maxPages, defaultMaxPages, 1, 100), sleepFn: options.sleepFn ?? ((delayMs: number) => new Promise((resolve) => setTimeout(resolve, delayMs))), @@ -233,7 +242,8 @@ async function fetchCheckRuns( const checks: NormalizedCheckRun[] = []; let page = 1; let expectedTotalCount: number | null = null; - while (true) { + // #10007: page-count cap (not checks.length) so a looping endpoint that re-serves the same page can't spin forever. + while (page <= options.maxPages) { const { payload, response } = await githubGetJsonResponse( apiUrl( options.apiBaseUrl, @@ -257,6 +267,8 @@ async function fetchCheckRuns( } page += 1; } + // Distinct from pagination_incomplete: the server kept advertising more pages past the hard cap. + throw new Error("github_check_runs_pagination_page_cap"); } export async function pollCheckRuns( diff --git a/test/unit/miner-ci-poller.test.ts b/test/unit/miner-ci-poller.test.ts index de2bfc3f4b..630f6168a8 100644 --- a/test/unit/miner-ci-poller.test.ts +++ b/test/unit/miner-ci-poller.test.ts @@ -604,4 +604,140 @@ describe("miner CI check-run poller (#2323)", () => { vi.useRealTimers(); } }); + + it('REGRESSION: a never-ending rel="next" chain stops at the page cap instead of looping forever (#10007)', async () => { + const fetchFn = vi.fn(async (input: RequestInfo | URL) => { + const url = String(input); + if (url.endsWith("/pulls/18")) return prResponse("cap-sha"); + if (url.includes("/check-runs")) { + // Always advertise another page with a non-empty body — the pre-fix while(true) never exits. + return checksResponse([checkRun("validate", "completed", "success")], { + totalCount: 10_000, + headers: { + link: `<${API}/repos/acme/widgets/commits/cap-sha/check-runs?per_page=100&page=999>; rel="next"`, + }, + }); + } + return jsonResponse({}, { status: 404 }); + }); + + await expect( + pollCheckRuns("acme/widgets", 18, { + apiBaseUrl: API, + fetchFn, + maxPages: 3, + sleepFn: vi.fn(async () => {}), + }), + ).rejects.toThrow("github_check_runs_pagination_page_cap"); + + const checkRunCalls = fetchFn.mock.calls.filter((call) => String(call[0]).includes("/check-runs")); + expect(checkRunCalls).toHaveLength(3); + }); + + it("still returns every check run across a two-page response under the default maxPages (#10007)", async () => { + const fetchFn = vi.fn(async (input: RequestInfo | URL) => { + const url = String(input); + if (url.endsWith("/pulls/19")) return prResponse("two-page-sha"); + if (url.includes("&page=1")) { + return checksResponse([checkRun("a", "completed", "success")], { + totalCount: 2, + headers: { + link: `<${API}/repos/acme/widgets/commits/two-page-sha/check-runs?per_page=100&page=2>; rel="next"`, + }, + }); + } + if (url.includes("&page=2")) { + return checksResponse([checkRun("b", "completed", "failure")], { totalCount: 2 }); + } + return jsonResponse({}, { status: 404 }); + }); + + const result = await pollCheckRuns("acme/widgets", 19, { + apiBaseUrl: API, + fetchFn, + sleepFn: vi.fn(async () => {}), + // omit maxPages — default must still allow a normal two-page follow + }); + expect(result.checks.map((check) => check.name)).toEqual(["a", "b"]); + expect(result.conclusion).toBe("failure"); + }); + + it("still throws github_check_runs_pagination_incomplete on an empty mid-stream page (#10007)", async () => { + const fetchFn = vi.fn(async (input: RequestInfo | URL) => { + const url = String(input); + if (url.endsWith("/pulls/20")) return prResponse("empty-page-sha"); + if (url.includes("&page=1")) { + return checksResponse([checkRun("a", "completed", "success")], { + totalCount: 2, + headers: { + link: `<${API}/repos/acme/widgets/commits/empty-page-sha/check-runs?per_page=100&page=2>; rel="next"`, + }, + }); + } + if (url.includes("&page=2")) { + return checksResponse([], { totalCount: 2 }); + } + return jsonResponse({}, { status: 404 }); + }); + + await expect( + pollCheckRuns("acme/widgets", 20, { + apiBaseUrl: API, + fetchFn, + sleepFn: vi.fn(async () => {}), + }), + ).rejects.toThrow("github_check_runs_pagination_incomplete"); + }); + + it("clamps maxPages to the floor of 1 and the ceiling of 100 (#10007)", async () => { + const endless = (sha: string) => + vi.fn(async (input: RequestInfo | URL) => { + const url = String(input); + if (url.includes("/pulls/")) return prResponse(sha); + if (url.includes("/check-runs")) { + return checksResponse([checkRun("validate", "completed", "success")], { + totalCount: 10_000, + headers: { + link: `<${API}/repos/acme/widgets/commits/${sha}/check-runs?per_page=100&page=999>; rel="next"`, + }, + }); + } + return jsonResponse({}, { status: 404 }); + }); + + // Floor: non-finite / sub-1 values fall back or clamp to 1 → exactly one check-runs page before the cap error. + const floorFetch = endless("floor-sha"); + await expect( + pollCheckRuns("acme/widgets", 21, { + apiBaseUrl: API, + fetchFn: floorFetch, + maxPages: 0, + sleepFn: vi.fn(async () => {}), + }), + ).rejects.toThrow("github_check_runs_pagination_page_cap"); + expect(floorFetch.mock.calls.filter((call) => String(call[0]).includes("/check-runs"))).toHaveLength(1); + + // Omitted maxPages uses the default (10): ten check-runs pages, then the cap error. + const defaultFetch = endless("default-sha"); + await expect( + pollCheckRuns("acme/widgets", 22, { + apiBaseUrl: API, + fetchFn: defaultFetch, + sleepFn: vi.fn(async () => {}), + }), + ).rejects.toThrow("github_check_runs_pagination_page_cap"); + expect(defaultFetch.mock.calls.filter((call) => String(call[0]).includes("/check-runs"))).toHaveLength(10); + + // Ceiling: values above 100 clamp to 100. + const ceilingFetch = endless("ceiling-sha"); + await expect( + pollCheckRuns("acme/widgets", 23, { + apiBaseUrl: API, + fetchFn: ceilingFetch, + maxPages: 10_000, + sleepFn: vi.fn(async () => {}), + }), + ).rejects.toThrow("github_check_runs_pagination_page_cap"); + expect(ceilingFetch.mock.calls.filter((call) => String(call[0]).includes("/check-runs"))).toHaveLength(100); + }); });