From 7f5d0793bd66b30e546b1020b8f722e012845264 Mon Sep 17 00:00:00 2001 From: Devin Foley Date: Thu, 5 Nov 2026 11:34:41 -0800 Subject: [PATCH] fix(daytona): record a bounded errno when a sandbox command fails (#19297) 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 > - The Daytona sandbox provider copies the agent's workspace back to the host after each turn (syncOut) > - When the sandbox `tar` fails in the `archive_create` step, the restore diagnostic records only `exitCode: 2` and `errorCode: "unknown"` > - The command output is not sent across the plugin RPC boundary, so an operator cannot tell a full disk from a permission error or a path that is too long > - This pull request maps the system error text at the end of a failed sandbox command's output line to the existing allowlisted `errorCode` > - The benefit is that the next failure records the real errno, and the output text stays private ## Linked Issues or Issue Description Refs #18172 Refs #18212 **What happened?** A hosted Daytona run failed its workspace restore. The `archive_create` step exited 2 on all three attempts. The run log recorded `{"step":"workspace_transfer","transferStep":"archive_create","transferFailureKind":"command_failed","exitCode":2,"errorCode":"unknown"}`. Two causes are possible. The sandbox disk was full, or a path was longer than PATH_MAX, for example from deeply nested GitHub helper config directories before #18212. The diagnostic cannot tell these causes apart. **Expected behavior** A failed sandbox command records a bounded errno such as `ENOSPC`, `EACCES` or `ENAMETOOLONG` when its output shows one. The output text itself must not be recorded. **Steps to reproduce** 1. Run `performSyncOut` against a sandbox whose `tar -c` exits 2 with `tar: ./x: Cannot open: Permission denied`. 2. Read the environment sync diagnostic from the thrown error. 3. See that `errorCode` is `unknown`. **Paperclip version or commit** `master` at 3fca8a1. ## What Changed - `packages/plugins/sandbox-providers/daytona/src/file-sync.ts`: `assertSandboxCommandOkWithOutput` maps the strerror suffix of a failed command's output to an allowlisted errno. It reads only the last 16 KiB in memory. A line must end with `: `. Fatal causes are checked first (ENOSPC, EIO, ENAMETOOLONG, EACCES, EPERM, ENOTDIR, EISDIR, ENOENT), so a warning about a vanished file does not hide the error that stopped `tar`. The error message, capacity fallback (#18172), restore and retain-on-failure behavior do not change. - `packages/plugins/sdk/src/environment-sync-error.ts` and `packages/adapter-utils/src/workspace-restore-diagnostics.ts`: add `ENAMETOOLONG` to both allowlists. Both sides still validate independently. - Tests: 7 new Daytona cases, plus one allowlist case each in the SDK and adapter-utils. ## Verification - `vitest run --config packages/plugins/sandbox-providers/daytona/vitest.config.ts packages/plugins/sandbox-providers/daytona/src/file-sync-diagnostics.test.ts`: 18 passed. Without the `file-sync.ts` change, the 7 new cases fail with `errorCode: "unknown"`. - `packages/plugins/sdk`: `vitest run tests/environment-sync-error.test.ts`: 20 passed. - `packages/adapter-utils`: `vitest run src/workspace-restore-diagnostics.test.ts`: 50 passed. - `tsc --noEmit` passes for the SDK and adapter-utils. `git diff --check` is clean. - I did not run the Daytona package typecheck or its full suite locally because the sandbox disk was full. CI covers them. ## Risks - Low risk. The change adds a diagnostic field only. Control flow, error messages and the capacity fallback regex do not change. - Localized `tar` output does not match, so it stays `unknown`. That is the current behavior. - `ENAMETOOLONG` is a new allowlisted code on both sides. It is a fixed string and carries no path or output text. ## Model Used - Claude Opus 5.5 (`claude-opus-5-5`), Anthropic, with tool use and code execution in an agent harness. ## 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 - [ ] 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 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5.5 --- .../src/workspace-restore-diagnostics.test.ts | 7 +++ .../src/workspace-restore-diagnostics.ts | 2 +- .../daytona/src/file-sync-diagnostics.test.ts | 47 +++++++++++++++++++ .../daytona/src/file-sync.ts | 27 ++++++++++- .../plugins/sdk/src/environment-sync-error.ts | 2 +- .../sdk/tests/environment-sync-error.test.ts | 1 + 6 files changed, 83 insertions(+), 3 deletions(-) diff --git a/packages/adapter-utils/src/workspace-restore-diagnostics.test.ts b/packages/adapter-utils/src/workspace-restore-diagnostics.test.ts index 936b68e302c..492d09ed8ee 100644 --- a/packages/adapter-utils/src/workspace-restore-diagnostics.test.ts +++ b/packages/adapter-utils/src/workspace-restore-diagnostics.test.ts @@ -113,6 +113,13 @@ describe("workspace restore diagnostics", () => { }, ); + it("keeps an allowlisted ENAMETOOLONG code", async () => { + const sink = vi.fn(); + const error = Object.assign(new Error("private"), { code: "ENAMETOOLONG" }); + await expect(withWorkspaceRestoreDiagnostics("workspace", async () => { throw error; }, sink)).rejects.toBe(error); + expect(sink.mock.calls[0]?.[0]).toContain('"errorCode":"ENAMETOOLONG"'); + }); + it("bounds cause traversal even when it cycles", async () => { const error = Object.assign(new Error("private"), { cause: null as unknown, code: "ENOENT" }); error.cause = error; diff --git a/packages/adapter-utils/src/workspace-restore-diagnostics.ts b/packages/adapter-utils/src/workspace-restore-diagnostics.ts index 8ac378374f8..ab5d42d509d 100644 --- a/packages/adapter-utils/src/workspace-restore-diagnostics.ts +++ b/packages/adapter-utils/src/workspace-restore-diagnostics.ts @@ -40,7 +40,7 @@ export interface WorkspaceRestoreDiagnostic { rpcCode?: number; } const ERROR_CODES = new Set([ - "ENOENT", "EACCES", "EPERM", "ENOSPC", "EIO", "EXDEV", "ENOTDIR", "EISDIR", + "ENOENT", "EACCES", "EPERM", "ENOSPC", "EIO", "EXDEV", "ENOTDIR", "EISDIR", "ENAMETOOLONG", "ECONNRESET", "ECONNREFUSED", "ETIMEDOUT", "EPIPE", "ENOTFOUND", "EAI_AGAIN", "ABORT_ERR", "UND_ERR_CONNECT_TIMEOUT", "UND_ERR_SOCKET", ]); diff --git a/packages/plugins/sandbox-providers/daytona/src/file-sync-diagnostics.test.ts b/packages/plugins/sandbox-providers/daytona/src/file-sync-diagnostics.test.ts index 4e5e2da1cee..e761e460fbd 100644 --- a/packages/plugins/sandbox-providers/daytona/src/file-sync-diagnostics.test.ts +++ b/packages/plugins/sandbox-providers/daytona/src/file-sync-diagnostics.test.ts @@ -54,6 +54,53 @@ it.each([ }); }); +it.each([ + ["tar: ./private-file: Cannot open: Permission denied\ntar: Exiting with failure status due to previous errors", "EACCES"], + ["tar: ./private-dir: Cannot savedir: Operation not permitted", "EPERM"], + ["tar: ./private-file: Cannot stat: No such file or directory", "ENOENT"], + ["tar: ./private-dir/private-file: Cannot open: File name too long", "ENAMETOOLONG"], + ["tar: ./private-file: Read error at byte 0, while reading 512 bytes: Input/output error", "EIO"], + // A vanished file must not hide the error that stopped tar. + ["tar: ./private-file: Cannot stat: No such file or directory\ntar: ./private-dir: Cannot open: Permission denied", "EACCES"], + ["private-command-output mentions Permission denied mid-line only", "unknown"], +] as const)("classifies a failed sandbox tar's output as a bounded errno: %s", async (output, errorCode) => { + const sandbox = { + process: { executeCommand: async (command: string) => + command.includes("tar -c") ? { exitCode: 2, result: output } : { exitCode: 0 } }, + fs: { downloadFiles: async () => [], deleteFile: async () => undefined }, + }; + await withEnvironmentSyncErrorCapture(async () => { + let caught: unknown; + try { + await performSyncOut({ sandbox: sandbox as never, remoteDir: "/workspace", timeoutSeconds: 30, + operations: [{ operationId: "fixture", files: [{ sourcePath: "/workspace/private-path", targetPath: "/unused", kind: "directory" }] }], + }); + } catch (error) { caught = error; } + expect(caught).toBeInstanceOf(Error); + const diagnostic = readEnvironmentSyncErrorDiagnostic({ data: environmentSyncErrorData(caught) }); + expect(diagnostic).toEqual({ errorCode, transferStep: "archive_create", transferFailureKind: "command_failed", exitCode: 2 }); + expect(JSON.stringify(diagnostic)).not.toContain("private"); + }); +}); + +it("classifies a full sandbox disk at the guard step without recording output", async () => { + const sandbox = { + process: { executeCommand: async () => ({ exitCode: 1, result: "mkdir: cannot create directory '/workspace/private-path': No space left on device" }) }, + fs: { downloadFiles: async () => [], deleteFile: async () => undefined }, + }; + await withEnvironmentSyncErrorCapture(async () => { + let caught: unknown; + try { + await performSyncOut({ sandbox: sandbox as never, remoteDir: "/workspace", timeoutSeconds: 30, + operations: [{ operationId: "fixture", files: [{ sourcePath: "/workspace/private-path", targetPath: "/unused", kind: "directory" }] }], + }); + } catch (error) { caught = error; } + const diagnostic = readEnvironmentSyncErrorDiagnostic({ data: environmentSyncErrorData(caught) }); + expect(diagnostic).toMatchObject({ errorCode: "ENOSPC", transferFailureKind: "command_failed", exitCode: 1 }); + expect(JSON.stringify(diagnostic)).not.toContain("private"); + }); +}); + it("keeps a thrown download exception identical and its nested safe code", async () => { const original = Object.freeze(new Error("private-download-error", { cause: { code: "ECONNRESET", token: "private-token" } })); const sandbox = { diff --git a/packages/plugins/sandbox-providers/daytona/src/file-sync.ts b/packages/plugins/sandbox-providers/daytona/src/file-sync.ts index f067a29130d..8ea41f23363 100644 --- a/packages/plugins/sandbox-providers/daytona/src/file-sync.ts +++ b/packages/plugins/sandbox-providers/daytona/src/file-sync.ts @@ -418,6 +418,31 @@ async function countHostFiles(root: string, exclude?: string[]): Promise return total; } +// Fatal causes first: tar can warn about a vanished file before the error that +// stopped it. Daytona's executeCommand result already includes stderr. +const SANDBOX_ERRNO_MESSAGES: ReadonlyArray = [ + ["ENOSPC", "No space left on device"], + ["EIO", "Input/output error"], + ["ENAMETOOLONG", "File name too long"], + ["EACCES", "Permission denied"], + ["EPERM", "Operation not permitted"], + ["ENOTDIR", "Not a directory"], + ["EISDIR", "Is a directory"], + ["ENOENT", "No such file or directory"], +]; + +/** + * Map the strerror suffix of a failed command's output to an allowlisted errno. + * Reads only a bounded tail in memory; the output text itself is never recorded. + */ +function sandboxCommandErrorCode(output: string): string | undefined { + const lines = output.slice(-16 * 1024).split(/\r?\n/).map((line) => line.trimEnd()); + for (const [code, message] of SANDBOX_ERRNO_MESSAGES) { + if (lines.some((line) => line.endsWith(`: ${message}`))) return code; + } + return undefined; +} + /** * Run one sandbox command and return its stdout on success. Throws the same * shaped error as {@link assertSandboxCommandOk} on a non-zero exit. Used by @@ -434,7 +459,7 @@ async function assertSandboxCommandOkWithOutput( if ((result.exitCode ?? 1) !== 0) { const detail = (result.result ?? result.artifacts?.stdout ?? "").toString().trim(); throw recordEnvironmentSyncError(new Error(`Daytona ${label} command failed (exit ${result.exitCode ?? "unknown"})${detail ? `: ${detail}` : ""}`), { - transferFailureKind: "command_failed", exitCode: result.exitCode, + transferFailureKind: "command_failed", exitCode: result.exitCode, errorCode: sandboxCommandErrorCode(detail), }); } return (result.result ?? result.artifacts?.stdout ?? "").toString(); diff --git a/packages/plugins/sdk/src/environment-sync-error.ts b/packages/plugins/sdk/src/environment-sync-error.ts index a630bb126e1..7d6a51a95da 100644 --- a/packages/plugins/sdk/src/environment-sync-error.ts +++ b/packages/plugins/sdk/src/environment-sync-error.ts @@ -68,7 +68,7 @@ const SCHEMA = "paperclip/environment-sync-error/v1"; // Match the host's workspace restore diagnostic allowlist. Both sides validate // independently: plugin-supplied data must never widen the host's log surface. const ERROR_CODES = new Set([ - "ENOENT", "EACCES", "EPERM", "ENOSPC", "EIO", "EXDEV", "ENOTDIR", "EISDIR", + "ENOENT", "EACCES", "EPERM", "ENOSPC", "EIO", "EXDEV", "ENOTDIR", "EISDIR", "ENAMETOOLONG", "ECONNRESET", "ECONNREFUSED", "ETIMEDOUT", "EPIPE", "ENOTFOUND", "EAI_AGAIN", "ABORT_ERR", "UND_ERR_CONNECT_TIMEOUT", "UND_ERR_SOCKET", ]); diff --git a/packages/plugins/sdk/tests/environment-sync-error.test.ts b/packages/plugins/sdk/tests/environment-sync-error.test.ts index c2480a707ce..f86196f0ba3 100644 --- a/packages/plugins/sdk/tests/environment-sync-error.test.ts +++ b/packages/plugins/sdk/tests/environment-sync-error.test.ts @@ -34,6 +34,7 @@ describe("environment sync diagnostic envelope", () => { const wrap = (cause: unknown) => ({ cause }); expect(environmentSyncErrorData(wrap(wrap(wrap({ code: "EIO" }))))).toEqual({ schema, diagnostic: { errorCode: "EIO" } }); expect(environmentSyncErrorData(wrap(wrap(wrap(wrap({ code: "EIO" })))))).toBeUndefined(); + expect(environmentSyncErrorData({ code: "ENAMETOOLONG" })).toEqual({ schema, diagnostic: { errorCode: "ENAMETOOLONG" } }); }); it("revalidates worker fields and ignores arbitrary payload and protocol versions", () => {