mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-10-29 12:59:13 +01:00
fix(adapter-utils): improve sandbox restore failure diagnostics (#8903)
## 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 <noreply@paperclip.ing>
This commit is contained in:
1 parent
246e1b38bf
commit
bac7307ec4
4 files changed
+105
-27
No files matched your search
@@ -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/,
|
||||
);
|
||||
});
|
||||
});
|
||||
@@ -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}`);
|
||||
}
|
||||
|
||||
|
||||
@@ -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");
|
||||
});
|
||||
});
|
||||
@@ -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");
|
||||
|
||||
Reference in new issue
Block a user