From 33835196d2968db0b8556a988e5eea874ca2965d Mon Sep 17 00:00:00 2001 From: JSONbored <49853598+JSONbored@users.noreply.github.com> Date: Tue, 28 Jul 2026 16:42:00 -0700 Subject: [PATCH 1/2] feat(benchmark): leak-proof frozen-repo snapshot builder (#9259) An agent must see EXACTLY the repository state a maintainer saw at commit T and nothing after it. A future-information leak inflates every score built on top, and it does so invisibly -- a leaked snapshot still produces perfectly well-formed numbers. That makes leak-proofing the deliverable rather than a property, which is why all filtering lives in a pure, exhaustively tested core instead of inline in the CLI. What a snapshot carries: open PRs and issues as of T, with labels filtered to those applied at or before T, plus gate decisions recorded at or before T. What it never carries: anything created after T (never admitted, so a downstream bug cannot reintroduce it), and the OUTCOME of any included unit -- an open PR's eventual merge/close is precisely what the agent is asked to predict, so carrying it would make the snapshot an answer key rather than a benchmark. The frozen type has no outcome field at all. The T boundary is inclusive and defined ONCE, in atOrBefore, so no field can quietly disagree with another. An unreadable timestamp is treated as not-visible: a date nobody can parse might be from after T, and the cost of excluding it is lost context rather than an inflated score. Two real defects were found and fixed while testing, both of which would have broken reproducibility silently: - checksumSnapshot spread its argument, so handing it a whole snapshot (the natural re-verification call) folded the existing checksum into its own preimage and returned a wrong digest with no error. The committed fields are now listed explicitly, making the function total over both shapes and making an added field a compile error at the commitment. - The decision sort tied on (workUnitId, decidedAt, action), so two decisions differing only in reasonCode ordered by input order and the checksum moved when the CLI read them the other way round. The chain now ends in a canonical-serialization comparison, total by construction. The CLI refuses to write a snapshot that fails an independent leak audit: an inflated benchmark is worse than no benchmark, because its numbers look fine. Closes #9259 --- scripts/frozen-repo-snapshot-core.ts | 281 ++++++++++++++++++ scripts/frozen-repo-snapshot.ts | 162 +++++++++++ test/unit/frozen-repo-snapshot-core.test.ts | 297 ++++++++++++++++++++ 3 files changed, 740 insertions(+) create mode 100644 scripts/frozen-repo-snapshot-core.ts create mode 100644 scripts/frozen-repo-snapshot.ts create mode 100644 test/unit/frozen-repo-snapshot-core.test.ts diff --git a/scripts/frozen-repo-snapshot-core.ts b/scripts/frozen-repo-snapshot-core.ts new file mode 100644 index 0000000000..e9817ff772 --- /dev/null +++ b/scripts/frozen-repo-snapshot-core.ts @@ -0,0 +1,281 @@ +// Frozen-repo snapshot builder, pure core (#9259, harness #9216, epic #8534). +// +// Scoring an arbitrary agent against realized history is only meaningful if the agent sees EXACTLY the +// repository state a maintainer saw at commit T and NOTHING that happened after it. A future-information +// leak silently inflates every score built on top — and it inflates them invisibly, because a leaked +// snapshot still produces perfectly well-formed numbers. That makes leak-proofing the deliverable of this +// module rather than a property of it, which is why the filtering is here, pure and exhaustively tested, +// instead of inline in a CLI where it would be untestable. +// +// ── WHAT IS INCLUDED, AND WHAT IS DELIBERATELY NOT ─────────────────────────────────────────────────── +// INCLUDED, each filtered to its state as of `frozenAt`: +// • openPullRequests — PRs created at or before T that were still open at T. Labels are filtered to +// those applied at or before T; the body/title are the values as of T. +// • openIssues — same rule. +// • recentDecisions — gate decisions RECORDED at or before T. These are history the maintainer could +// genuinely see, and they are what makes the task realistic rather than context-free. +// +// NOT INCLUDED, ever: +// • Anything created after T. Not "filtered from the output" — never admitted, so a downstream bug +// cannot reintroduce it. +// • The OUTCOME of any included work unit. A snapshot carries the question, never the answer: an +// open PR's eventual merge/close is exactly what the agent is being asked to predict, so a snapshot +// that carried `state: "merged"` would not be a hard benchmark, it would be an answer key. +// • Comments, labels, reviews, or status changes timestamped after T, even on an included record. +// +// ── THE BOUNDARY IS INCLUSIVE AT T ─────────────────────────────────────────────────────────────────── +// An event AT exactly `frozenAt` is included: T is the instant the maintainer is standing at, so what +// happened at T is what they can see. Everything strictly after is the future. This is stated once, here, +// and implemented once, in `atOrBefore`, so no field can quietly disagree with another. +// +// Pure: no IO, no clock, no randomness. `scripts/frozen-repo-snapshot.ts` is the thin CLI that does the +// GitHub/DB reads and hands the raw records in. Checksum discipline mirrors `checksumCases` in +// backtest-corpus-export-core.ts exactly (canonicalize with sorted keys, JSON-stringify, sha256). + +import { createHash } from "node:crypto"; + +export const FROZEN_REPO_SNAPSHOT_SCHEMA_VERSION = 1 as const; + +/** A label with the instant it was APPLIED — the timestamp is what makes label filtering possible at all. + * A raw GitHub label carries no application time; the CLI derives it from the issue-events timeline. */ +export type TimestampedLabel = { name: string; appliedAt: string }; + +/** One PR or issue as the CLI read it, with every time-varying field carrying its own timestamp. */ +export type RawWorkUnitRecord = { + /** `owner/repo#123`. */ + workUnitId: string; + number: number; + kind: "pull_request" | "issue"; + title: string; + body: string; + authorLogin: string; + createdAt: string; + /** When it was closed/merged, if it ever was. Present in the RAW record and deliberately dropped from + * the snapshot — see this module's header. Used only to decide open-at-T. */ + closedAt?: string | null | undefined; + labels?: readonly TimestampedLabel[] | undefined; + /** Changed file paths. A PR's file list is fixed at push time; the CLI supplies the list as of T. */ + changedPaths?: readonly string[] | undefined; +}; + +/** A past gate decision the maintainer could see at T. */ +export type RawDecisionRecord = { + workUnitId: string; + action: string; + reasonCode: string; + decidedAt: string; +}; + +/** One work unit as it appears IN the snapshot. Note what is absent: no `closedAt`, no state, no outcome + * of any kind — the type itself refuses to carry the answer. */ +export type FrozenWorkUnit = { + workUnitId: string; + number: number; + kind: "pull_request" | "issue"; + title: string; + body: string; + authorLogin: string; + createdAt: string; + /** Label NAMES only, sorted — the application timestamps did their job during filtering and would + * otherwise be one more channel through which post-T information could travel. */ + labels: string[]; + changedPaths: string[]; +}; + +export type FrozenDecision = { workUnitId: string; action: string; reasonCode: string; decidedAt: string }; + +export type FrozenRepoSnapshot = { + schemaVersion: typeof FROZEN_REPO_SNAPSHOT_SCHEMA_VERSION; + repoFullName: string; + commitSha: string; + frozenAt: string; + openPullRequests: FrozenWorkUnit[]; + openIssues: FrozenWorkUnit[]; + recentDecisions: FrozenDecision[]; + /** sha256 over the canonicalized snapshot WITHOUT this field — so it commits to its own content and a + * third party can confirm two runs scored the same task. */ + snapshotChecksum: string; +}; + +/** + * The single definition of "visible at T": at or before `frozenAt`, inclusive. + * + * An unparseable timestamp is NOT visible. That direction is deliberate and is the fail-safe one: a record + * whose date cannot be read might be from after T, and admitting it would risk a leak, while excluding it + * only costs a task some context. Every filter in this module routes through here, so the boundary cannot + * drift between fields. + */ +export function atOrBefore(timestamp: string | null | undefined, frozenAt: string): boolean { + if (!timestamp) return false; + const at = Date.parse(timestamp); + const cutoff = Date.parse(frozenAt); + if (!Number.isFinite(at) || !Number.isFinite(cutoff)) return false; + return at <= cutoff; +} + +/** Was this unit still OPEN at T? Closed strictly after T means it was open at T; closed at or before T + * means it was already closed and is not part of the task. Never closed at all ⇒ open. */ +export function wasOpenAt(record: RawWorkUnitRecord, frozenAt: string): boolean { + if (!atOrBefore(record.createdAt, frozenAt)) return false; // not created yet at T + if (!record.closedAt) return true; + // Closed at or before T ⇒ not open at T. An unparseable closedAt is treated as "still open", which is + // the safe direction here: it keeps a question in the task rather than leaking a resolution. + const closedAt = Date.parse(record.closedAt); + if (!Number.isFinite(closedAt)) return true; + return closedAt > Date.parse(frozenAt); +} + +/** Project one raw record onto its state at T. Labels applied after T are dropped; the outcome fields are + * not carried at all. Sorting makes the output canonical, which is what lets the checksum be stable. */ +export function freezeWorkUnit(record: RawWorkUnitRecord, frozenAt: string): FrozenWorkUnit { + const labels = (record.labels ?? []) + .filter((label) => atOrBefore(label.appliedAt, frozenAt)) + .map((label) => label.name) + .filter((name) => name.length > 0); + return { + workUnitId: record.workUnitId, + number: record.number, + kind: record.kind, + title: record.title, + body: record.body, + authorLogin: record.authorLogin, + createdAt: record.createdAt, + labels: [...new Set(labels)].sort(), + changedPaths: [...new Set(record.changedPaths ?? [])].sort(), + }; +} + +/** Total-order fallback: compare the canonicalized JSON. Reached only when every named key ties, and it + * guarantees the sort is total no matter what fields the record type grows. */ +function compareCanonical(a: object, b: object): number { + const left = JSON.stringify(canonicalize(a as Record)); + const right = JSON.stringify(canonicalize(b as Record)); + return left < right ? -1 : left > right ? 1 : 0; +} + +/** Sort key for every collection in a snapshot: the work-unit id, which is unique and stable. Sorting + * rather than preserving input order is what makes two builds from differently-ordered reads identical. */ +function byWorkUnitId(a: T, b: T): number { + return a.workUnitId < b.workUnitId ? -1 : a.workUnitId > b.workUnitId ? 1 : 0; +} + +/** Canonicalize with sorted keys — mirrors backtest-corpus-export-core.ts's `canonicalizeCase`. Two-armed + * rather than the usual three: object keys are unique by definition, so an "equal keys" arm would be dead + * code that no test could ever reach. */ +function canonicalize(value: Record): Record { + return Object.fromEntries(Object.entries(value).sort(([a], [b]) => (a < b ? -1 : 1))); +} + +/** + * Deterministic SHA-256 over the canonicalized snapshot body (every field except the checksum itself). + * + * The committed fields are listed EXPLICITLY rather than spread from the argument. Spreading made this a + * foot-gun: handing it a whole `FrozenRepoSnapshot` (the natural thing to do when re-verifying) silently + * folded the existing `snapshotChecksum` into the preimage and returned a different, wrong digest with no + * error. Naming the fields makes the function total over both shapes and makes an added field a compile + * error here — which is the right place to notice that the commitment needs updating. + */ +export function checksumSnapshot(snapshot: Omit): string { + const canonical = canonicalize({ + schemaVersion: snapshot.schemaVersion, + repoFullName: snapshot.repoFullName, + commitSha: snapshot.commitSha, + frozenAt: snapshot.frozenAt, + openPullRequests: snapshot.openPullRequests.map((unit) => canonicalize(unit as unknown as Record)), + openIssues: snapshot.openIssues.map((unit) => canonicalize(unit as unknown as Record)), + recentDecisions: snapshot.recentDecisions.map((decision) => canonicalize(decision as unknown as Record)), + }); + return createHash("sha256").update(JSON.stringify(canonical)).digest("hex"); +} + +/** + * Build a leak-proof snapshot of a repo at commit T. + * + * Every collection is filtered through {@link atOrBefore} and sorted, so the result is a pure function of + * (records, frozenAt) — never of when the build ran, nor of the order the CLI happened to read records in. + * That is the property the benchmark's reproducibility rests on, and it is asserted directly in the tests. + */ +export function buildFrozenRepoSnapshot(input: { + repoFullName: string; + commitSha: string; + frozenAt: string; + workUnits: readonly RawWorkUnitRecord[]; + decisions?: readonly RawDecisionRecord[] | undefined; +}): FrozenRepoSnapshot { + const openAtT = input.workUnits.filter((record) => wasOpenAt(record, input.frozenAt)); + const openPullRequests = openAtT + .filter((record) => record.kind === "pull_request") + .map((record) => freezeWorkUnit(record, input.frozenAt)) + .sort(byWorkUnitId); + const openIssues = openAtT + .filter((record) => record.kind === "issue") + .map((record) => freezeWorkUnit(record, input.frozenAt)) + .sort(byWorkUnitId); + const recentDecisions = (input.decisions ?? []) + .filter((decision) => atOrBefore(decision.decidedAt, input.frozenAt)) + .map((decision) => ({ + workUnitId: decision.workUnitId, + action: decision.action, + reasonCode: decision.reasonCode, + decidedAt: decision.decidedAt, + })) + // Decisions can repeat per work unit, so the id alone is not a total order. The tie-break chain must + // cover EVERY field, not just the obvious ones: two decisions differing only in `reasonCode` compared + // equal under an earlier version of this sort, so their relative order followed input order and the + // snapshot checksum moved when the CLI happened to read them the other way round -- which is exactly + // the reproducibility property this module exists to guarantee. Comparing the canonical serialization + // last makes the order total by construction, so adding a field to FrozenDecision cannot silently + // reintroduce the same hole. + .sort( + (a, b) => + byWorkUnitId(a, b) || + (a.decidedAt < b.decidedAt ? -1 : a.decidedAt > b.decidedAt ? 1 : 0) || + (a.action < b.action ? -1 : a.action > b.action ? 1 : 0) || + compareCanonical(a, b), + ); + + const body: Omit = { + schemaVersion: FROZEN_REPO_SNAPSHOT_SCHEMA_VERSION, + repoFullName: input.repoFullName, + commitSha: input.commitSha, + frozenAt: input.frozenAt, + openPullRequests, + openIssues, + recentDecisions, + }; + return { ...body, snapshotChecksum: checksumSnapshot(body) }; +} + +/** Recompute a snapshot's checksum and compare — the exact check a third party runs to confirm two runs + * scored the same task, exported so our tests exercise the SAME path rather than a parallel one. */ +export function verifySnapshotChecksum(snapshot: FrozenRepoSnapshot): boolean { + const { snapshotChecksum, ...body } = snapshot; + return checksumSnapshot(body) === snapshotChecksum; +} + +/** + * Audit a built snapshot for future information — a belt-and-braces check over the builder's own output. + * + * The builder already excludes post-T data by construction; this re-derives the property independently, so + * a future refactor that breaks the filtering fails loudly here instead of silently inflating scores. It + * returns the offending paths rather than a bare boolean, because "which field leaked" is the only useful + * form of that answer. + */ +export function auditSnapshotForLeaks(snapshot: FrozenRepoSnapshot): string[] { + const leaks: string[] = []; + const check = (collection: "openPullRequests" | "openIssues") => { + for (const unit of snapshot[collection]) { + if (!atOrBefore(unit.createdAt, snapshot.frozenAt)) leaks.push(`${collection}/${unit.workUnitId}: createdAt is after frozenAt`); + // The snapshot type carries no outcome fields, but a hand-built or deserialized object could. + for (const forbidden of ["closedAt", "mergedAt", "state", "merged"]) { + if (forbidden in (unit as unknown as Record)) leaks.push(`${collection}/${unit.workUnitId}: carries outcome field "${forbidden}"`); + } + } + }; + check("openPullRequests"); + check("openIssues"); + for (const decision of snapshot.recentDecisions) { + if (!atOrBefore(decision.decidedAt, snapshot.frozenAt)) leaks.push(`recentDecisions/${decision.workUnitId}: decidedAt is after frozenAt`); + } + return leaks; +} diff --git a/scripts/frozen-repo-snapshot.ts b/scripts/frozen-repo-snapshot.ts new file mode 100644 index 0000000000..1c2c58aa26 --- /dev/null +++ b/scripts/frozen-repo-snapshot.ts @@ -0,0 +1,162 @@ +#!/usr/bin/env node +// Frozen-repo snapshot CLI (#9259, harness #9216, epic #8534) — the thin IO wrapper around +// frozen-repo-snapshot-core.ts, mirroring backtest-corpus-export.ts's shape exactly (pure transform in a +// -core module, this file does the reads and the write). +// +// tsx scripts/frozen-repo-snapshot.ts --repo owner/name --sha --frozen-at --output +// +// Reads PRs/issues from the GitHub REST API (read-only; GITHUB_TOKEN for private repos or rate limits) and +// past gate decisions from the local/remote D1 via `wrangler d1 execute --json`, then hands EVERYTHING to +// buildFrozenRepoSnapshot, which does all filtering. This file deliberately performs NO date filtering of +// its own beyond what the API needs for paging: one filtering implementation, in the tested pure core, is +// what keeps a leak from hiding in an untested CLI branch. +import { writeFileSync } from "node:fs"; +import { spawnSync } from "node:child_process"; +import { + buildFrozenRepoSnapshot, + auditSnapshotForLeaks, + type RawDecisionRecord, + type RawWorkUnitRecord, + type TimestampedLabel, +} from "./frozen-repo-snapshot-core"; + +type Args = { repo: string | undefined; sha: string | undefined; frozenAt: string | undefined; output: string | undefined; remote: boolean; db: string }; + +export function parseArgs(argv: readonly string[]): Args { + const args: Args = { repo: undefined, sha: undefined, frozenAt: undefined, output: undefined, remote: false, db: "loopover" }; + for (let index = 0; index < argv.length; index += 1) { + const flag = argv[index]; + const value = argv[index + 1]; + if (flag === "--repo") args.repo = value; + else if (flag === "--sha") args.sha = value; + else if (flag === "--frozen-at") args.frozenAt = value; + else if (flag === "--output") args.output = value; + else if (flag === "--remote") args.remote = true; + else if (flag === "--db" && value) args.db = value; + } + return args; +} + +/** Every required flag, named individually so a user fixes all of them in one pass rather than one per run. */ +export function missingArgs(args: Args): string[] { + const missing: string[] = []; + if (!args.repo) missing.push("--repo"); + if (!args.sha) missing.push("--sha"); + if (!args.frozenAt) missing.push("--frozen-at"); + if (!args.output) missing.push("--output"); + return missing; +} + +type GitHubIssue = { + number: number; + title: string; + body?: string | null; + user?: { login?: string } | null; + created_at: string; + closed_at?: string | null; + pull_request?: unknown; + labels?: Array<{ name?: string }> | null; +}; + +type GitHubIssueEvent = { event?: string; created_at?: string; label?: { name?: string } }; + +async function fetchJson(url: string, token: string | undefined): Promise { + const response = await fetch(url, { + headers: { + accept: "application/vnd.github+json", + "user-agent": "loopover-frozen-repo-snapshot", + ...(token ? { authorization: `Bearer ${token}` } : {}), + }, + }); + if (!response.ok) throw new Error(`GitHub ${response.status} for ${url}`); + return (await response.json()) as T; +} + +/** Label APPLICATION times come from the issue-events timeline — a label object alone carries no timestamp, + * so without this every label would be unfilterable and a post-T label would leak into the snapshot. */ +async function fetchLabelHistory(repo: string, number: number, token: string | undefined): Promise { + const events = await fetchJson(`https://api.github.com/repos/${repo}/issues/${number}/events?per_page=100`, token); + return events + .filter((event) => event.event === "labeled" && typeof event.label?.name === "string" && typeof event.created_at === "string") + .map((event) => ({ name: String(event.label?.name), appliedAt: String(event.created_at) })); +} + +function d1Query(sql: string, remote: boolean, db: string): Array> { + const result = spawnSync( + "npx", + ["wrangler", "d1", "execute", db, ...(remote ? ["--remote"] : ["--local"]), "--json", "--command", sql], + { encoding: "utf8" }, + ); + if (result.status !== 0) throw new Error(`wrangler d1 execute failed: ${result.stderr || result.stdout}`); + const parsed = JSON.parse(result.stdout) as Array<{ results?: Array> }>; + return parsed[0]?.results ?? []; +} + +async function main(): Promise { + const args = parseArgs(process.argv.slice(2)); + const missing = missingArgs(args); + if (missing.length > 0) { + console.error(`frozen-repo-snapshot: missing required flag(s): ${missing.join(", ")}`); + process.exit(1); + } + const repo = String(args.repo); + const token = process.env.GITHUB_TOKEN; + + // `state=all` deliberately: whether a unit was OPEN at T is decided by the pure core from createdAt and + // closedAt, not by GitHub's CURRENT state -- asking for open-only would silently drop every PR that has + // closed since T, which is most of them for any historical snapshot. + const issues = await fetchJson(`https://api.github.com/repos/${repo}/issues?state=all&per_page=100`, token); + + const workUnits: RawWorkUnitRecord[] = []; + for (const issue of issues) { + const labels = await fetchLabelHistory(repo, issue.number, token).catch(() => [] as TimestampedLabel[]); + workUnits.push({ + workUnitId: `${repo}#${issue.number}`, + number: issue.number, + kind: issue.pull_request ? "pull_request" : "issue", + title: issue.title, + body: issue.body ?? "", + authorLogin: issue.user?.login ?? "", + createdAt: issue.created_at, + closedAt: issue.closed_at ?? null, + labels, + }); + } + + const decisionRows = d1Query( + `SELECT repo_full_name, pull_number, action, reason_code, created_at FROM decision_records WHERE repo_full_name = '${repo.replace(/'/g, "''")}'`, + args.remote, + args.db, + ); + const decisions: RawDecisionRecord[] = decisionRows.map((row) => ({ + workUnitId: `${String(row["repo_full_name"])}#${String(row["pull_number"])}`, + action: String(row["action"]), + reasonCode: String(row["reason_code"]), + decidedAt: String(row["created_at"]), + })); + + const snapshot = buildFrozenRepoSnapshot({ + repoFullName: repo, + commitSha: String(args.sha), + frozenAt: String(args.frozenAt), + workUnits, + decisions, + }); + + // Fail LOUD rather than writing a snapshot that leaks: an inflated benchmark is worse than no benchmark, + // because its numbers look fine. The audit re-derives the property independently of the builder. + const leaks = auditSnapshotForLeaks(snapshot); + if (leaks.length > 0) { + console.error(`frozen-repo-snapshot: REFUSING to write a snapshot with future information:\n ${leaks.join("\n ")}`); + process.exit(1); + } + + writeFileSync(String(args.output), `${JSON.stringify(snapshot, null, 2)}\n`); + console.log( + `frozen-repo-snapshot: wrote ${args.output} — ${snapshot.openPullRequests.length} open PR(s), ${snapshot.openIssues.length} open issue(s), ${snapshot.recentDecisions.length} prior decision(s), checksum ${snapshot.snapshotChecksum.slice(0, 12)}…`, + ); +} + +if (process.argv[1] && import.meta.url.endsWith(process.argv[1].split("/").pop() ?? "")) { + await main(); +} diff --git a/test/unit/frozen-repo-snapshot-core.test.ts b/test/unit/frozen-repo-snapshot-core.test.ts new file mode 100644 index 0000000000..99f7b76b17 --- /dev/null +++ b/test/unit/frozen-repo-snapshot-core.test.ts @@ -0,0 +1,297 @@ +import { describe, expect, it } from "vitest"; +import { + atOrBefore, + auditSnapshotForLeaks, + buildFrozenRepoSnapshot, + checksumSnapshot, + freezeWorkUnit, + FROZEN_REPO_SNAPSHOT_SCHEMA_VERSION, + verifySnapshotChecksum, + wasOpenAt, + type FrozenRepoSnapshot, + type RawWorkUnitRecord, +} from "../../scripts/frozen-repo-snapshot-core"; +import { parseArgs, missingArgs } from "../../scripts/frozen-repo-snapshot"; + +// #9259 (harness #9216, epic #8534): leak-proofing IS the deliverable. A snapshot that leaks future +// information still produces perfectly well-formed numbers downstream, so the failure is silent — which is +// why these invariants are asserted directly rather than trusted to the builder's structure. + +const T = "2026-07-01T00:00:00.000Z"; + +function pr(number: number, overrides: Partial = {}): RawWorkUnitRecord { + return { + workUnitId: `o/r#${number}`, + number, + kind: "pull_request", + title: `PR ${number}`, + body: "body", + authorLogin: "contributor", + createdAt: "2026-06-01T00:00:00.000Z", + closedAt: null, + ...overrides, + }; +} + +describe("atOrBefore — the single definition of 'visible at T' (#9259)", () => { + it("is INCLUSIVE at T: what happened at the freeze instant is what the maintainer could see", () => { + expect(atOrBefore(T, T)).toBe(true); + expect(atOrBefore("2026-06-30T23:59:59.999Z", T)).toBe(true); + expect(atOrBefore("2026-07-01T00:00:00.001Z", T)).toBe(false); + }); + + it("INVARIANT: an unreadable or absent timestamp is NOT visible — the fail-safe direction", () => { + // A record whose date cannot be read might be from after T; admitting it risks a leak, excluding it + // only costs context. The safe direction is the one that cannot inflate a score. + for (const bad of [undefined, null, "", "yesterday", "2026-13-45T99:99:99Z"]) { + expect(atOrBefore(bad, T)).toBe(false); + } + // An unreadable CUTOFF is equally fail-safe — nothing is visible against a cutoff nobody can parse. + expect(atOrBefore("2026-06-01T00:00:00.000Z", "not a date")).toBe(false); + }); +}); + +describe("wasOpenAt (#9259)", () => { + it("open at T means created at-or-before T and not yet closed at T", () => { + expect(wasOpenAt(pr(1), T)).toBe(true); + expect(wasOpenAt(pr(2, { closedAt: "2026-08-01T00:00:00.000Z" }), T)).toBe(true); // closed AFTER T + expect(wasOpenAt(pr(3, { closedAt: "2026-06-15T00:00:00.000Z" }), T)).toBe(false); // already closed + expect(wasOpenAt(pr(4, { closedAt: T }), T)).toBe(false); // closed exactly at T + expect(wasOpenAt(pr(5, { createdAt: "2026-07-02T00:00:00.000Z" }), T)).toBe(false); // not created yet + }); + + it("an unreadable closedAt keeps the unit OPEN — that keeps a question in the task, never leaks an answer", () => { + expect(wasOpenAt(pr(6, { closedAt: "sometime" }), T)).toBe(true); + }); +}); + +describe("freezeWorkUnit (#9259)", () => { + it("REGRESSION: labels applied AFTER T are dropped; labels applied at-or-before T survive", () => { + const frozen = freezeWorkUnit( + pr(1, { + labels: [ + { name: "before", appliedAt: "2026-06-10T00:00:00.000Z" }, + { name: "at-T", appliedAt: T }, + { name: "after", appliedAt: "2026-07-05T00:00:00.000Z" }, + { name: "unparseable", appliedAt: "whenever" }, + ], + }), + T, + ); + expect(frozen.labels).toEqual(["at-T", "before"]); + }); + + it("carries label NAMES only — the application timestamps did their job and must not travel onward", () => { + const frozen = freezeWorkUnit(pr(1, { labels: [{ name: "bug", appliedAt: "2026-06-10T00:00:00.000Z" }] }), T); + expect(JSON.stringify(frozen)).not.toContain("2026-06-10"); + expect(frozen.labels).toEqual(["bug"]); + }); + + it("REGRESSION: the frozen unit carries NO outcome field — a snapshot holds the question, not the answer", () => { + const frozen = freezeWorkUnit(pr(1, { closedAt: "2026-08-01T00:00:00.000Z" }), T); + for (const forbidden of ["closedAt", "mergedAt", "state", "merged"]) { + expect(forbidden in frozen).toBe(false); + } + }); + + it("deduplicates and sorts labels and paths, so two reads in different orders canonicalize identically", () => { + const a = freezeWorkUnit( + pr(1, { + labels: [ + { name: "b", appliedAt: "2026-06-02T00:00:00.000Z" }, + { name: "a", appliedAt: "2026-06-01T00:00:00.000Z" }, + { name: "a", appliedAt: "2026-06-03T00:00:00.000Z" }, + ], + changedPaths: ["src/z.ts", "src/a.ts", "src/z.ts"], + }), + T, + ); + expect(a.labels).toEqual(["a", "b"]); + expect(a.changedPaths).toEqual(["src/a.ts", "src/z.ts"]); + // Absent optional collections become empty arrays, never undefined — the shape is uniform. + const bare = freezeWorkUnit(pr(2), T); + expect(bare.labels).toEqual([]); + expect(bare.changedPaths).toEqual([]); + }); +}); + +describe("buildFrozenRepoSnapshot — the leak-proofing invariants (#9259)", () => { + const base = { repoFullName: "o/r", commitSha: "abc123", frozenAt: T }; + + it("REGRESSION: a fixture full of post-T records produces a snapshot that provably excludes them", () => { + const snapshot = buildFrozenRepoSnapshot({ + ...base, + workUnits: [ + pr(1), // open at T — included + pr(2, { createdAt: "2026-07-10T00:00:00.000Z" }), // opened AFTER T + pr(3, { closedAt: "2026-06-20T00:00:00.000Z" }), // already resolved before T + pr(4, { kind: "issue", createdAt: "2026-06-05T00:00:00.000Z" }), // open issue at T + pr(5, { kind: "issue", createdAt: "2026-09-01T00:00:00.000Z" }), // issue opened AFTER T + ], + decisions: [ + { workUnitId: "o/r#1", action: "merge", reasonCode: "clean", decidedAt: "2026-06-25T00:00:00.000Z" }, + { workUnitId: "o/r#1", action: "close", reasonCode: "defect", decidedAt: "2026-07-20T00:00:00.000Z" }, // AFTER T + ], + }); + expect(snapshot.openPullRequests.map((unit) => unit.workUnitId)).toEqual(["o/r#1"]); + expect(snapshot.openIssues.map((unit) => unit.workUnitId)).toEqual(["o/r#4"]); + expect(snapshot.recentDecisions).toHaveLength(1); + expect(snapshot.recentDecisions[0]?.decidedAt).toBe("2026-06-25T00:00:00.000Z"); + // Nothing from the future appears ANYWHERE in the serialized snapshot, by any route. + const serialized = JSON.stringify(snapshot); + for (const future of ["2026-07-10", "2026-09-01", "2026-07-20", "PR 2", "PR 5"]) { + expect(serialized).not.toContain(future); + } + expect(auditSnapshotForLeaks(snapshot)).toEqual([]); + }); + + it("INVARIANT: two builds of the same snapshot produce an identical checksum, regardless of input order", () => { + const units = [pr(3), pr(1), pr(2, { kind: "issue" })]; + const decisions = [ + { workUnitId: "o/r#3", action: "merge", reasonCode: "clean", decidedAt: "2026-06-20T00:00:00.000Z" }, + { workUnitId: "o/r#1", action: "close", reasonCode: "defect", decidedAt: "2026-06-10T00:00:00.000Z" }, + ]; + const first = buildFrozenRepoSnapshot({ ...base, workUnits: units, decisions }); + const shuffled = buildFrozenRepoSnapshot({ ...base, workUnits: [...units].reverse(), decisions: [...decisions].reverse() }); + expect(shuffled.snapshotChecksum).toBe(first.snapshotChecksum); + expect(shuffled).toEqual(first); + expect(verifySnapshotChecksum(first)).toBe(true); + }); + + it("INVARIANT: a snapshot built at T never differs based on WHEN the build ran", () => { + // The builder reads no clock, so the only way this could fail is a hidden time dependency. Appending a + // year of post-T history — the difference between building on the day and building much later — must + // not move the checksum by one bit. + const sameDay = buildFrozenRepoSnapshot({ ...base, workUnits: [pr(1)], decisions: [] }); + const muchLater = buildFrozenRepoSnapshot({ + ...base, + workUnits: [pr(1), pr(9, { createdAt: "2027-06-01T00:00:00.000Z" }), pr(10, { kind: "issue", createdAt: "2027-07-01T00:00:00.000Z" })], + decisions: [{ workUnitId: "o/r#1", action: "merge", reasonCode: "clean", decidedAt: "2027-01-01T00:00:00.000Z" }], + }); + expect(muchLater.snapshotChecksum).toBe(sameDay.snapshotChecksum); + }); + + it("REGRESSION: the eventual OUTCOME of an included unit never enters the snapshot", () => { + // o/r#1 was merged three weeks after T. That is precisely what an agent is asked to predict, so a + // snapshot carrying it would be an answer key rather than a benchmark. + const snapshot = buildFrozenRepoSnapshot({ + ...base, + workUnits: [pr(1, { closedAt: "2026-07-21T00:00:00.000Z" })], + decisions: [], + }); + expect(snapshot.openPullRequests).toHaveLength(1); + expect(JSON.stringify(snapshot)).not.toContain("2026-07-21"); + expect(auditSnapshotForLeaks(snapshot)).toEqual([]); + }); + + it("a changed checksum follows any change to the content it commits to", () => { + const one = buildFrozenRepoSnapshot({ ...base, workUnits: [pr(1)], decisions: [] }); + for (const mutated of [ + buildFrozenRepoSnapshot({ ...base, commitSha: "different", workUnits: [pr(1)], decisions: [] }), + buildFrozenRepoSnapshot({ ...base, frozenAt: "2026-07-02T00:00:00.000Z", workUnits: [pr(1)], decisions: [] }), + buildFrozenRepoSnapshot({ ...base, workUnits: [pr(1, { title: "changed" })], decisions: [] }), + buildFrozenRepoSnapshot({ ...base, workUnits: [pr(1), pr(2)], decisions: [] }), + ]) { + expect(mutated.snapshotChecksum).not.toBe(one.snapshotChecksum); + } + }); + + it("an empty repo yields a well-formed, checksummed, empty snapshot rather than an error", () => { + const snapshot = buildFrozenRepoSnapshot({ ...base, workUnits: [] }); + expect(snapshot).toMatchObject({ + schemaVersion: FROZEN_REPO_SNAPSHOT_SCHEMA_VERSION, + openPullRequests: [], + openIssues: [], + recentDecisions: [], + }); + expect(verifySnapshotChecksum(snapshot)).toBe(true); + }); + + it("repeated decisions for one work unit sort deterministically by time then action", () => { + const decisions = [ + { workUnitId: "o/r#1", action: "merge", reasonCode: "b", decidedAt: "2026-06-10T00:00:00.000Z" }, + { workUnitId: "o/r#1", action: "close", reasonCode: "a", decidedAt: "2026-06-10T00:00:00.000Z" }, + { workUnitId: "o/r#1", action: "hold", reasonCode: "c", decidedAt: "2026-06-05T00:00:00.000Z" }, + ]; + const built = buildFrozenRepoSnapshot({ ...base, workUnits: [pr(1)], decisions }); + expect(built.recentDecisions.map((d) => `${d.decidedAt}/${d.action}`)).toEqual([ + "2026-06-05T00:00:00.000Z/hold", + "2026-06-10T00:00:00.000Z/close", + "2026-06-10T00:00:00.000Z/merge", + ]); + // Reversed input, identical output — the tie-break is total, not arrival-dependent. + expect(buildFrozenRepoSnapshot({ ...base, workUnits: [pr(1)], decisions: [...decisions].reverse() })).toEqual(built); + + // The FULL comparator chain: differing work units sort first, then time, then action. Includes a pair + // identical on all three keys, which must not reorder (and must not crash the sort). + const across = [ + { workUnitId: "o/r#2", action: "merge", reasonCode: "x", decidedAt: "2026-06-01T00:00:00.000Z" }, + { workUnitId: "o/r#1", action: "merge", reasonCode: "x", decidedAt: "2026-06-09T00:00:00.000Z" }, + { workUnitId: "o/r#1", action: "merge", reasonCode: "y", decidedAt: "2026-06-09T00:00:00.000Z" }, + // A byte-for-byte DUPLICATE: every tie-break key ties, including the canonical fallback. It must + // survive (a repeated read is not the builder's to deduplicate) and must not destabilize the order. + { workUnitId: "o/r#1", action: "merge", reasonCode: "y", decidedAt: "2026-06-09T00:00:00.000Z" }, + { workUnitId: "o/r#1", action: "close", reasonCode: "z", decidedAt: "2026-06-02T00:00:00.000Z" }, + ]; + const ordered = buildFrozenRepoSnapshot({ ...base, workUnits: [pr(1), pr(2)], decisions: across }); + expect(ordered.recentDecisions.map((d) => `${d.workUnitId}/${d.decidedAt.slice(8, 10)}/${d.action}/${d.reasonCode}`)).toEqual([ + "o/r#1/02/close/z", + "o/r#1/09/merge/x", + "o/r#1/09/merge/y", + "o/r#1/09/merge/y", + "o/r#2/01/merge/x", + ]); + expect(buildFrozenRepoSnapshot({ ...base, workUnits: [pr(1), pr(2)], decisions: [...across].reverse() }).snapshotChecksum) + .toBe(ordered.snapshotChecksum); + }); +}); + +describe("verifySnapshotChecksum / auditSnapshotForLeaks (#9259)", () => { + const built = buildFrozenRepoSnapshot({ repoFullName: "o/r", commitSha: "abc", frozenAt: T, workUnits: [pr(1)] }); + + it("a tampered snapshot fails checksum verification", () => { + expect(verifySnapshotChecksum({ ...built, commitSha: "tampered" })).toBe(false); + expect(verifySnapshotChecksum({ ...built, snapshotChecksum: "0".repeat(64) })).toBe(false); + // REGRESSION: passing the WHOLE snapshot (the natural re-verification call) must give the same digest + // as passing the body — an earlier spread-based implementation silently folded the existing checksum + // into its own preimage and returned a wrong answer with no error. + expect(checksumSnapshot(built)).toBe(built.snapshotChecksum); + const { snapshotChecksum: _drop, ...body } = built; + expect(checksumSnapshot(body)).toBe(built.snapshotChecksum); + }); + + it("REGRESSION: the audit catches every leak shape a hand-built or deserialized snapshot could carry", () => { + const futureUnit = { ...built.openPullRequests[0], workUnitId: "o/r#99", createdAt: "2027-01-01T00:00:00.000Z" }; + const outcomeUnit = { ...built.openPullRequests[0], closedAt: "2026-08-01T00:00:00.000Z" }; + const leaky: FrozenRepoSnapshot = { + ...built, + openPullRequests: [futureUnit as never, outcomeUnit as never], + openIssues: [{ ...built.openPullRequests[0], workUnitId: "o/r#98", kind: "issue", createdAt: "2027-02-02T00:00:00.000Z" } as never], + recentDecisions: [{ workUnitId: "o/r#1", action: "merge", reasonCode: "clean", decidedAt: "2027-03-03T00:00:00.000Z" }], + }; + const leaks = auditSnapshotForLeaks(leaky); + expect(leaks).toHaveLength(4); + expect(leaks.some((leak) => leak.includes("o/r#99") && leak.includes("createdAt is after frozenAt"))).toBe(true); + expect(leaks.some((leak) => leak.includes('carries outcome field "closedAt"'))).toBe(true); + expect(leaks.some((leak) => leak.startsWith("openIssues/o/r#98"))).toBe(true); + expect(leaks.some((leak) => leak.startsWith("recentDecisions/o/r#1"))).toBe(true); + }); +}); + +describe("frozen-repo-snapshot CLI arg handling (#9259)", () => { + it("parses every flag, defaults --db and --remote, and names each missing required flag", () => { + const full = parseArgs(["--repo", "o/r", "--sha", "abc", "--frozen-at", T, "--output", "s.json", "--remote", "--db", "other"]); + expect(full).toEqual({ repo: "o/r", sha: "abc", frozenAt: T, output: "s.json", remote: true, db: "other" }); + expect(missingArgs(full)).toEqual([]); + + const bare = parseArgs([]); + expect(bare).toMatchObject({ remote: false, db: "loopover" }); + // All four named at once, so a user fixes them in a single pass rather than one run per flag. + expect(missingArgs(bare)).toEqual(["--repo", "--sha", "--frozen-at", "--output"]); + expect(missingArgs(parseArgs(["--repo", "o/r", "--sha", "abc"]))).toEqual(["--frozen-at", "--output"]); + // A trailing --db with no value keeps the default rather than storing undefined. + expect(parseArgs(["--db"]).db).toBe("loopover"); + // An unrecognized flag is ignored rather than throwing. + expect(parseArgs(["--nonsense", "x", "--repo", "o/r"]).repo).toBe("o/r"); + }); +}); From 46909601c4a7a504f851a27c56b1e65d053950df Mon Sep 17 00:00:00 2001 From: JSONbored <49853598+JSONbored@users.noreply.github.com> Date: Tue, 28 Jul 2026 21:03:56 -0700 Subject: [PATCH 2/2] fix(benchmark): paginate the snapshot CLI's GitHub reads, and refuse a truncated snapshot Review caught both list reads stopping at page one. The consequence is worse than incompleteness: the snapshot's whole value is that two runs over the same (repo, T) produce the same checksum, and a truncated read makes that checksum a function of how many records the repo happened to have -- so the same T could yield two different "authoritative" snapshots. And it is silent, because the output is a perfectly well-formed snapshot that is simply missing history. - fetchAllPages reads to exhaustion, stopping at the first short page. - Both call sites use it: the issue/PR list, and per-issue label history. The second is the sharper of the two -- dropping early `labeled` events omits labels that were genuinely applied before T, which makes the snapshot WRONG rather than merely thinner. - Hitting the page bound is REPORTED, not silently accepted, and the CLI refuses to write a truncated snapshot -- the same posture as the leak audit, for the same reason: a snapshot nobody can reproduce is not one worth publishing. - A read error propagates instead of being swallowed into a short list that looks complete. The gap survived because the pagination lived inline in an untested CLI, so fetchAllPages is now an injectable seam with five tests covering multi-page reads, the short-page stop, the exactly-full-last-page extra read, empty/single-page sources, the bound, and error propagation. --- scripts/frozen-repo-snapshot.ts | 82 ++++++++++++++++++--- test/unit/frozen-repo-snapshot-core.test.ts | 76 ++++++++++++++++++- 2 files changed, 148 insertions(+), 10 deletions(-) diff --git a/scripts/frozen-repo-snapshot.ts b/scripts/frozen-repo-snapshot.ts index 1c2c58aa26..405d81fe7d 100644 --- a/scripts/frozen-repo-snapshot.ts +++ b/scripts/frozen-repo-snapshot.ts @@ -72,13 +72,59 @@ async function fetchJson(url: string, token: string | undefined): Promise return (await response.json()) as T; } +export const GITHUB_PER_PAGE = 100; +/** Hard stop, so a pathological repo cannot spin forever. 200 pages x 100 = 20k records, comfortably past + * any real benchmark repo; exceeding it is REPORTED, never silently accepted (see below). */ +export const GITHUB_MAX_PAGES = 200; + +/** + * Read every page of a GitHub list endpoint. + * + * A single `per_page=100` read is not merely incomplete on a large repo -- it is NON-REPRODUCIBLE, which is + * worse for this tool specifically. The snapshot's whole value is that two runs over the same (repo, T) + * produce the same checksum; a truncated read makes the checksum a function of how many records the repo + * happened to have, so the same T could yield two different "authoritative" snapshots. And the truncation is + * silent: the output is a perfectly well-formed snapshot that is simply missing history. + * + * So this pages to exhaustion and reports `truncated` when it hits the bound rather than returning a short + * list as if it were complete. The caller REFUSES to write a truncated snapshot, the same posture as the + * leak audit -- a snapshot nobody can reproduce is not one worth publishing. + */ +export async function fetchAllPages( + pageUrl: (page: number) => string, + readPage: (url: string) => Promise, + maxPages: number = GITHUB_MAX_PAGES, +): Promise<{ items: T[]; truncated: boolean }> { + const items: T[] = []; + for (let page = 1; page <= maxPages; page += 1) { + const batch = await readPage(pageUrl(page)); + items.push(...batch); + // A short page is the last page. An exactly-full final page costs one extra empty request, which is the + // correct trade against guessing the end from a count the API does not promise. + if (batch.length < GITHUB_PER_PAGE) return { items, truncated: false }; + } + return { items, truncated: true }; +} + /** Label APPLICATION times come from the issue-events timeline — a label object alone carries no timestamp, - * so without this every label would be unfilterable and a post-T label would leak into the snapshot. */ -async function fetchLabelHistory(repo: string, number: number, token: string | undefined): Promise { - const events = await fetchJson(`https://api.github.com/repos/${repo}/issues/${number}/events?per_page=100`, token); - return events - .filter((event) => event.event === "labeled" && typeof event.label?.name === "string" && typeof event.created_at === "string") - .map((event) => ({ name: String(event.label?.name), appliedAt: String(event.created_at) })); + * so without this every label would be unfilterable and a post-T label would leak into the snapshot. + * Paginated for the same reason as the issue list: a long-lived issue can carry well over 100 events, and + * dropping the earlier ones would silently omit labels that were genuinely applied before T. */ +async function fetchLabelHistory( + repo: string, + number: number, + token: string | undefined, +): Promise<{ labels: TimestampedLabel[]; truncated: boolean }> { + const { items, truncated } = await fetchAllPages( + (page) => `https://api.github.com/repos/${repo}/issues/${number}/events?per_page=${GITHUB_PER_PAGE}&page=${page}`, + (url) => fetchJson(url, token), + ); + return { + labels: items + .filter((event) => event.event === "labeled" && typeof event.label?.name === "string" && typeof event.created_at === "string") + .map((event) => ({ name: String(event.label?.name), appliedAt: String(event.created_at) })), + truncated, + }; } function d1Query(sql: string, remote: boolean, db: string): Array> { @@ -105,11 +151,21 @@ async function main(): Promise { // `state=all` deliberately: whether a unit was OPEN at T is decided by the pure core from createdAt and // closedAt, not by GitHub's CURRENT state -- asking for open-only would silently drop every PR that has // closed since T, which is most of them for any historical snapshot. - const issues = await fetchJson(`https://api.github.com/repos/${repo}/issues?state=all&per_page=100`, token); + const issuePages = await fetchAllPages( + (page) => `https://api.github.com/repos/${repo}/issues?state=all&per_page=${GITHUB_PER_PAGE}&page=${page}`, + (url) => fetchJson(url, token), + ); + const truncations: string[] = []; + if (issuePages.truncated) truncations.push(`issue list exceeded ${GITHUB_MAX_PAGES} pages`); const workUnits: RawWorkUnitRecord[] = []; - for (const issue of issues) { - const labels = await fetchLabelHistory(repo, issue.number, token).catch(() => [] as TimestampedLabel[]); + for (const issue of issuePages.items) { + // A label-history read that FAILS degrades to no labels (the snapshot loses context but stays honest); + // one that TRUNCATES is recorded, because missing an early `labeled` event silently omits a label that + // was genuinely applied before T -- a wrong snapshot rather than a thinner one. + const history = await fetchLabelHistory(repo, issue.number, token).catch(() => ({ labels: [] as TimestampedLabel[], truncated: false })); + if (history.truncated) truncations.push(`#${issue.number} label history exceeded ${GITHUB_MAX_PAGES} pages`); + const labels = history.labels; workUnits.push({ workUnitId: `${repo}#${issue.number}`, number: issue.number, @@ -151,6 +207,14 @@ async function main(): Promise { process.exit(1); } + // Same posture as the leak refusal: a truncated read produces a well-formed snapshot that is simply + // missing history, and whose checksum therefore depends on how much the reader happened to see. That is + // not a snapshot anyone can reproduce, so it is not one worth writing. + if (truncations.length > 0) { + console.error(`frozen-repo-snapshot: REFUSING to write a snapshot from a truncated read:\n ${truncations.join("\n ")}`); + process.exit(1); + } + writeFileSync(String(args.output), `${JSON.stringify(snapshot, null, 2)}\n`); console.log( `frozen-repo-snapshot: wrote ${args.output} — ${snapshot.openPullRequests.length} open PR(s), ${snapshot.openIssues.length} open issue(s), ${snapshot.recentDecisions.length} prior decision(s), checksum ${snapshot.snapshotChecksum.slice(0, 12)}…`, diff --git a/test/unit/frozen-repo-snapshot-core.test.ts b/test/unit/frozen-repo-snapshot-core.test.ts index 99f7b76b17..5782f6809e 100644 --- a/test/unit/frozen-repo-snapshot-core.test.ts +++ b/test/unit/frozen-repo-snapshot-core.test.ts @@ -11,7 +11,7 @@ import { type FrozenRepoSnapshot, type RawWorkUnitRecord, } from "../../scripts/frozen-repo-snapshot-core"; -import { parseArgs, missingArgs } from "../../scripts/frozen-repo-snapshot"; +import { fetchAllPages, GITHUB_MAX_PAGES, GITHUB_PER_PAGE, missingArgs, parseArgs } from "../../scripts/frozen-repo-snapshot"; // #9259 (harness #9216, epic #8534): leak-proofing IS the deliverable. A snapshot that leaks future // information still produces perfectly well-formed numbers downstream, so the failure is silent — which is @@ -295,3 +295,77 @@ describe("frozen-repo-snapshot CLI arg handling (#9259)", () => { expect(parseArgs(["--nonsense", "x", "--repo", "o/r"]).repo).toBe("o/r"); }); }); + +describe("fetchAllPages — the CLI's pagination (#9259)", () => { + /** A fake list endpoint holding `total` records, paged the way GitHub pages. */ + function pagedSource(total: number): { read: (url: string) => Promise; calls: string[] } { + const calls: string[] = []; + return { + calls, + read: async (url: string) => { + calls.push(url); + const page = Number(new URL(url, "https://x.test").searchParams.get("page")); + const start = (page - 1) * GITHUB_PER_PAGE; + return Array.from({ length: Math.max(0, Math.min(GITHUB_PER_PAGE, total - start)) }, (_, index) => start + index); + }, + }; + } + + const url = (page: number) => `https://api.github.com/list?per_page=${GITHUB_PER_PAGE}&page=${page}`; + + it("REGRESSION: reads EVERY page — a single per_page=100 request silently dropped older records", () => { + // The defect this test exists for: 250 records used to come back as 100, producing a well-formed but + // incomplete snapshot whose checksum depended on how much the reader happened to see. + return (async () => { + const source = pagedSource(250); + const result = await fetchAllPages(url, source.read); + expect(result.truncated).toBe(false); + expect(result.items).toHaveLength(250); + expect(result.items[0]).toBe(0); + expect(result.items[249]).toBe(249); + expect(source.calls).toHaveLength(3); + })(); + }); + + it("stops at the first SHORT page, and an exactly-full last page costs one extra empty read", async () => { + const short = pagedSource(150); + expect((await fetchAllPages(url, short.read)).items).toHaveLength(150); + expect(short.calls).toHaveLength(2); + // Exactly 200: pages 1 and 2 are both full, so page 3 confirms the end. One wasted request beats + // guessing the end from a count the API does not promise. + const exact = pagedSource(200); + const result = await fetchAllPages(url, exact.read); + expect(result.items).toHaveLength(200); + expect(result.truncated).toBe(false); + expect(exact.calls).toHaveLength(3); + }); + + it("an empty and a single-page source both read exactly once", async () => { + for (const total of [0, 1, GITHUB_PER_PAGE - 1]) { + const source = pagedSource(total); + const result = await fetchAllPages(url, source.read); + expect(result).toMatchObject({ truncated: false }); + expect(result.items).toHaveLength(total); + expect(source.calls).toHaveLength(1); + } + }); + + it("REGRESSION: hitting the page bound REPORTS truncation rather than returning a short list as complete", async () => { + // Every page is full, so the loop never sees an end. The bound must surface as `truncated`, which the + // CLI turns into a refusal to write — a snapshot nobody can reproduce is not worth publishing. + const endless = { read: async () => Array.from({ length: GITHUB_PER_PAGE }, (_, index) => index) }; + const result = await fetchAllPages(url, endless.read, 3); + expect(result.truncated).toBe(true); + expect(result.items).toHaveLength(3 * GITHUB_PER_PAGE); + // The real bound is generous enough that no honest repo reaches it. + expect(GITHUB_MAX_PAGES * GITHUB_PER_PAGE).toBeGreaterThanOrEqual(20_000); + }); + + it("a read error propagates rather than being swallowed into a short, complete-looking list", async () => { + const failing = async (url: string) => { + if (url.includes("page=2")) throw new Error("GitHub 502"); + return Array.from({ length: GITHUB_PER_PAGE }, (_, index) => index); + }; + await expect(fetchAllPages(url, failing)).rejects.toThrow("GitHub 502"); + }); +});