From 5e26b733b9668d24b005418704d43f66109a7dd8 Mon Sep 17 00:00:00 2001 From: Devin Foley Date: Sat, 24 Oct 2026 00:30:06 -0700 Subject: [PATCH] fix(server): finish workspace operations when the log write fails (#17415) 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. > - Before a run starts, Paperclip prepares the execution workspace (for example `git worktree add`). Each step is a workspace operation. The workspace operation recorder writes it as a row and as a log file on disk. > - When an operation fails, the recorder writes the error to the log file, then sets the row to `failed`. > - If that log write also fails (for example ENOSPC when the disk is full), the recorder throws before it updates the row. > - The row stays `running` with no `finishedAt`. The log write error also replaces the real error of the operation. > - This pull request makes the failure-path log write best-effort, so the row always reaches `failed` and callers get the correct error. > - The benefit: on a full disk, the operator sees a terminal operation with the real failure reason. Operations do not stay "running" for ever. ## Linked Issues or Issue Description No public issue exists. Bug description: **What happened?** A run failed during workspace setup on a host with a full disk (`ENOSPC: no space left on device, write`). The stack shows the error in the workspace operation log append inside `recordOperation`. In the failure path, the recorder calls the log append before it updates the row. Because the append threw again, the row stayed `running` with no `finishedAt`. When the operation itself failed (for example a git error), callers got the ENOSPC log error and not the git error. **Expected behavior** A failed workspace operation always reaches `failed` with `finishedAt` set. Callers get the operation's own error. A log write failure must not hide that error. **Steps to reproduce** 1. Make the operation log append throw ENOSPC (the new tests stub `fs.appendFile` for the log root). 2. Record an operation whose `run` throws, or whose output append throws. 3. Before this fix: the row stays `running`, and the caller gets the ENOSPC error and not the operation's error. **Paperclip version or commit** `ee29e28` (current `master`). **Agent adapter(s) involved** Not adapter-specific (core bug). Related open PRs with disk-full handling in other places (no overlap with this code path): #5882 (run-log rotation), #16773 (backup disk use). ## What Changed - `server/src/services/workspace-operations.ts`: in the `recordOperation` failure path, the stderr log append is now best-effort (`.catch(() => undefined)`). The stderr excerpt still records the message, because the excerpt updates before the file write. `logStore.finalize` was already best-effort in this path. - `server/src/__tests__/workspace-operations-reconciliation.test.ts`: two regression tests with log appends that fail with ENOSPC: - The output append fails after a successful run. The row reaches `failed` with `finishedAt` and the ENOSPC message in the stderr excerpt. The caller gets the ENOSPC error. - The operation itself throws. The row reaches `failed` and the caller gets the operation's own error, not the log write error. ## Verification - Red before the fix: both new tests failed on `master`. The row stayed `running`, and the caller got the ENOSPC error and not the git error. - Green after the fix: `pnpm exec vitest run src/__tests__/workspace-operations-reconciliation.test.ts src/__tests__/workspace-runtime-control-recovery.test.ts` passes (17 tests). - Sibling suites that use the recorder (26 files, 945 tests): all pass except 4 tests in `workspace-runtime.test.ts`. Those 4 tests also fail on unchanged `master` in the same sandbox, because they need the local pnpm and worktree CLI. This change does not cause them. - Typecheck: `tsc --noEmit` on `workspace-operations.ts` passes. The full server `tsc` was not run locally because the 4 GB sandbox ran out of memory. CI runs the full typecheck. ## Risks - Low risk. The change affects only the failure path, and only the log file write. The success path does not change: if a log write fails after a successful run, the operation still fails with the write error. - The log file for a failed operation can lose its last stderr line on a full disk. The row's stderr excerpt still has the message. ## Model Used - Claude Opus 5.5 (`claude-opus-5-5`), Anthropic, used through Claude Code with tool use (shell, file edit, test execution). ## 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 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Devin Foley <139239+devinfoley@users.noreply.github.com> Co-authored-by: Claude Opus 5.5 --- ...orkspace-operations-reconciliation.test.ts | 87 ++++++++++++++++++- server/src/services/workspace-operations.ts | 5 +- 2 files changed, 90 insertions(+), 2 deletions(-) diff --git a/server/src/__tests__/workspace-operations-reconciliation.test.ts b/server/src/__tests__/workspace-operations-reconciliation.test.ts index 9133fbae0e6..03d1d80bbeb 100644 --- a/server/src/__tests__/workspace-operations-reconciliation.test.ts +++ b/server/src/__tests__/workspace-operations-reconciliation.test.ts @@ -2,7 +2,7 @@ import { randomUUID } from "node:crypto"; import fs from "node:fs/promises"; import os from "node:os"; import path from "node:path"; -import { afterAll, afterEach, beforeAll, describe, expect, it } from "vitest"; +import { afterAll, afterEach, beforeAll, describe, expect, it, vi } from "vitest"; import { companies, createDb, @@ -271,4 +271,89 @@ describeEmbeddedPostgres("workspace operation reconciliation", () => { "Workspace repair target_backup: succeeded.", ); }); + + describe("when the operation log file cannot be written", () => { + // The log file is created, then every append hits a full disk (PAPERCLIP-BACKEND-4H). + function failLogAppends() { + const enospc = Object.assign(new Error("ENOSPC: no space left on device, write"), { + code: "ENOSPC", + errno: -28, + syscall: "write", + }); + const appendFile = fs.appendFile; + const spy = vi.spyOn(fs, "appendFile").mockImplementation(async (file, ...rest) => { + if (typeof file === "string" && file.startsWith(logRoot)) throw enospc; + return appendFile(file, ...rest); + }); + return { enospc, spy }; + } + + async function seedCompany() { + const companyId = randomUUID(); + await db.insert(companies).values({ + id: companyId, + name: "Workspace operation full disk", + issuePrefix: `F${companyId.replace(/-/g, "").slice(0, 7).toUpperCase()}`, + requireBoardApprovalForNewAgents: false, + }); + return companyId; + } + + async function operationRows(companyId: string) { + return db.select().from(workspaceOperations).where(eq(workspaceOperations.companyId, companyId)); + } + + it("terminalizes the operation and surfaces the write error", async () => { + const companyId = await seedCompany(); + const { enospc, spy } = failLogAppends(); + try { + await expect( + workspaceOperationService(db) + .createRecorder({ companyId }) + .recordOperation({ + phase: "worktree_prepare", + command: "git worktree add", + cwd: "/tmp/full-disk-workspace", + run: async () => ({ status: "succeeded", exitCode: 0, stdout: "Preparing worktree\n" }), + }), + ).rejects.toBe(enospc); + } finally { + spy.mockRestore(); + } + + const rows = await operationRows(companyId); + expect(rows).toHaveLength(1); + expect(rows[0]).toMatchObject({ status: "failed", phase: "worktree_prepare" }); + expect(rows[0]?.finishedAt).toBeInstanceOf(Date); + expect(rows[0]?.stderrExcerpt).toContain("ENOSPC: no space left on device, write"); + }); + + it("keeps the operation's own error instead of the log write error", async () => { + const companyId = await seedCompany(); + const { spy } = failLogAppends(); + const gitError = new Error("fatal: could not create work tree dir"); + try { + await expect( + workspaceOperationService(db) + .createRecorder({ companyId }) + .recordOperation({ + phase: "worktree_prepare", + command: "git worktree add", + cwd: "/tmp/full-disk-workspace", + run: async () => { + throw gitError; + }, + }), + ).rejects.toBe(gitError); + } finally { + spy.mockRestore(); + } + + const rows = await operationRows(companyId); + expect(rows).toHaveLength(1); + expect(rows[0]).toMatchObject({ status: "failed" }); + expect(rows[0]?.finishedAt).toBeInstanceOf(Date); + expect(rows[0]?.stderrExcerpt).toContain("fatal: could not create work tree dir"); + }); + }); }); diff --git a/server/src/services/workspace-operations.ts b/server/src/services/workspace-operations.ts index 233773bb110..53fcbf5292c 100644 --- a/server/src/services/workspace-operations.ts +++ b/server/src/services/workspace-operations.ts @@ -654,7 +654,10 @@ export function workspaceOperationService(db: Db) { if (!row) throw notFound("Workspace operation not found"); return toWorkspaceOperation(row); } catch (error) { - await append("stderr", error instanceof Error ? error.message : String(error)); + // The log file is best-effort once the operation has failed: a log write error (for + // example ENOSPC on a full disk) must neither strand this row in `running` nor replace + // the operation's own error. The excerpt still records the message. + await append("stderr", error instanceof Error ? error.message : String(error)).catch(() => undefined); const finalized = await logStore.finalize(handle).catch(() => null); const finishedAt = new Date(); await db