Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
24 changes: 24 additions & 0 deletions src/github/backfill.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1712,6 +1712,11 @@ async function fetchPagedSegment<T>(
expectedCount: number | undefined,
persistPage: (payloads: T[], scanStartedAt: string) => Promise<number>,
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<number>;
reconcileOnComplete?: (scanStartedAt: string) => Promise<number>;
Expand Down Expand Up @@ -1814,6 +1819,25 @@ async function fetchPagedSegment<T>(
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 {
Expand Down
51 changes: 51 additions & 0 deletions test/unit/backfill.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import { afterEach, describe, expect, it, vi } from "vitest";
import { clearInstallationTokenCacheForTest } from "../../src/github/app";
import {
getInstallationHealth,
getRepoSyncSegment,
listCheckSummaries,
listContributorRepoStats,
listIssues,
Expand Down Expand Up @@ -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: `<https://api.github.com/repositories/1/pulls?page=${page + 1}>; 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 () => {
Expand Down