From 6401f4f78cbbae6ed5abb46ff5b6938267516229 Mon Sep 17 00:00:00 2001 From: Devin Foley Date: Fri, 31 Jul 2026 23:41:42 -0700 Subject: [PATCH] fix(sandbox): bundle git copy-back against the merge-base so diverged/reset workspaces still import (#10601) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - An agent that runs in a sandbox has its workspace copied back to the host when the run ends, so its work persists — the copy-back ships a git bundle of the sandbox's commits > - The bundle is created as a thin delta, `git bundle create HEAD --not `, which records `baseSha` (the host workspace HEAD captured at export) as a prerequisite the host must already hold > - That assumption breaks when the sandbox HEAD has diverged from `baseSha`, or when a shared host workspace no longer holds `baseSha` at import time — then `git fetch` on the host hard-fails and the entire run is lost even though the agent finished its work > - This pull request bundles against the merge-base of `baseSha` and the sandbox HEAD (with a full-bundle fallback), which the host can satisfy in those cases > - The benefit is that copy-back no longer discards a completed run's work over a base the host can't reconcile ## Linked Issues or Issue Description **What happened?** A sandbox agent run completed its work, then failed during workspace finalize: ``` git -C fetch --force refs/…/export:refs/…/imported error: Could not read fatal: revision walk setup failed error: git-delta.bundle did not send all necessary objects ``` The run is reported as `adapter_failed` even though the agent produced output. The copy-back bundle names the host workspace's recorded HEAD (`baseSha`) as a prerequisite, but the host cannot satisfy it. **Steps to reproduce** Two independent triggers, both reproduced in tests: 1. The sandbox's HEAD has diverged from `baseSha` — e.g. the sandbox carries a local-only branch that forked from an older commit than the host's current HEAD. 2. The shared host workspace no longer holds `baseSha` at import time (it was reset / re-realized between export and import). In either case `git fetch` of the thin bundle fails with a missing prerequisite. **Expected behavior** Copy-back imports the sandbox's work as long as the host holds any common ancestor, instead of hard-failing and discarding the run. **Paperclip version** Current `master`. **Deployment mode** Any deployment running agents in sandbox environments with workspace sync (notably shared-workspace clones and custom images that carry a local-ahead branch). ## What Changed - `buildRemoteGitDeltaBundleScript` now computes `bundle_base = git merge-base HEAD` and bundles `HEAD --not `. The merge-base is an ancestor of `baseSha`, so any host that holds `baseSha` (or an ancestor of it — e.g. after a reset) can satisfy the prerequisite, and the bundle stays a delta rather than a full-history transfer. - When `baseSha` is absent from the sandbox, or no merge-base exists, it falls back to a full, self-contained bundle (no prerequisites) so the import can always complete. - The existing empty-bundle no-op (no new commits) and the ordinary fast-forward path are unchanged; the `cat-file` base check no longer aborts the script under `set -e`. ## Verification - `pnpm vitest run packages/adapter-utils/src/git-workspace-sync.test.ts packages/adapter-utils/src/sandbox-managed-runtime.test.ts` — new cases: a diverged sandbox HEAD imports when the host holds only the merge-base (not `baseSha`), and the full-bundle fallback imports into a host that shares no history; existing thin-delta and empty-bundle cases still pass. - `pnpm --filter @paperclipai/adapter-utils exec tsc --noEmit`. - Standalone shell repro confirmed the old thin bundle fails with "Repository lacks these prerequisite commits" in both trigger cases, and the merge-base bundle imports successfully. ## Risks - Low. For the common case (sandbox HEAD descends from `baseSha`) the merge-base is `baseSha`, so the bundle is byte-for-byte the same delta as before. The change only alters behavior when the old code would have hard-failed. - This makes the copy-back import succeed on a diverged base; the subsequent reconciliation of divergent histories (`integrateImportedGitHead`) is unchanged and still owns how the imported head is merged into the host branch. Where a workspace's history has genuinely diverged (e.g. a stale custom image carrying a local-only branch), a clean re-clone/re-capture is still the right operational fix — this change prevents work loss, it does not reconcile intentional divergence. ## Model Used Claude Fable 5 (`claude-fable-5`, Anthropic) via Claude Code — extended thinking, agentic tool use (file edits, shell repro, vitest/tsc runs). No other models involved. ## 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 - [x] I have considered and documented any risks above - [ ] All Paperclip CI gates are green - [ ] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge --- .../src/git-workspace-sync.test.ts | 174 ++++++++++++++++++ .../adapter-utils/src/git-workspace-sync.ts | 63 ++++++- .../src/sandbox-managed-runtime.ts | 88 +++++---- 3 files changed, 288 insertions(+), 37 deletions(-) diff --git a/packages/adapter-utils/src/git-workspace-sync.test.ts b/packages/adapter-utils/src/git-workspace-sync.test.ts index 7a7b8c646a..596cd70981 100644 --- a/packages/adapter-utils/src/git-workspace-sync.test.ts +++ b/packages/adapter-utils/src/git-workspace-sync.test.ts @@ -11,6 +11,7 @@ import { createRemoteGitExportRef, deleteLocalGitRef, fetchGitBundleIntoLocalRef, + isMissingGitPrerequisiteError, readGitWorkspaceSnapshot, runLocalGit, withShallowGitWorkspaceClone, @@ -125,4 +126,177 @@ describe("git workspace sync", () => { } }); }); + + it("imports a diverged sandbox HEAD even when the host no longer holds baseSha", async () => { + const rootDir = await mkdtemp(path.join(os.tmpdir(), "paperclip-git-diverge-")); + cleanupDirs.push(rootDir); + // Host holds only the shared ancestor B (the eventual merge-base), not the + // recorded base H — the state a shared workspace lands in when it is reset + // between export and import. + const host = await createRepo(rootDir); + const mergeBase = await git(host, ["rev-parse", "HEAD"]); + + // Sandbox holds B, an advanced commit H (the recorded baseSha), and a + // local-only commit S that forked from B and diverges from H. + const sandbox = path.join(rootDir, "sandbox"); + await git(rootDir, ["clone", host, sandbox]); + await git(sandbox, ["config", "user.name", "Paperclip Remote"]); + await git(sandbox, ["config", "user.email", "remote@paperclip.dev"]); + await writeFile(path.join(sandbox, "advance.txt"), "advance\n", "utf8"); + await git(sandbox, ["add", "-A"]); + await git(sandbox, ["commit", "-m", "advance"]); + const baseSha = await git(sandbox, ["rev-parse", "HEAD"]); + await git(sandbox, ["reset", "--hard", mergeBase]); + await writeFile(path.join(sandbox, "local.txt"), "local\n", "utf8"); + await git(sandbox, ["add", "-A"]); + await git(sandbox, ["commit", "-m", "local-only"]); + const sandboxHead = await git(sandbox, ["rev-parse", "HEAD"]); + + // The host genuinely lacks baseSha; the old thin bundle would name it as an + // unsatisfiable prerequisite. + await expect(git(host, ["cat-file", "-e", `${baseSha}^{commit}`])).rejects.toThrow(); + + const bundle = path.join(rootDir, "diverge.bundle"); + const exportRef = createRemoteGitExportRef("test"); + const importedRef = createImportedGitRef("test"); + try { + await execFile("sh", ["-c", buildRemoteGitDeltaBundleScript({ + remoteDir: sandbox, + baseSha, + exportRef, + bundlePath: bundle, + })]); + expect((await stat(bundle)).size).toBeGreaterThan(0); + + const importedHead = await fetchGitBundleIntoLocalRef({ + localDir: host, + bundlePath: bundle, + exportRef, + importedRef, + baseSha, + }); + expect(importedHead).toBe(sandboxHead); + // The host received the local-only commit and its parent (the merge-base). + expect(await git(host, ["cat-file", "-e", `${sandboxHead}^{commit}`])).toBe(""); + } finally { + await deleteLocalGitRef({ localDir: host, ref: importedRef }); + } + }); + + it("re-exports a full bundle that imports when the host holds neither baseSha nor the merge-base", async () => { + const rootDir = await mkdtemp(path.join(os.tmpdir(), "paperclip-git-ancestor-")); + cleanupDirs.push(rootDir); + // Host was reset to a strict ancestor of the eventual merge-base: it holds + // only the very first commit, not baseSha and not the fork point. + const host = await createRepo(rootDir); + const ancestor = await git(host, ["rev-parse", "HEAD"]); + + const sandbox = path.join(rootDir, "sandbox"); + await git(rootDir, ["clone", host, sandbox]); + await git(sandbox, ["config", "user.name", "Paperclip Remote"]); + await git(sandbox, ["config", "user.email", "remote@paperclip.dev"]); + // Advance the merge-base past the host, then baseSha past that, then a + // divergent local commit — so merge-base(baseSha, HEAD) is itself a commit + // the host does not hold. + await writeFile(path.join(sandbox, "fork.txt"), "fork\n", "utf8"); + await git(sandbox, ["add", "-A"]); + await git(sandbox, ["commit", "-m", "fork point"]); + const forkPoint = await git(sandbox, ["rev-parse", "HEAD"]); + await writeFile(path.join(sandbox, "advance.txt"), "advance\n", "utf8"); + await git(sandbox, ["add", "-A"]); + await git(sandbox, ["commit", "-m", "advance"]); + const baseSha = await git(sandbox, ["rev-parse", "HEAD"]); + await git(sandbox, ["reset", "--hard", forkPoint]); + await writeFile(path.join(sandbox, "local.txt"), "local\n", "utf8"); + await git(sandbox, ["add", "-A"]); + await git(sandbox, ["commit", "-m", "local-only"]); + const sandboxHead = await git(sandbox, ["rev-parse", "HEAD"]); + + // Host holds only the initial commit; it lacks both baseSha and the fork point. + expect(await git(host, ["rev-parse", "HEAD"])).toBe(ancestor); + await expect(git(host, ["cat-file", "-e", `${forkPoint}^{commit}`])).rejects.toThrow(); + + const exportRef = createRemoteGitExportRef("test"); + const importedRef = createImportedGitRef("test"); + + // The delta bundle (relative to the merge-base = fork point) names a + // prerequisite the host lacks, so its import fails and is detected. + const deltaBundle = path.join(rootDir, "delta.bundle"); + await execFile("sh", ["-c", buildRemoteGitDeltaBundleScript({ + remoteDir: sandbox, + baseSha, + exportRef, + bundlePath: deltaBundle, + })]); + let deltaError: unknown; + try { + await fetchGitBundleIntoLocalRef({ localDir: host, bundlePath: deltaBundle, exportRef, importedRef, baseSha }); + } catch (error) { + deltaError = error; + } + expect(deltaError).toBeDefined(); + expect(isMissingGitPrerequisiteError(deltaError)).toBe(true); + + // The forced full bundle is self-contained and imports into the same host. + const fullBundle = path.join(rootDir, "full.bundle"); + try { + await execFile("sh", ["-c", buildRemoteGitDeltaBundleScript({ + remoteDir: sandbox, + baseSha, + exportRef, + bundlePath: fullBundle, + forceFullBundle: true, + })]); + const importedHead = await fetchGitBundleIntoLocalRef({ + localDir: host, + bundlePath: fullBundle, + exportRef, + importedRef, + baseSha, + }); + expect(importedHead).toBe(sandboxHead); + } finally { + await deleteLocalGitRef({ localDir: host, ref: importedRef }); + } + }); + + it("falls back to a full self-contained bundle when the sandbox lacks baseSha", async () => { + const rootDir = await mkdtemp(path.join(os.tmpdir(), "paperclip-git-full-")); + cleanupDirs.push(rootDir); + const sandbox = await createRepo(rootDir); + await writeFile(path.join(sandbox, "more.txt"), "more\n", "utf8"); + await git(sandbox, ["add", "-A"]); + await git(sandbox, ["commit", "-m", "more"]); + const sandboxHead = await git(sandbox, ["rev-parse", "HEAD"]); + + // A fresh, unrelated host that shares no history with the sandbox. + const host = path.join(rootDir, "fresh-host"); + await mkdir(host, { recursive: true }); + await git(host, ["init"]); + + const bundle = path.join(rootDir, "full.bundle"); + const exportRef = createRemoteGitExportRef("test"); + const importedRef = createImportedGitRef("test"); + try { + await execFile("sh", ["-c", buildRemoteGitDeltaBundleScript({ + remoteDir: sandbox, + // A base the sandbox does not have forces the full-bundle fallback. + baseSha: "0000000000000000000000000000000000000000", + exportRef, + bundlePath: bundle, + })]); + expect((await stat(bundle)).size).toBeGreaterThan(0); + + const importedHead = await fetchGitBundleIntoLocalRef({ + localDir: host, + bundlePath: bundle, + exportRef, + importedRef, + baseSha: "0000000000000000000000000000000000000000", + }); + expect(importedHead).toBe(sandboxHead); + } finally { + await deleteLocalGitRef({ localDir: host, ref: importedRef }); + } + }); }); diff --git a/packages/adapter-utils/src/git-workspace-sync.ts b/packages/adapter-utils/src/git-workspace-sync.ts index 1e0a37d8a1..0e36dee982 100644 --- a/packages/adapter-utils/src/git-workspace-sync.ts +++ b/packages/adapter-utils/src/git-workspace-sync.ts @@ -197,6 +197,24 @@ export async function fetchGitBundleIntoLocalRef(input: { return importedHead.stdout.trim(); } +/** Substrings git emits when a bundle names a prerequisite the importer lacks. */ +const GIT_MISSING_PREREQUISITE_MARKERS = [ + "did not send all necessary objects", + "lacks these prerequisite commits", + "revision walk setup failed", +]; + +/** + * True when a bundle import failed because the host repository does not hold a + * commit the (delta) bundle assumes as a prerequisite. Such a failure is + * recoverable by re-exporting a full, self-contained bundle from the still-live + * sandbox rather than discarding the run. + */ +export function isMissingGitPrerequisiteError(error: unknown): boolean { + const message = error instanceof Error ? error.message : String(error); + return GIT_MISSING_PREREQUISITE_MARKERS.some((marker) => message.includes(marker)); +} + export function buildRemoteGitDeltaBundleScript(input: { remoteDir: string; baseSha: string; @@ -205,6 +223,12 @@ export function buildRemoteGitDeltaBundleScript(input: { statusPath?: string; catBundle?: boolean; cleanupBundle?: boolean; + /** + * Skip the delta boundary entirely and always emit a full, self-contained + * bundle (no prerequisites). Used as the recovery path when a delta import + * failed because the host lacked the bundle's prerequisite. + */ + forceFullBundle?: boolean; }): string { const remoteDir = shellQuote(input.remoteDir); const bundlePath = shellQuote(input.bundlePath); @@ -222,11 +246,44 @@ export function buildRemoteGitDeltaBundleScript(input: { input.cleanupBundle ? "trap cleanup EXIT" : "", `mkdir -p ${shellQuote(path.posix.dirname(input.bundlePath))}`, `rm -f ${bundlePath}`, - `git -C ${remoteDir} cat-file -e ${baseSha}^{commit}`, - `commit_count=$(git -C ${remoteDir} rev-list --count HEAD --not ${baseSha})`, + // Choose the bundle boundary. A thin bundle `HEAD --not ` records + // baseSha as a prerequisite the importer (host) must already hold. That + // assumption breaks in two real cases, and then `git fetch` on the host + // hard-fails with "did not send all necessary objects" and the run's work + // is lost: + // 1. The sandbox HEAD has diverged from baseSha (e.g. a local-only branch + // that forked from an older commit) — the host may still hold baseSha, + // but a repo whose history is inconsistent cannot satisfy the walk. + // 2. The host workspace no longer holds baseSha at import time (a shared + // workspace that was reset/re-realized between export and import). + // Bundle relative to the merge-base of baseSha and HEAD instead: that + // merge-base is an ancestor of baseSha, so any host that holds baseSha (or + // an ancestor of it) can satisfy the prerequisite, while the bundle stays a + // delta. When baseSha is absent from the sandbox — or no merge-base exists, + // or the caller forces it after a delta import failed on a missing + // prerequisite — fall back to a full, self-contained bundle with no + // prerequisites. + ...(input.forceFullBundle + ? [`bundle_base=""`] + : [ + `if git -C ${remoteDir} cat-file -e ${baseSha}^{commit} 2>/dev/null; then`, + ` bundle_base=$(git -C ${remoteDir} merge-base ${baseSha} HEAD 2>/dev/null || true)`, + "else", + ` bundle_base=""`, + "fi", + ]), + `if [ -n "$bundle_base" ]; then`, + ` commit_count=$(git -C ${remoteDir} rev-list --count HEAD --not "$bundle_base")`, + "else", + ` commit_count=$(git -C ${remoteDir} rev-list --count HEAD)`, + "fi", 'if [ "$commit_count" -gt 0 ]; then', ` git -C ${remoteDir} update-ref ${exportRef} HEAD`, - ` git -C ${remoteDir} bundle create ${bundlePath} ${exportRef} --not ${baseSha} >/dev/null`, + ` if [ -n "$bundle_base" ]; then`, + ` git -C ${remoteDir} bundle create ${bundlePath} ${exportRef} --not "$bundle_base" >/dev/null`, + " else", + ` git -C ${remoteDir} bundle create ${bundlePath} ${exportRef} >/dev/null`, + " fi", "else", ` : > ${bundlePath}`, "fi", diff --git a/packages/adapter-utils/src/sandbox-managed-runtime.ts b/packages/adapter-utils/src/sandbox-managed-runtime.ts index 74b1f4520c..2f9db88dc8 100644 --- a/packages/adapter-utils/src/sandbox-managed-runtime.ts +++ b/packages/adapter-utils/src/sandbox-managed-runtime.ts @@ -6,6 +6,7 @@ import path from "node:path"; import { promisify } from "node:util"; import { buildRemoteGitDeltaBundleScript, + isMissingGitPrerequisiteError, createImportedGitRef, createRemoteGitExportRef, deleteLocalGitRef, @@ -1063,41 +1064,60 @@ export async function prepareSandboxManagedRuntime(input: { const remoteGitBundle = path.posix.join(runtimeRootDir, "git-delta.bundle"); const remoteWorkspaceStatusPath = path.posix.join(runtimeRootDir, "workspace-status.txt"); const exportRef = createRemoteGitExportRef("sandbox"); - await input.client.run( - `sh -c ${shellQuote(buildRemoteGitDeltaBundleScript({ - remoteDir: workspaceRemoteDir, - baseSha: gitSnapshot.headCommit, + const localBundlePath = path.join(tempDir, "git-delta.bundle"); + + // Export the sandbox history and import it into the host workspace. + // The delta bundle assumes the host holds the bundle's boundary + // commit; when the host has been reset far enough that it does not, + // the import fails on a missing prerequisite. In that case re-export + // a full, self-contained bundle from the still-live sandbox rather + // than discard the completed run. + const exportAndImport = async (forceFullBundle: boolean): Promise => { + await input.client.run( + `sh -c ${shellQuote(buildRemoteGitDeltaBundleScript({ + remoteDir: workspaceRemoteDir, + baseSha: gitSnapshot.headCommit, + exportRef, + bundlePath: remoteGitBundle, + statusPath: forceFullBundle ? undefined : remoteWorkspaceStatusPath, + forceFullBundle, + }))}`, + { timeoutMs: input.spec.timeoutMs }, + ); + const gitExport = makeTransferProgress( + restoreSink, + "Exporting git history", + "from", + undefined, + { sink: input.onRuntimeProgress, phase: "export" }, + ); + const bundleBytes = await input.client.readFile(remoteGitBundle, gitExport.options); + const bundleBuffer = toBuffer(bundleBytes); + await gitExport.finish(bundleBuffer.byteLength, bundleBuffer.byteLength); + await input.client.remove(remoteGitBundle).catch(() => undefined); + if (!forceFullBundle) { + remoteWorkspaceStatus = await input.client.readFile(remoteWorkspaceStatusPath) + .then((bytes) => toBuffer(bytes).toString("utf8").trim()) + .catch(() => "dirty"); + remoteWorkspaceStatus = remoteWorkspaceStatus === "clean" ? "clean" : "dirty"; + await input.client.remove(remoteWorkspaceStatusPath).catch(() => undefined); + } + await fs.writeFile(localBundlePath, bundleBuffer); + return fetchGitBundleIntoLocalRef({ + localDir: input.workspaceLocalDir, + bundlePath: localBundlePath, exportRef, - bundlePath: remoteGitBundle, - statusPath: remoteWorkspaceStatusPath, - }))}`, - { timeoutMs: input.spec.timeoutMs }, - ); - const gitExport = makeTransferProgress( - restoreSink, - "Exporting git history", - "from", - undefined, - { sink: input.onRuntimeProgress, phase: "export" }, - ); - const bundleBytes = await input.client.readFile(remoteGitBundle, gitExport.options); - const bundleBuffer = toBuffer(bundleBytes); - await gitExport.finish(bundleBuffer.byteLength, bundleBuffer.byteLength); - await input.client.remove(remoteGitBundle).catch(() => undefined); - remoteWorkspaceStatus = await input.client.readFile(remoteWorkspaceStatusPath) - .then((bytes) => toBuffer(bytes).toString("utf8").trim()) - .catch(() => "dirty"); - remoteWorkspaceStatus = remoteWorkspaceStatus === "clean" ? "clean" : "dirty"; - await input.client.remove(remoteWorkspaceStatusPath).catch(() => undefined); - const bundlePath = path.join(tempDir, "git-delta.bundle"); - await fs.writeFile(bundlePath, bundleBuffer); - importedHead = await fetchGitBundleIntoLocalRef({ - localDir: input.workspaceLocalDir, - bundlePath, - exportRef, - importedRef, - baseSha: gitSnapshot.headCommit, - }); + importedRef: importedRef!, + baseSha: gitSnapshot.headCommit, + }); + }; + + try { + importedHead = await exportAndImport(false); + } catch (error) { + if (!isMissingGitPrerequisiteError(error)) throw error; + importedHead = await exportAndImport(true); + } } await emitRuntimeStatus(input.onRuntimeProgress, "restore", "Restoring workspace from sandbox");