From bf26e14324a64506d0e6089f3ce6e209c8dec3da Mon Sep 17 00:00:00 2001 From: Shane Loeffler Date: Thu, 15 Oct 2026 18:40:13 -0500 Subject: [PATCH 1/2] Walk v2 snapshot history via repo file parent offsets v2 snapshot files carry no parent pointer, so walkHistory stopped after the head snapshot on every v2 repo. Resolve parents from the repo file's parent_offset indices, reloading the repo info once when a snapshot is newer than the cached copy. Reject corrupt offsets and cyclic ancestry. --- src/format/flatbuffers/repo-parser.ts | 46 +++++++++++ src/reader/repository.ts | 42 +++++++++- tests/format/flatbuffers/repo-parser.test.ts | 41 +++++++++- tests/helpers.ts | 22 +++++ tests/reader/repository.test.ts | 84 ++++++++++++++++++++ 5 files changed, 232 insertions(+), 3 deletions(-) diff --git a/src/format/flatbuffers/repo-parser.ts b/src/format/flatbuffers/repo-parser.ts index f0c16a4..6bb9dc4 100644 --- a/src/format/flatbuffers/repo-parser.ts +++ b/src/format/flatbuffers/repo-parser.ts @@ -343,3 +343,49 @@ export function listTagsFromRepo(parsedRepo: ParsedRepo): string[] { const { repo } = parsedRepo; return listRefs((i) => repo.tags(i), repo.tagsLength()); } + +const snapshotIndexCache = new WeakMap>(); + +function snapshotIndexById(parsedRepo: ParsedRepo): Map { + let index = snapshotIndexCache.get(parsedRepo); + if (!index) { + index = new Map(); + for (let i = 0; i < parsedRepo.snapshotsLength; i++) { + index.set(bytesKey(getSnapshotIdByIndex(parsedRepo, i)), i); + } + snapshotIndexCache.set(parsedRepo, index); + } + return index; +} + +function bytesKey(bytes: Uint8Array): string { + return Array.from(bytes, (b) => b.toString(16).padStart(2, "0")).join(""); +} + +/** + * Resolve a snapshot's parent ID from the repo file's snapshots list. + * + * v2 snapshot files carry no parent pointer; ancestry lives only in the + * repo file as `parent_offset` indices. + * + * @returns Parent snapshot ID bytes, null for a root snapshot, or undefined + * if the snapshot is absent from the repo file + * @throws Error on corrupted data (invalid parent offset) + */ +export function resolveSnapshotParent( + parsedRepo: ParsedRepo, + snapshotId: Uint8Array, +): Uint8Array | null | undefined { + const index = snapshotIndexById(parsedRepo).get(bytesKey(snapshotId)); + if (index === undefined) return undefined; + + const parentOffset = parsedRepo.repo.snapshots(index)!.parentOffset(); + if (parentOffset === -1) return null; + if (parentOffset < 0 || parentOffset >= parsedRepo.snapshotsLength) { + throw new Error( + `Invalid parent offset ${parentOffset} for snapshot at index ${index}, ` + + `snapshots array has ${parsedRepo.snapshotsLength} entries`, + ); + } + return getSnapshotIdByIndex(parsedRepo, parentOffset); +} diff --git a/src/reader/repository.ts b/src/reader/repository.ts index ecca3af..3069ec0 100644 --- a/src/reader/repository.ts +++ b/src/reader/repository.ts @@ -19,6 +19,7 @@ import { resolveTag, listBranchesFromRepo, listTagsFromRepo, + resolveSnapshotParent, type ParsedRepo, } from "../format/flatbuffers/repo-parser.js"; import { ReadSession } from "./session.js"; @@ -472,22 +473,59 @@ export class Repository { metadata: Record; }> { let current: ReadSession | null = session; + const seen = new Set(); while (current) { + const id = encodeObjectId12(current.getSnapshotId()); + if (seen.has(id)) { + throw new Error(`Cycle in snapshot history at ${id}`); + } + seen.add(id); + yield { - id: encodeObjectId12(current.getSnapshotId()), + id, message: current.getMessage(), flushedAt: current.getFlushedAt(), metadata: current.getSnapshotMetadata(), }; - const parentId = current.getParentSnapshotId(); + const parentId = + current.getParentSnapshotId() ?? + (await this.resolveParentFromRepoInfo(current, options)); if (!parentId) break; current = await this.checkoutSnapshot(parentId, options); } } + /** + * v2 snapshots record ancestry only in the repo file. A snapshot missing + * from the cached copy may postdate it, so reload once before giving up. + */ + private async resolveParentFromRepoInfo( + session: ReadSession, + options?: RequestOptions, + ): Promise { + const snapshotId = session.getSnapshotId(); + const cached = await this.loadRepoInfo(options); + if (!cached) return null; + + const parent = resolveSnapshotParent(cached, snapshotId); + if (parent !== undefined) return parent; + + this.repoInfoAttempted = false; + const reloaded = await this.loadRepoInfo(options); + const reloadedParent = reloaded + ? resolveSnapshotParent(reloaded, snapshotId) + : undefined; + if (reloadedParent === undefined) { + throw new Error( + `Snapshot ${encodeObjectId12(snapshotId)} not found in repo info`, + ); + } + return reloadedParent; + } + /** * Get the storage backend. */ diff --git a/tests/format/flatbuffers/repo-parser.test.ts b/tests/format/flatbuffers/repo-parser.test.ts index f20c2e9..a30a53d 100644 --- a/tests/format/flatbuffers/repo-parser.test.ts +++ b/tests/format/flatbuffers/repo-parser.test.ts @@ -8,8 +8,13 @@ import { listTagsFromRepo, resolveBranch, resolveTag, + resolveSnapshotParent, } from "../../../src/format/flatbuffers/repo-parser.js"; -import { encodeObjectId12 } from "../../../src/format/object-id.js"; +import { + decodeObjectId12, + encodeObjectId12, +} from "../../../src/format/object-id.js"; +import { withParentOffset } from "../../helpers.js"; // ESM equivalent of __dirname const __dirname = dirname(fileURLToPath(import.meta.url)); @@ -96,6 +101,40 @@ describe("repo-parser", () => { }); }); + describe("resolveSnapshotParent", () => { + const repo = () => parseRepo(readFileSync(join(TEST_REPO_V2_PATH, "repo"))); + const MAIN_HEAD = decodeObjectId12("2768FV3XW5ZXYM84XNNG"); + + it("should resolve a snapshot's parent", () => { + const parent = resolveSnapshotParent(repo(), MAIN_HEAD); + expect(encodeObjectId12(parent!)).toBe("ZA24GZGPNK3GH682Q1R0"); + }); + + it("should return null for the root snapshot", () => { + const root = decodeObjectId12("1CECHNKREP0F1RSTCMT0"); + expect(resolveSnapshotParent(repo(), root)).toBeNull(); + }); + + it("should return undefined for a snapshot absent from the repo file", () => { + const unknown = new Uint8Array(12).fill(7); + expect(resolveSnapshotParent(repo(), unknown)).toBeUndefined(); + }); + + it("should throw on an out-of-range parent offset", () => { + const corrupt = withParentOffset(repo(), 1, 99); + expect(() => resolveSnapshotParent(corrupt, MAIN_HEAD)).toThrow( + "Invalid parent offset 99", + ); + }); + + it("should throw on a negative parent offset other than -1", () => { + const corrupt = withParentOffset(repo(), 1, -2); + expect(() => resolveSnapshotParent(corrupt, MAIN_HEAD)).toThrow( + "Invalid parent offset -2", + ); + }); + }); + describe("virtualChunkContainers", () => { it("includes the unnamed container with its parsed S3 store config", () => { // test-repo-v2's single container is unnamed (name: null) but carries an diff --git a/tests/helpers.ts b/tests/helpers.ts index aeb6773..8b2d118 100644 --- a/tests/helpers.ts +++ b/tests/helpers.ts @@ -2,6 +2,8 @@ * Test utilities and helpers. */ +import type { ParsedRepo } from "../src/format/flatbuffers/repo-parser.js"; + // Redeclare globalThis types for TypeScript declare global { var FIXTURE_SERVER_PORT: number; @@ -15,3 +17,23 @@ export function getFixtureUrl(repoName: string): string { const baseUrl = globalThis.FIXTURE_SERVER_URL ?? "http://localhost:8765"; return `${baseUrl}/${repoName}`; } + +/** + * Return a copy of a parsed repo whose snapshot at `index` reports + * `parentOffset`, for simulating corrupted ancestry. + */ +export function withParentOffset( + parsed: ParsedRepo, + index: number, + parentOffset: number, +): ParsedRepo { + const repo = Object.create(parsed.repo) as ParsedRepo["repo"]; + repo.snapshots = (i, obj) => { + const info = parsed.repo.snapshots(i, obj); + if (i !== index || !info) return info; + const patched = Object.create(info) as typeof info; + patched.parentOffset = () => parentOffset; + return patched; + }; + return { ...parsed, repo }; +} diff --git a/tests/reader/repository.test.ts b/tests/reader/repository.test.ts index 3c27b36..91f97ae 100644 --- a/tests/reader/repository.test.ts +++ b/tests/reader/repository.test.ts @@ -3,6 +3,12 @@ import { readFileSync, readdirSync } from "node:fs"; import { join, dirname } from "node:path"; import { fileURLToPath } from "node:url"; import { Repository } from "../../src/reader/repository.js"; +import type { ReadSession } from "../../src/reader/session.js"; +import { + parseRepo, + type ParsedRepo, +} from "../../src/format/flatbuffers/repo-parser.js"; +import { withParentOffset } from "../helpers.js"; import { MockStorage, MockStorageNoList, @@ -593,6 +599,84 @@ describe("Repository", () => { }); }); + describe("walkHistory (v2)", () => { + // v2 snapshot files carry no parent; ancestry comes from the repo file. + const V2_MAIN_HISTORY = [ + { id: "2768FV3XW5ZXYM84XNNG", message: "set virtual chunk" }, + { id: "ZA24GZGPNK3GH682Q1R0", message: "fill data" }, + { id: "7T7DSS71H2JNBBZQ9M8G", message: "empty structure" }, + { id: "1CECHNKREP0F1RSTCMT0", message: "Repository initialized" }, + ]; + + async function collect(repo: Repository, session: ReadSession) { + const entries = []; + for await (const entry of repo.walkHistory(session)) { + entries.push({ id: entry.id, message: entry.message }); + } + return entries; + } + + async function collectBranch(branch: string) { + const storage = loadRepoIntoMockStorage(TEST_REPO_V2_PATH); + const repo = await Repository.open({ storage }); + return collect(repo, await repo.checkoutBranch(branch)); + } + + /** The oldest overwritten repo file, which predates the main head. */ + function oldestRepoInfo(): Uint8Array { + const dir = join(TEST_REPO_V2_PATH, "overwritten"); + return readFileSync(join(dir, readdirSync(dir).sort().at(-1)!)); + } + + it("should walk linear history from branch head to root", async () => { + expect(await collectBranch("main")).toEqual(V2_MAIN_HISTORY); + }); + + it("should walk history including diverged commits on feature branch", async () => { + expect(await collectBranch("my-branch")).toEqual([ + { id: "H7T8YA6KR5TH32F3PS10", message: "some more structure" }, + { id: "QTP2RTYS5YQW8RPSQ93G", message: "delete a chunk" }, + ...V2_MAIN_HISTORY, + ]); + }); + + it("should reload repo info for snapshots newer than the cached copy", async () => { + const storage = loadRepoIntoMockStorage(TEST_REPO_V2_PATH); + const latest = await storage.getObject(REPO_INFO_PATH); + storage.addFile(REPO_INFO_PATH, oldestRepoInfo()); + const repo = await Repository.open({ storage }); + storage.addFile(REPO_INFO_PATH, latest); + + const session = await repo.checkoutSnapshot(V2_MAIN_HISTORY[0].id); + expect(await collect(repo, session)).toEqual(V2_MAIN_HISTORY); + }); + + it("should throw for a snapshot absent from the repo info", async () => { + const storage = loadRepoIntoMockStorage(TEST_REPO_V2_PATH); + storage.addFile(REPO_INFO_PATH, oldestRepoInfo()); + const repo = await Repository.open({ storage }); + + const session = await repo.checkoutSnapshot(V2_MAIN_HISTORY[0].id); + await expect(collect(repo, session)).rejects.toThrow( + `Snapshot ${V2_MAIN_HISTORY[0].id} not found in repo info`, + ); + }); + + it("should throw on cyclic ancestry", async () => { + const storage = loadRepoIntoMockStorage(TEST_REPO_V2_PATH); + const parsed = parseRepo(await storage.getObject(REPO_INFO_PATH)); + // Point "fill data" (index 5) back at the main head (index 1). + const corrupt = withParentOffset(parsed, 5, 1); + const repo = await Repository.open({ storage }); + (repo as unknown as { repoInfo: ParsedRepo }).repoInfo = corrupt; + + const session = await repo.checkoutBranch("main"); + await expect(collect(repo, session)).rejects.toThrow( + "Cycle in snapshot history", + ); + }); + }); + describe("formatVersion hint", () => { it("should skip all validation when formatVersion is v1", async () => { const storage = new MockStorageNoList({}); From f52cc0a3a9a93415f7c7d569077710560da75bc8 Mon Sep 17 00:00:00 2001 From: Shane Loeffler Date: Thu, 15 Oct 2026 20:18:32 -0500 Subject: [PATCH 2/2] Drop repo info reload from walkHistory A snapshot absent from the cached repo info now throws rather than triggering a refetch, keeping the Repository's view fixed at open. --- src/reader/repository.ts | 24 +++++++----------------- tests/reader/repository.test.ts | 11 ----------- 2 files changed, 7 insertions(+), 28 deletions(-) diff --git a/src/reader/repository.ts b/src/reader/repository.ts index 3069ec0..1cc8f7b 100644 --- a/src/reader/repository.ts +++ b/src/reader/repository.ts @@ -498,32 +498,22 @@ export class Repository { } } - /** - * v2 snapshots record ancestry only in the repo file. A snapshot missing - * from the cached copy may postdate it, so reload once before giving up. - */ + /** v2 snapshots record ancestry only in the repo file. */ private async resolveParentFromRepoInfo( session: ReadSession, options?: RequestOptions, ): Promise { + const repoInfo = await this.loadRepoInfo(options); + if (!repoInfo) return null; + const snapshotId = session.getSnapshotId(); - const cached = await this.loadRepoInfo(options); - if (!cached) return null; - - const parent = resolveSnapshotParent(cached, snapshotId); - if (parent !== undefined) return parent; - - this.repoInfoAttempted = false; - const reloaded = await this.loadRepoInfo(options); - const reloadedParent = reloaded - ? resolveSnapshotParent(reloaded, snapshotId) - : undefined; - if (reloadedParent === undefined) { + const parent = resolveSnapshotParent(repoInfo, snapshotId); + if (parent === undefined) { throw new Error( `Snapshot ${encodeObjectId12(snapshotId)} not found in repo info`, ); } - return reloadedParent; + return parent; } /** diff --git a/tests/reader/repository.test.ts b/tests/reader/repository.test.ts index 91f97ae..bbf5a7e 100644 --- a/tests/reader/repository.test.ts +++ b/tests/reader/repository.test.ts @@ -640,17 +640,6 @@ describe("Repository", () => { ]); }); - it("should reload repo info for snapshots newer than the cached copy", async () => { - const storage = loadRepoIntoMockStorage(TEST_REPO_V2_PATH); - const latest = await storage.getObject(REPO_INFO_PATH); - storage.addFile(REPO_INFO_PATH, oldestRepoInfo()); - const repo = await Repository.open({ storage }); - storage.addFile(REPO_INFO_PATH, latest); - - const session = await repo.checkoutSnapshot(V2_MAIN_HISTORY[0].id); - expect(await collect(repo, session)).toEqual(V2_MAIN_HISTORY); - }); - it("should throw for a snapshot absent from the repo info", async () => { const storage = loadRepoIntoMockStorage(TEST_REPO_V2_PATH); storage.addFile(REPO_INFO_PATH, oldestRepoInfo());