From b3bf12a5dbf5457bc762bc92220d350e0c5de461 Mon Sep 17 00:00:00 2001 From: Dotta <34892728+cryppadotta@users.noreply.github.com> Date: Mon, 9 Nov 2026 06:25:15 -0600 Subject: [PATCH] fix(workspaces): close a retained local task directory as archived, not cleanup_failed (#20248) 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 > - A task runs in an execution workspace. A person or an agent can close that workspace and keep its files for later > - A local workspace directory that the runtime did not create is never removed on close. Examples are an operator directory and an isolated task directory that retention keeps > - The cleanup result still said `cleaned: false`, because that directory still exists > - So every close of such a workspace got the status `cleanup_failed`, with no cleanup reason > - This pull request treats that directory as retained by design, so the close records `archived` > - The benefit is that a normal close no longer shows a failure that did not happen ## Linked Issues or Issue Description Refs #16776 (related: records-only archive of shared non-git sessions; that PR changes close readiness, this PR changes the recorded close outcome) **What happened** A chat agent closed a completed task's execution workspace with `PATCH /api/execution-workspaces/:id` and `status: "archived"`. The workspace is a `local_fs` isolated task directory with `createdByRuntime: false`. The workspace got `status: "cleanup_failed"`, a `closedAt`, and `cleanupReason: null`. The files stayed, as intended. The agent then had to tell the person that cleanup failed. **Expected behavior** The close records `status: "archived"`. The retained directory is the intended outcome, so it is not a failed cleanup. A runtime-created directory is still removed. **Steps to reproduce** 1. Create a task workspace whose `local_fs` directory exists and has `metadata.createdByRuntime: false` (for example an isolated task directory). 2. Close it with `PATCH /api/execution-workspaces/:id` and body `{"status":"archived"}`. 3. Read the workspace. Its status is `cleanup_failed` and `cleanupReason` is `null`. ## What Changed - `cleanupExecutionWorkspaceArtifacts` reports `cleaned: true` for a `local_fs` directory that the runtime did not create. It never removes that directory, so nothing is left to clean. - Added `server/src/__tests__/workspace-cleanup-retained-local-dir.test.ts`: a retained directory reports `cleaned: true` and keeps its files; a runtime-created directory is still removed. ## Verification - `pnpm exec vitest run server/src/__tests__/workspace-cleanup-retained-local-dir.test.ts` fails before the change (`cleaned: false`) and passes after it. - `server/src/__tests__/workspace-runtime.test.ts` (179), `execution-workspaces-routes.test.ts` (17), `execution-workspaces-service.test.ts` (85) and `workspace-delivery-cleanup.integration.test.ts` (19) pass. - End to end: the chat eval `workspaces.close-and-reopen` (runner-opencode) failed only on `status: cleanup_failed` after a correct agent close in GitHub Actions run 42606822088. This fix is cherry-picked onto the eval branch for a rerun. ## Risks - Low risk. Only `local_fs` workspaces with `createdByRuntime` not `true` change. Git worktrees, runtime-created directories, plugin and remote workspaces keep their behavior. - The terminal reaper uses the same cleanup function. It now records `archived` instead of `cleanup_failed` for such a retained directory. Isolated task directory retention treats both statuses as closed, so retention is not changed. ## Model Used - Anthropic Claude Opus 5.5 (`claude-opus-5-5`), tool use and code execution in Claude Code. ## 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: Claude Opus 5.5 --- ...rkspace-cleanup-retained-local-dir.test.ts | 33 +++++++++++++++++++ server/src/services/workspace-runtime.ts | 5 +++ 2 files changed, 38 insertions(+) create mode 100644 server/src/__tests__/workspace-cleanup-retained-local-dir.test.ts diff --git a/server/src/__tests__/workspace-cleanup-retained-local-dir.test.ts b/server/src/__tests__/workspace-cleanup-retained-local-dir.test.ts new file mode 100644 index 00000000000..eb417517e60 --- /dev/null +++ b/server/src/__tests__/workspace-cleanup-retained-local-dir.test.ts @@ -0,0 +1,33 @@ +import fs from "node:fs/promises"; +import os from "node:os"; +import path from "node:path"; +import { afterEach, describe, expect, it } from "vitest"; +import { cleanupExecutionWorkspaceArtifacts } from "../services/workspace-runtime.ts"; + +const dirs: string[] = []; +afterEach(async () => { await Promise.all(dirs.splice(0).map(dir => fs.rm(dir, { recursive: true, force: true }))); }); + +async function localWorkspace(createdByRuntime: boolean) { + const root = await fs.mkdtemp(path.join(os.tmpdir(), "paperclip-retained-dir-")); dirs.push(root); + const cwd = path.join(root, "task-directory"); + await fs.mkdir(cwd); await fs.writeFile(path.join(cwd, "website-fixture.txt"), "saved\n"); + return { cwd, workspace: { id: "workspace-1", companyId: "company-1", cwd, providerType: "local_fs", providerRef: null, + branchName: null, repoUrl: null, baseRef: null, projectId: null, projectWorkspaceId: null, sourceIssueId: "issue-1", + metadata: { createdByRuntime } } }; +} + +describe("closing a local workspace directory", () => { + it("reports a retained directory the runtime did not create as cleaned and keeps its files", async () => { + const { cwd, workspace } = await localWorkspace(false); + const cleanup = await cleanupExecutionWorkspaceArtifacts({ workspace }); + expect(cleanup).toMatchObject({ cleaned: true, warnings: [] }); + await expect(fs.readFile(path.join(cwd, "website-fixture.txt"), "utf8")).resolves.toBe("saved\n"); + }); + + it("still removes a runtime-created directory", async () => { + const { cwd, workspace } = await localWorkspace(true); + const cleanup = await cleanupExecutionWorkspaceArtifacts({ workspace }); + expect(cleanup.cleaned).toBe(true); + await expect(fs.stat(cwd)).rejects.toThrow(); + }); +}); diff --git a/server/src/services/workspace-runtime.ts b/server/src/services/workspace-runtime.ts index fc7196c75b7..518f0898dfc 100644 --- a/server/src/services/workspace-runtime.ts +++ b/server/src/services/workspace-runtime.ts @@ -4590,8 +4590,13 @@ export async function cleanupExecutionWorkspaceArtifacts(input: { } } + // A local directory the runtime did not create is never removed here (operator-owned or + // retained, such as an isolated task directory). Closing such a record has nothing left to + // clean, so its surviving directory is the intended outcome, not a failed cleanup. + const retainedByDesign = input.workspace.providerType === "local_fs" && !createdByRuntime; const cleaned = !workspacePath || + retainedByDesign || !(await directoryExists(workspacePath)); if (cleaned) warnings.push(...await releaseNestedRepositoryWorktrees(nestedWorktrees));