From bac7307ec4b879e339f1cd8e99c131883c1d89b4 Mon Sep 17 00:00:00 2001 From: Devin Foley Date: Thu, 2 Jul 2026 17:06:34 -0700 Subject: [PATCH] fix(adapter-utils): improve sandbox restore failure diagnostics (#8903) 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. > - Agent adapters share managed-runtime helpers from `@paperclipai/adapter-utils` so local and sandboxed runs can prepare, execute, and restore workspaces consistently. > - The sandbox managed runtime syncs workspaces in both directions by creating tar archives on the local host and inside the remote sandbox. > - A prior fix avoided archiving `.` during upload because tar self-entries can force chmod/utime on a directory the command user does not own. > - The restore/download path still archived the remote workspace as `.`, and failed managed commands only surfaced stderr. > - This pull request applies entry-based tar creation to the sandbox restore path and includes stdout in managed command failure diagnostics. > - The benefit is that restore failures become visible to operators and sandbox workspace restore avoids the same directory-metadata failure class already fixed for upload. ## Linked Issues or Issue Description No existing public issue found; describing in-PR (bug). - **What happens:** sandbox workspace restore can fail while creating `workspace-download.tar` from the remote workspace when the tar command includes a `.` self-entry and the command user cannot update metadata on the workspace directory. If the failing command writes its diagnostic to stdout, the managed runtime error can collapse to a generic failed shell command without the useful tar message. - **Expected behavior:** restore should archive the workspace entries without a `.` self-entry, and failed managed runtime commands should include useful stdout/stderr diagnostics. - **Where:** `packages/adapter-utils/src/sandbox-managed-runtime.ts` restore/download path and `packages/adapter-utils/src/command-managed-runtime.ts` command error formatting. - **Related public context:** #7836 fixed the upload side of the same tar self-entry failure class. ## What Changed - Added stdout-aware failed-command formatting in the command managed runtime, keeping diagnostics bounded to the tail of stdout/stderr. - Added remote workspace tarball creation that names top-level entries explicitly instead of archiving `.` during sandbox restore. - Preserved empty-workspace restore support by creating a valid empty tarball when the remote workspace has no entries. - Added regression coverage for stdout diagnostics, restore tar members, and empty workspace restore tarballs. ## Verification - `pnpm vitest run packages/adapter-utils/src/command-managed-runtime.test.ts packages/adapter-utils/src/sandbox-managed-runtime.test.ts` — 2 files, 17 tests passed. - `pnpm --filter @paperclipai/adapter-utils typecheck` — passed. - `git diff --check origin/master..HEAD` — passed. - Local public-hygiene/PII scan of the committed diff checked for internal ticket refs, local/private URLs, common token/key patterns, private-key blocks, and email-like values — passed. - GitHub duplicate/related search found no public issue or PR for `workspace-download.tar Permission denied` or `sandbox restore tar permission denied`; #7836 is linked as related prior work. - PR CI on commit `3ad4c8d` — all GitHub Actions lanes, security scans, and aggregate `verify` passed. - Greptile Review on commit `3ad4c8d` — Confidence Score 5/5; the prior P2 thread is resolved with no open P2s, recommendations, or follow-ups. ## Risks Low. This is limited to shared adapter runtime error formatting and sandbox restore archive construction. Archive contents should remain equivalent apart from the removed `.` self-entry, and the new diagnostics are bounded to avoid dumping unbounded command output. ## Model Used OpenAI Codex, GPT-5, tool-enabled coding agent with shell and GitHub CLI access. Context window size was not reported by the runtime. ## 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 (n/a; internal adapter-runtime behavior only) - [x] I have considered and documented any risks above - [x] All Paperclip CI gates are green - [x] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge --------- Co-authored-by: Paperclip --- .../src/command-managed-runtime.test.ts | 20 ++++++ .../src/command-managed-runtime.ts | 19 +++++- .../src/sandbox-managed-runtime.test.ts | 64 +++++++++++++------ .../src/sandbox-managed-runtime.ts | 29 +++++++-- 4 files changed, 105 insertions(+), 27 deletions(-) diff --git a/packages/adapter-utils/src/command-managed-runtime.test.ts b/packages/adapter-utils/src/command-managed-runtime.test.ts index 1d97a20a60f..d9eb811780f 100644 --- a/packages/adapter-utils/src/command-managed-runtime.test.ts +++ b/packages/adapter-utils/src/command-managed-runtime.test.ts @@ -395,4 +395,24 @@ describe("command managed runtime", () => { expect(progress.every((entry) => entry.total === payload.length)).toBe(true); expect(progress.at(-1)?.done).toBe(payload.length); }); + + it("includes stdout diagnostics when a managed runtime command fails", async () => { + const startedAt = new Date().toISOString(); + const runner: CommandManagedRuntimeRunner = { + execute: async () => ({ + exitCode: 2, + signal: null, + timedOut: false, + stdout: "tar: workspace-download.tar: Cannot open: Permission denied\n", + stderr: "", + pid: null, + startedAt, + }), + }; + const client = createCommandManagedRuntimeClient({ runner, commandCwd: "/", timeoutMs: 30_000 }); + + await expect(client.run("tar -cf workspace-download.tar .", { timeoutMs: 30_000 })).rejects.toThrow( + /stdout: tar: workspace-download\.tar: Cannot open: Permission denied/, + ); + }); }); diff --git a/packages/adapter-utils/src/command-managed-runtime.ts b/packages/adapter-utils/src/command-managed-runtime.ts index beb0d4dbdfe..77acad78cbe 100644 --- a/packages/adapter-utils/src/command-managed-runtime.ts +++ b/packages/adapter-utils/src/command-managed-runtime.ts @@ -69,10 +69,25 @@ function toBuffer(bytes: Buffer | Uint8Array | ArrayBuffer): Buffer { return Buffer.from(bytes.buffer, bytes.byteOffset, bytes.byteLength); } +const FAILED_COMMAND_OUTPUT_TAIL_CHARS = 4_000; + +function formatFailedCommandOutput(result: RunProcessResult): string { + const tail = (text: string): string => { + const trimmed = text.trim(); + if (trimmed.length <= FAILED_COMMAND_OUTPUT_TAIL_CHARS) return trimmed; + return `...[truncated]\n${trimmed.slice(-FAILED_COMMAND_OUTPUT_TAIL_CHARS)}`; + }; + const stderr = tail(result.stderr); + const stdout = tail(result.stdout); + const parts: string[] = []; + if (stderr.length > 0) parts.push(`stderr: ${stderr}`); + if (stdout.length > 0) parts.push(`stdout: ${stdout}`); + return parts.length > 0 ? `:\n${parts.join("\n")}` : ""; +} + function requireSuccessfulResult(result: RunProcessResult, action: string): void { if (result.exitCode === 0 && !result.timedOut) return; - const stderr = result.stderr.trim(); - const detail = stderr.length > 0 ? `: ${stderr}` : ""; + const detail = formatFailedCommandOutput(result); throw new Error(`${action} failed with exit code ${result.exitCode ?? "null"}${detail}`); } diff --git a/packages/adapter-utils/src/sandbox-managed-runtime.test.ts b/packages/adapter-utils/src/sandbox-managed-runtime.test.ts index 6a002521ce1..5338a8e7680 100644 --- a/packages/adapter-utils/src/sandbox-managed-runtime.test.ts +++ b/packages/adapter-utils/src/sandbox-managed-runtime.test.ts @@ -507,8 +507,9 @@ describe("sandbox managed runtime", () => { await writeFile(path.join(localWorkspaceDir, "src", "main.ts"), "x\n", "utf8"); await writeFile(path.join(localAssetsDir, "asset.txt"), "a\n", "utf8"); - // Capture every tar uploaded to the sandbox so we can inspect its members. + // Capture every tar uploaded/downloaded through the sandbox so we can inspect its members. const uploadedTars: { remotePath: string; bytes: Buffer }[] = []; + const downloadedTars: { remotePath: string; bytes: Buffer }[] = []; const client: SandboxManagedRuntimeClient = { makeDir: async (remotePath) => { await mkdir(remotePath, { recursive: true }); @@ -519,7 +520,11 @@ describe("sandbox managed runtime", () => { if (remotePath.endsWith("-upload.tar")) uploadedTars.push({ remotePath, bytes: buffer }); await writeFile(remotePath, buffer); }, - readFile: async (remotePath) => await readFile(remotePath), + readFile: async (remotePath) => { + const buffer = await readFile(remotePath); + if (remotePath.endsWith("workspace-download.tar")) downloadedTars.push({ remotePath, bytes: buffer }); + return buffer; + }, listFiles: async () => [], remove: async (remotePath) => { await rm(remotePath, { recursive: true, force: true }); @@ -529,7 +534,7 @@ describe("sandbox managed runtime", () => { }, }; - await prepareSandboxManagedRuntime({ + const prepared = await prepareSandboxManagedRuntime({ spec: { transport: "sandbox", provider: "test", @@ -559,6 +564,12 @@ describe("sandbox managed runtime", () => { // And the workspace still extracts correctly into an existing target dir. await expect(readFile(path.join(remoteWorkspaceDir, "README.md"), "utf8")).resolves.toBe("ws\n"); await expect(readFile(path.join(remoteWorkspaceDir, "src", "main.ts"), "utf8")).resolves.toBe("x\n"); + + await prepared.restoreWorkspace(); + expect(downloadedTars).toHaveLength(1); + const downloadMembers = await listTarMembers(rootDir, "workspace-download-list.tar", downloadedTars[0]!.bytes); + expect(downloadMembers).not.toContain("."); + expect(downloadMembers).not.toContain("./"); }); it("excludes transient symlinked home dirs from the asset tar while keeping required content", async () => { @@ -735,13 +746,15 @@ describe("sandbox managed runtime", () => { expect(restoreLines.some((line) => line.includes("100%"))).toBe(true); }); - it("creates a valid empty workspace tarball when the local workspace is empty", async () => { + it("creates valid empty workspace tarballs when the workspace is empty", async () => { const rootDir = await mkdtemp(path.join(os.tmpdir(), "paperclip-sandbox-empty-")); cleanupDirs.push(rootDir); const localWorkspaceDir = path.join(rootDir, "local-workspace"); const remoteWorkspaceDir = path.join(rootDir, "remote-workspace"); await mkdir(localWorkspaceDir, { recursive: true }); + const downloadedTars: { remotePath: string; bytes: Buffer }[] = []; + const runCommands: string[] = []; const client: SandboxManagedRuntimeClient = { makeDir: async (remotePath) => { await mkdir(remotePath, { recursive: true }); @@ -750,30 +763,41 @@ describe("sandbox managed runtime", () => { await mkdir(path.dirname(remotePath), { recursive: true }); await writeFile(remotePath, Buffer.from(bytes)); }, - readFile: async (remotePath) => await readFile(remotePath), + readFile: async (remotePath) => { + const buffer = await readFile(remotePath); + if (remotePath.endsWith("workspace-download.tar")) downloadedTars.push({ remotePath, bytes: buffer }); + return buffer; + }, listFiles: async () => [], remove: async (remotePath) => { await rm(remotePath, { recursive: true, force: true }); }, run: async (command) => { + runCommands.push(command); await execFile("sh", ["-c", command], { maxBuffer: 32 * 1024 * 1024 }); }, }; - await expect( - prepareSandboxManagedRuntime({ - spec: { - transport: "sandbox", - provider: "test", - sandboxId: "sandbox-1", - remoteCwd: remoteWorkspaceDir, - timeoutMs: 30_000, - apiKey: null, - }, - adapterKey: "test-adapter", - client, - workspaceLocalDir: localWorkspaceDir, - }), - ).resolves.toBeDefined(); + const prepared = await prepareSandboxManagedRuntime({ + spec: { + transport: "sandbox", + provider: "test", + sandboxId: "sandbox-1", + remoteCwd: remoteWorkspaceDir, + timeoutMs: 30_000, + apiKey: null, + }, + adapterKey: "test-adapter", + client, + workspaceLocalDir: localWorkspaceDir, + }); + + await prepared.restoreWorkspace(); + expect(downloadedTars).toHaveLength(1); + const members = await listTarMembers(rootDir, "empty-workspace-download.tar", downloadedTars[0]!.bytes); + expect(members).toEqual([]); + const emptyArchiveCommand = runCommands.find((command) => command.includes("dd if=/dev/zero")); + expect(emptyArchiveCommand).toBeDefined(); + expect(emptyArchiveCommand).not.toContain("/dev/null"); }); }); diff --git a/packages/adapter-utils/src/sandbox-managed-runtime.ts b/packages/adapter-utils/src/sandbox-managed-runtime.ts index e6fb7355d95..cbaed7298e3 100644 --- a/packages/adapter-utils/src/sandbox-managed-runtime.ts +++ b/packages/adapter-utils/src/sandbox-managed-runtime.ts @@ -323,6 +323,25 @@ function tarExcludeFlags(exclude: string[] | undefined): string { return ["._*", ...(exclude ?? [])].map((entry) => `--exclude ${shellQuote(entry)}`).join(" "); } +function createRemoteTarballFromDirectoryCommand(input: { + remoteDir: string; + archivePath: string; + exclude?: string[]; +}): string { + // Match the local archive path: name top-level entries explicitly so tar + // does not include a "." self-entry that it later tries to chmod/utime. + return [ + `mkdir -p ${shellQuote(path.posix.dirname(input.archivePath))}`, + `cd ${shellQuote(input.remoteDir)}`, + "set -- *", + `if [ "$#" -eq 1 ] && [ "$1" = "*" ] && [ ! -e "$1" ] && [ ! -L "$1" ]; then set --; fi`, + `for entry in .[!.]* ..?*; do [ -e "$entry" ] || [ -L "$entry" ] || continue; set -- "$@" "$entry"; done`, + `if [ "$#" -eq 0 ]; then ` + + `dd if=/dev/zero of=${shellQuote(input.archivePath)} bs=1024 count=1; ` + + `else tar -cf ${shellQuote(input.archivePath)} ${tarExcludeFlags(input.exclude)} -- "$@"; fi`, + ].join(" && "); +} + async function emitRuntimeStatus( sink: RuntimeStatusSink | undefined, phase: RuntimeStatusPhase, @@ -586,11 +605,11 @@ export async function prepareSandboxManagedRuntime(input: { const remoteWorkspaceTar = path.posix.join(runtimeRootDir, "workspace-download.tar"); await emitRuntimeStatus(input.onRuntimeProgress, "restore", "Restoring workspace from sandbox"); await input.client.run( - `sh -c ${shellQuote( - `mkdir -p ${shellQuote(runtimeRootDir)} && ` + - `tar -cf ${shellQuote(remoteWorkspaceTar)} -C ${shellQuote(workspaceRemoteDir)} ` + - `${tarExcludeFlags(restoreExclude)} .`, - )}`, + `sh -c ${shellQuote(createRemoteTarballFromDirectoryCommand({ + remoteDir: workspaceRemoteDir, + archivePath: remoteWorkspaceTar, + exclude: restoreExclude, + }))}`, { timeoutMs: input.spec.timeoutMs }, ); const workspaceRestore = makeTransferProgress(restoreSink, "Restoring", "from", "workspace");