fix(server): skip the workspace reopen for comments that do not resume a terminal issue
## 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 <noreply@paperclip.ing>
This commit is contained in:
parent
4042eb1c48
commit
0e3d8377a3
|
|
@ -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,
|
||||
|
|
|
|||
|
|
@ -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<ExecutionWorkspace, "id"> | null = null;
|
||||
let reopenedGeneration: number | null = null;
|
||||
if (closedExecutionWorkspace) {
|
||||
if (closedExecutionWorkspace && (!isClosed || effectiveMoveToTodoRequested)) {
|
||||
const reopenOutcome =
|
||||
await reopenClosedIssueExecutionWorkspaceOrRespond(
|
||||
req,
|
||||
|
|
|
|||
Loading…
Reference in New Issue