From 0e3d8377a337b078840121a48dd003e73c2a7df1 Mon Sep 17 00:00:00 2001 From: Waseem Ilyas <1478353+Waseemilyas@users.noreply.github.com> Date: Thu, 10 Sep 2026 22:43:10 +0000 Subject: [PATCH] fix(server): skip the workspace reopen for comments that do not resume a terminal issue 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 comment on an issue linked to a closed isolated workspace reopens the workspace so resumed work has a live worktree > - The comments route attempted that reopen for every comment, including a pure audit record on a terminal issue that never moves it out of done or cancelled > - Such a comment either churned a workspace the reaper immediately reaped again, or hit a 409 when the workspace could not be rebuilt > - This pull request scopes the reopen to comments that can still use the worktree: anything on a non-terminal issue, and any comment that actually resumes a terminal one ## Linked Issues or Issue Description Refs #11472 — the flat rejection reported there predates the reopen machinery; this closes the residual where a comment that never resumes the issue was still gated on the workspace being rebuildable. ## What Changed - server/src/routes/issues.ts: the comments route attempts the closed workspace reopen only when the issue is non-terminal or the comment will move it out of a terminal status. A plain record on a done or cancelled issue lands without a rebuild and cannot 409 on an un-reopenable workspace. - server/src/__tests__/issue-closed-workspace-routes.test.ts: coverage for the no-resume comment, the same comment against an un-reopenable workspace, and a reopen:true comment still driving the rebuild. ## Verification - pnpm --filter @paperclipai/server vitest run src/__tests__/issue-closed-workspace-routes.test.ts — 15 tests pass. ## Risks - Low risk. The only skipped reopen is for a comment that provably does not move the issue, so no resumed run can arrive at a missing worktree that this route was supposed to rebuild. ## Model Used - Anthropic Claude — SWE-2 Max agent via Devin CLI, tool use and code execution. Co-authored-by: Paperclip --- .../issue-closed-workspace-routes.test.ts | 50 +++++++++++++++++++ server/src/routes/issues.ts | 7 ++- 2 files changed, 56 insertions(+), 1 deletion(-) diff --git a/server/src/__tests__/issue-closed-workspace-routes.test.ts b/server/src/__tests__/issue-closed-workspace-routes.test.ts index b15070a906..c657ebb006 100644 --- a/server/src/__tests__/issue-closed-workspace-routes.test.ts +++ b/server/src/__tests__/issue-closed-workspace-routes.test.ts @@ -308,6 +308,56 @@ describe.sequential("closed isolated workspace issue routes", () => { expect(mockIssueService.addComment).not.toHaveBeenCalled(); }); + it("accepts a plain comment on a done issue without touching the closed workspace", async () => { + // A comment that does not resume a terminal issue is a pure record: it + // must not rebuild the worktree, and it must not be blocked by a + // workspace it never needed. + mockIssueService.getById.mockResolvedValue({ ...makeIssue(), status: "done", assigneeAgentId: null }); + mockIssueService.addComment.mockResolvedValue({ id: "comment-1", body: "decision recorded" }); + + const res = await request(createApp()) + .post(`/api/issues/${issueId}/comments`) + .send({ body: "decision recorded" }); + + expect(res.status).toBe(201); + expect(mockIssueService.addComment).toHaveBeenCalledTimes(1); + expect(mockExecutionWorkspaceService.reopenClosedIsolatedExecutionWorkspaceForIssue).not.toHaveBeenCalled(); + }); + + it("accepts a plain comment on a done issue even when the workspace cannot be reopened", async () => { + // The audit comment is not resuming anything, so a workspace that is + // un-reopenable must not turn it into a 409. + mockIssueService.getById.mockResolvedValue({ ...makeIssue(), status: "done", assigneeAgentId: null }); + mockIssueService.addComment.mockResolvedValue({ id: "comment-1", body: "decision recorded" }); + mockExecutionWorkspaceService.reopenClosedIsolatedExecutionWorkspaceForIssue.mockResolvedValue({ + ok: false, + code: "not_reopenable", + message: "Execution workspace is not reopenable", + }); + + const res = await request(createApp()) + .post(`/api/issues/${issueId}/comments`) + .send({ body: "decision recorded" }); + + expect(res.status).toBe(201); + expect(mockIssueService.addComment).toHaveBeenCalledTimes(1); + }); + + it("still reopens the closed workspace for a comment that resumes a done issue", async () => { + // `reopen: true` on a terminal issue is a resume: it needs the worktree, + // so the reopen still runs and still blocks when the rebuild fails. + mockIssueService.getById.mockResolvedValue({ ...makeIssue(), status: "done", assigneeAgentId: null }); + mockIssueService.update.mockResolvedValue({ ...makeIssue(), status: "todo" }); + mockIssueService.addComment.mockResolvedValue({ id: "comment-1", body: "hello" }); + + const res = await request(createApp()) + .post(`/api/issues/${issueId}/comments`) + .send({ body: "please continue", reopen: true }); + + expect(mockExecutionWorkspaceService.reopenClosedIsolatedExecutionWorkspaceForIssue).toHaveBeenCalledTimes(1); + expect(res.status).toBe(201); + }); + it("returns 503 and blocks the checkout when the rebuild fails", async () => { mockExecutionWorkspaceService.reopenClosedIsolatedExecutionWorkspaceForIssue.mockResolvedValue({ ok: false, diff --git a/server/src/routes/issues.ts b/server/src/routes/issues.ts index 0cc963a4e0..5ba1426b2c 100644 --- a/server/src/routes/issues.ts +++ b/server/src/routes/issues.ts @@ -17065,9 +17065,14 @@ export function issueRoutes( // blocker, and run-cap gate passes. A rejected comment must not rebuild and // republish the workspace as active, because the issue stays terminal and the // reaper then skips the leaked workspace. + // + // A comment on a terminal issue that does not resume the work is a plain + // record: it never needs the worktree. Rebuilding one for it would either + // churn a workspace the reaper immediately reaps again or block an audit + // comment with a 409 the workspace was never needed for. let reopenedWorkspace: Pick | null = null; let reopenedGeneration: number | null = null; - if (closedExecutionWorkspace) { + if (closedExecutionWorkspace && (!isClosed || effectiveMoveToTodoRequested)) { const reopenOutcome = await reopenClosedIssueExecutionWorkspaceOrRespond( req,