diff --git a/src/github/backfill.ts b/src/github/backfill.ts index 4884aea8b..1cc1dc807 100644 --- a/src/github/backfill.ts +++ b/src/github/backfill.ts @@ -1712,6 +1712,11 @@ async function fetchPagedSegment( expectedCount: number | undefined, persistPage: (payloads: T[], scanStartedAt: string) => Promise, options: { + /** Marks a segment as a BOUNDED RECENT-WINDOW SAMPLE rather than an exhaustive crawl: when the page budget + * runs out with more pages upstream, the run settles as `sampled` instead of `running`, and records no + * continuation cursor because none can be consumed (#10209 — see the fuller note at the `sampled` branch). + * The name is historical and reads as "walks deeper each run"; it does not. Coverage grows only by + * accretion as the underlying `sort=updated` window slides. */ progressiveHistory?: boolean; countPersisted?: () => Promise; reconcileOnComplete?: (scanStartedAt: string) => Promise; @@ -1814,6 +1819,25 @@ async function fetchPagedSegment( if (status === "complete") { if (hasMore && options.progressiveHistory) { status = "sampled"; + // #10209: a `sampled` segment is NOT resumable, so a nextCursor recorded here is written and never read. + // Three independent gates make that so: this branch maps complete+hasMore to `sampled` rather than + // `running`; `canResumePreviousScan` accepts only running/partial/waiting_rate_limit, so both + // `previous.nextCursor` and an explicitly passed `cursor` are ignored and startPage falls back to 1; and + // the automatic resume re-send below is scoped to labels/open_issues/open_pull_requests, with the + // scheduled cron only ever dispatching light/full. Confirmed live on edge-nl-01: a `resume` run with an + // explicit cursor: "11" against a segment at next_cursor=11 re-crawled pages 1-10 and persisted nothing + // new, returning the same nextCursor: "11" it started with. + // + // Clearing it makes the stored row describe what this segment actually IS -- a bounded window over the + // most-recently-updated closed PRs, re-crawled from page 1 every run, whose coverage grows by accretion + // as the window slides (and is trimmed by the 30-day updated_at retention in src/db/retention.ts). That + // is a defensible design; recording a continuation position no scheduled or manual path can consume is + // not, because it invites a reader to conclude the crawl is advancing when it never can. + // + // `expectedCount` is deliberately KEPT: "this window holds N of the M closed PRs GitHub reports" is a + // true and useful coverage statement. It is only misleading when read as progress toward M, which is + // what the absent cursor now signals. + nextCursor = undefined; } else if (hasMore) { status = "running"; } else { diff --git a/test/unit/backfill.test.ts b/test/unit/backfill.test.ts index 71d19d235..5a7544af3 100644 --- a/test/unit/backfill.test.ts +++ b/test/unit/backfill.test.ts @@ -2,6 +2,7 @@ import { afterEach, describe, expect, it, vi } from "vitest"; import { clearInstallationTokenCacheForTest } from "../../src/github/app"; import { getInstallationHealth, + getRepoSyncSegment, listCheckSummaries, listContributorRepoStats, listIssues, @@ -4052,6 +4053,56 @@ describe("GitHub backfill", () => { expect(result).toMatchObject({ status: "sampled", fetchedCount: 10, expectedCount: 2000 }); expect(await listRecentMergedPullRequests(env, "JSONbored/gittensory")).toHaveLength(10); + + // REGRESSION (#10209): a `sampled` segment records NO continuation cursor. It is not resumable -- + // canResumePreviousScan accepts only running/partial/waiting_rate_limit -- so a stored nextCursor would be + // written and never read, describing a continuation no scheduled or manual path can perform. expectedCount + // is deliberately still recorded: "10 of 2000" is a true coverage statement, just not a progress one. + expect(result.nextCursor ?? null).toBeNull(); + const stored = await getRepoSyncSegment(env, "JSONbored/gittensory", "recent_merged_pull_requests"); + expect(stored?.status).toBe("sampled"); + expect(stored?.nextCursor ?? null).toBeNull(); + expect(stored?.expectedCount).toBe(2000); + }); + + it("REGRESSION (#10209): a resume dispatched against a sampled segment restarts at page 1 rather than advancing", async () => { + // Pins the behaviour the absent cursor now advertises honestly. Verified live on edge-nl-01 before the + // change: a resume run with an explicit cursor: "11" against next_cursor=11 re-crawled pages 1-10 and + // persisted nothing new. The segment is a rolling most-recently-updated window, not a deepening crawl. + const env = createTestEnv({ GITHUB_PUBLIC_TOKEN: "public-token" }); + await seedRegisteredRepo(env); + const pagesRequested: number[] = []; + vi.stubGlobal("fetch", async (input: RequestInfo | URL) => { + const url = input.toString(); + if (url === "https://api.github.com/graphql") return githubTotalsResponse({ openIssues: 0, openPullRequests: 0, mergedPullRequests: 2000, closedPullRequests: 0, labels: 0 }); + if (/\/pulls\/\d+\/files/.test(url)) return Response.json([]); + if (url.includes("/pulls?state=closed")) { + const page = Number(new URL(url).searchParams.get("page") ?? "1"); + pagesRequested.push(page); + return Response.json( + [{ number: page, title: `Merged ${page}`, state: "closed", merged_at: "2026-05-20T00:00:00.000Z", user: { login: "oktofeesh1" }, labels: [], body: "" }], + { headers: { link: `; rel="next"` } }, + ); + } + return Response.json([]); + }); + + await backfillRepositorySegment(env, { repoFullName: "JSONbored/gittensory", segment: "recent_merged_pull_requests", mode: "full" }); + pagesRequested.length = 0; + + const resumed = await backfillRepositorySegment(env, { + repoFullName: "JSONbored/gittensory", + segment: "recent_merged_pull_requests", + mode: "resume", + cursor: "11", + }); + + // The explicitly-passed cursor is ignored: the crawl restarts from page 1, so nothing beyond the window + // is ever reachable and the run settles as `sampled` again with no cursor to hand back. + expect(pagesRequested[0]).toBe(1); + expect(pagesRequested).not.toContain(11); + expect(resumed).toMatchObject({ status: "sampled" }); + expect(resumed.nextCursor ?? null).toBeNull(); }); it("hydrates PR files and reviews through GraphQL when public-token REST detail endpoints are hidden", async () => {