mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-11-11 05:11:49 +01:00
perf(workspaces): verify saved Git objects without a single-threaded full fsck (#20329)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - Remote (sandbox) workspaces are saved to durable storage as encrypted snapshots, and restored before a later turn > - Saves and restores are a reported source of slowness on staging, so #20280 added run-log timing for each step > - A Daytona storage wave with a Paperclip-sized workspace (R1, ~750 MB, 19.5k entries) showed each save taking 58–68 s. Storage was only 2–3 s of that; capture inside the sandbox was 44–47 s > - The first save runs while the first run prepares its workspace, so it delays the first token > - A local profile of the same capture found one cause: `git fsck --full` re-hashes the whole history on one thread on every save > - This pull request keeps the same object checks but verifies packs on several threads > - The benefit is about 10 s less per save on a Paperclip-sized repository, with the same corruption protection ## Linked Issues or Issue Description Refs #18990, #20280 **What happened?** Saving a remote workspace with a large Git history is slow. On Daytona (3 vCPU), a Paperclip clone's save took 58–68 s; capture was 44–47 s of it. The first save is on the first run's critical path: `heartbeat.prepare.workspace` took 90–110 s before the first turn. **Steps to reproduce** 1. Clone Paperclip (`git clone https://github.com/paperclipai/paperclip.git`, ~750 MB, one 459 MiB pack). 2. Run the generated archive helper: `node archive.cjs capture <clone> <out.pcws>` (from `remoteWorkspaceArchiveScript()`). 3. It takes ~30 s. A wrapper on `git` shows 22.5 s in one `git fsck --full --no-dangling` on the normalized copy. **Expected behavior** Capture verifies saved Git objects as #18990 requires (a missing or corrupt object blocks the save), without spending most of the save re-hashing unchanged history on one thread. ## What Changed - `remote-workspace-snapshot-archive.ts`: both `git fsck --full --no-dangling` calls (the offline source check and the normalized-copy check) now call a new `verifyGitObjects`. It runs: - `git fsck --connectivity-only --no-dangling`: every object reachable from refs, the index and saved Git state is present. - A direct hash of each loose object (inflate, then SHA-1 or SHA-256 per the repository's object format) against its file name. A mismatch or unreadable object fails with the existing `invalid_git_repository` reason. - `git index-pack --verify --threads=N` for every pack (N = available CPUs, at most 8). This re-hashes every packed object, on several threads. - `fsck --no-full` was not used: in a shallow clone it reports reflog entries that point at packed objects as invalid. - Tests (`remote-workspace-snapshot-archive.test.ts`): - New: capture refuses a repository missing a staged object, or a committed object. - New: capture refuses a repository with a corrupt byte inside a pack. - The existing #18990 test `rejects corrupt available objects in full/shallow/partial Git before publishing a receipt` (valid zlib, wrong hash) still passes. It failed with connectivity-only fsck alone, which is why the loose-object hash is there. ## Verification - `npx vitest run` on `remote-workspace-snapshot-archive`, `remote-workspace-transport`, `remote-workspace-migration`, `remote-workspace-online-source` and `remote-workspace-readonly-capture-budget`: 526 passed. - Local benchmark: a fresh clone of `https://github.com/paperclipai/paperclip.git` (753,506,675-byte archive, 19,506 entries), alternating runs on one host: | Capture | Run 1 | Run 2 | |---|---|---| | master helper | 32.04 s | 30.05 s | | this branch | 21.07 s | 20.09 s | Both produce the same archive: equal byte size, manifest digest (`e5facf033409…`) and sha256 (`a4eb843bff1f…`). - Component timings on the same clone: `fsck --full` 20.5 s; `index-pack --verify` 19.5 s on 1 thread, 10.7 s on 3 threads; `fsck --connectivity-only` 0.9 s. - Daytona evidence for the problem (wave `w-20261109T170632-s9921`, R1, 3 vCPU, MinIO): save `captureMs` 44–47 s, `transferMs` 11–20 s, storage `putMs` 1.3–1.8 s, `encryptMs` 0.4–0.7 s. I did not rerun Daytona on this branch: its image must match the checkout's runner content. The local helper run is the same code the sandbox executes. ## Risks - Same guarantees as before for missing and corrupt objects. `fsck --full` also applies its object syntax checks to every packed object; `index-pack --verify` checks hashes and pack integrity. Commits and trees are still parsed by the connectivity walk, so malformed ones still fail. - More CPU threads during capture, for a shorter time. Capture already runs inside the sandbox, off the control plane. - A follow-up could skip re-verifying a pack that an earlier save of the same workspace already verified. Most saves keep the same clone pack, so that would remove the remaining ~10 s on repeat saves. Not in this PR. ## Model Used Claude Opus 5.5 (`claude-opus-5-5`, Anthropic), 1M context window, tool use and code execution, via Claude Code. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
1 parent
4a349ea447
commit
4d2a77cf5a
2 files changed
+59
-2
No files matched your search
@@ -726,6 +726,30 @@ describe("portable remote workspace archives", () => {
|
||||
expect(await sourceBytes(cwd)).toBe(bytes);
|
||||
await expect(fs.access(path.join(root, "snapshot.pcws"))).rejects.toMatchObject({ code: "ENOENT" });
|
||||
});
|
||||
it.each(["staged", "committed"])("refuses to save a repository missing a %s object", async kind => {
|
||||
const cwd = await source();
|
||||
await git(cwd, "init", "-q", "-b", "main");
|
||||
await fs.writeFile(path.join(cwd, "work.txt"), "committed\n"); await git(cwd, "add", ".");
|
||||
await git(cwd, "-c", "user.name=t", "-c", "user.email=t@example.invalid", "commit", "-qm", "base");
|
||||
if (kind === "staged") { await fs.writeFile(path.join(cwd, "work.txt"), "staged\n"); await git(cwd, "add", "work.txt"); }
|
||||
const object = await git(cwd, "rev-parse", kind === "staged" ? ":work.txt" : "HEAD:work.txt");
|
||||
await fs.rm(path.join(cwd, ".git", "objects", object.slice(0, 2), object.slice(2)), { force: true });
|
||||
await expect(capture(cwd)).rejects.toThrow();
|
||||
await expect(fs.access(path.join(root, "snapshot.pcws"))).rejects.toMatchObject({ code: "ENOENT" });
|
||||
});
|
||||
it("refuses to save a repository with a corrupt packed object", async () => {
|
||||
const cwd = await source();
|
||||
await git(cwd, "init", "-q", "-b", "main");
|
||||
for (const name of ["a", "b", "c"]) await fs.writeFile(path.join(cwd, `${name}.txt`), `${name} `.repeat(4096));
|
||||
await git(cwd, "add", "."); await git(cwd, "-c", "user.name=t", "-c", "user.email=t@example.invalid", "commit", "-qm", "base");
|
||||
await git(cwd, "repack", "-qad");
|
||||
const packs = path.join(cwd, ".git", "objects", "pack");
|
||||
const pack = path.join(packs, (await fs.readdir(packs)).find(name => name.endsWith(".pack"))!);
|
||||
const bytes = await fs.readFile(pack); bytes[Math.floor(bytes.length / 2)] ^= 0xff;
|
||||
await fs.chmod(pack, 0o600); await fs.writeFile(pack, bytes);
|
||||
await expect(capture(cwd)).rejects.toThrow();
|
||||
await expect(fs.access(path.join(root, "snapshot.pcws"))).rejects.toMatchObject({ code: "ENOENT" });
|
||||
});
|
||||
it("rejects a damaged Git entry with a closed reason instead of a raw command error", async () => {
|
||||
const cwd = await source(); await fs.mkdir(path.join(cwd, ".git"));
|
||||
await fs.writeFile(path.join(cwd, "work.txt"), "unsaved\n");
|
||||
|
||||
@@ -315,6 +315,39 @@ async function workspaceArchive(input: {
|
||||
return { gitDirectory, commonDirectory, stores };
|
||||
};
|
||||
const gitStatePath = async (repo: string, name: string) => (await git(repo, ["rev-parse", "--path-format=absolute", "--git-path", name])).trim();
|
||||
// The checks of `git fsck --full --no-dangling` without its single-threaded
|
||||
// re-hash of every packed object, which took ~20 s for a Paperclip-sized
|
||||
// history on every save. Connectivity proves every reachable object (refs,
|
||||
// index, saved Git state) is present; each loose object must hash to its
|
||||
// name; index-pack --verify hashes every packed object on several threads.
|
||||
const verifyGitObjects = async (repo: string) => {
|
||||
await git(repo, ["fsck", "--connectivity-only", "--no-dangling"]);
|
||||
const format = (await git(repo, ["rev-parse", "--show-object-format"])).trim();
|
||||
if (!["sha1", "sha256"].includes(format)) fail("unsupported_git_setting");
|
||||
const zlib = await import("node:zlib");
|
||||
const { pipeline } = await import("node:stream/promises");
|
||||
const objects = await gitStatePath(repo, "objects");
|
||||
const listing = async (directory: string) => fs.readdir(directory)
|
||||
.catch((error: NodeJS.ErrnoException) => { if (error.code === "ENOENT") return [] as string[]; throw error; });
|
||||
const loose = new RegExp(`^[0-9a-f]{${format === "sha1" ? 38 : 62}}$`, "u");
|
||||
for (const prefix of (await listing(objects)).filter(name => /^[0-9a-f]{2}$/u.test(name)).sort()) {
|
||||
for (const rest of (await listing(path.join(objects, prefix))).filter(name => loose.test(name)).sort()) {
|
||||
check();
|
||||
const hash = crypto.createHash(format);
|
||||
// pipeline forwards read errors and cancellation from both streams and
|
||||
// closes them; only damaged zlib data names the entry as damaged Git.
|
||||
await pipeline(createReadStream(path.join(objects, prefix, rest), { signal: input.signal }), zlib.createInflate(),
|
||||
async (inflated: AsyncIterable<Buffer>) => { for await (const bytes of inflated) hash.update(bytes); },
|
||||
).catch((error: NodeJS.ErrnoException) => { if (/^Z_/u.test(error?.code ?? "")) fail("invalid_git_repository"); throw error; });
|
||||
if (hash.digest("hex") !== prefix + rest) fail("invalid_git_repository");
|
||||
}
|
||||
}
|
||||
const packs = path.join(objects, "pack");
|
||||
const threads = Math.max(1, Math.min(8, os.availableParallelism?.() ?? os.cpus().length));
|
||||
for (const name of (await listing(packs)).filter(name => name.endsWith(".pack")).sort()) {
|
||||
await git(repo, ["index-pack", "--verify", `--threads=${threads}`, path.join(packs, name)]);
|
||||
}
|
||||
};
|
||||
const gitStateHash = async (repo: string, name: string) => {
|
||||
const file = await gitStatePath(repo, name);
|
||||
return readStableFile(file).then(entry => entry.sha256).catch((error: NodeJS.ErrnoException) => {
|
||||
@@ -471,7 +504,7 @@ async function workspaceArchive(input: {
|
||||
// A shallow local clone uses upload-pack. It drops index-only objects
|
||||
// and asks a filtered source for promised blobs it never downloaded.
|
||||
// Preserve its existing stores and promisor boundaries without fetch.
|
||||
await git(repo, ["fsck", "--full", "--no-dangling"]);
|
||||
await verifyGitObjects(repo);
|
||||
const format = (await git(repo, ["rev-parse", "--show-object-format"])).trim();
|
||||
if (!["sha1", "sha256"].includes(format)) fail("unsupported_git_setting");
|
||||
await git(root, ["init", "--bare", "--template=", `--object-format=${format}`, normalized]);
|
||||
@@ -522,7 +555,7 @@ async function workspaceArchive(input: {
|
||||
await git(restoredRepo, ["cat-file", "-e", `${object}^{${type}}`]);
|
||||
}
|
||||
await fs.rm(path.join(destination, "objects", "info", "alternates"), { force: true });
|
||||
await git(restoredRepo, ["fsck", "--full", "--no-dangling"]);
|
||||
await verifyGitObjects(restoredRepo);
|
||||
if (await signature(repo) !== gitSignatures.get(relative)) fail("changed");
|
||||
}
|
||||
const originals = new Map(before.map(entry => [entry.path, entry]));
|
||||
|
||||
Reference in new issue
Block a user