From 3e92a4be31892d16901863d3d0d10cf256802a0e Mon Sep 17 00:00:00 2001 From: Priya Raman Date: Thu, 13 Aug 2026 01:23:40 +0000 Subject: [PATCH] test(server): cover reopen, scope, failure-closed, and the cleanup fence Update the closed-workspace route tests: the resume, checkout, and comment guards now reopen the workspace and continue instead of a 409 dead end. A non-reopenable workspace returns 409 and a rebuild failure returns 503. Add embedded-Postgres tests for the reopen service: - a round trip reopens the archived row in place, keeps the issue link, and raises the lifecycle generation; - two issues that share one workspace both keep access after a reopen; - a failed rebuild keeps the row closed and retryable; - a workspace in another company cannot be reopened; - an already-active workspace reports success without a rebuild; - the destruction fence runs the destroy when the generation matches and skips it after a reopen raised the generation. Co-authored-by: Paperclip --- .../execution-workspace-reopen.test.ts | 337 ++++++++++++++++++ .../issue-closed-workspace-routes.test.ts | 64 +++- 2 files changed, 390 insertions(+), 11 deletions(-) create mode 100644 server/src/__tests__/execution-workspace-reopen.test.ts diff --git a/server/src/__tests__/execution-workspace-reopen.test.ts b/server/src/__tests__/execution-workspace-reopen.test.ts new file mode 100644 index 00000000000..cdde33b18b2 --- /dev/null +++ b/server/src/__tests__/execution-workspace-reopen.test.ts @@ -0,0 +1,337 @@ +import { mkdtemp, rm } from "node:fs/promises"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { randomUUID } from "node:crypto"; +import { eq } from "drizzle-orm"; +import { afterAll, afterEach, beforeAll, describe, expect, it, vi } from "vitest"; +import { + activityLog, + companies, + createDb, + executionWorkspaces, + issues, + projectWorkspaces, + projects, +} from "@paperclipai/db"; +import { + getEmbeddedPostgresTestSupport, + startEmbeddedPostgresTestDatabase, +} from "./helpers/embedded-postgres.js"; +import { + EXECUTION_WORKSPACE_LIFECYCLE_GENERATION_METADATA_KEY, + EXECUTION_WORKSPACE_REOPEN_FAILED_REASON, + executionWorkspaceService, + readExecutionWorkspaceLifecycleGeneration, +} from "../services/execution-workspaces.js"; + +const embeddedPostgresSupport = await getEmbeddedPostgresTestSupport(); +const describeEmbeddedPostgres = embeddedPostgresSupport.supported ? describe : describe.skip; + +if (!embeddedPostgresSupport.supported) { + console.warn( + `Skipping embedded Postgres reopen tests on this host: ${embeddedPostgresSupport.reason ?? "unsupported environment"}`, + ); +} + +describeEmbeddedPostgres("reopen archived isolated execution workspace", () => { + let db!: ReturnType; + let tempDb: Awaited> | null = null; + const tempDirs: string[] = []; + + beforeAll(async () => { + tempDb = await startEmbeddedPostgresTestDatabase("paperclip-reopen-"); + db = createDb(tempDb.connectionString); + }, 20_000); + + afterEach(async () => { + await db.delete(activityLog); + await db.delete(issues); + await db.delete(executionWorkspaces); + await db.delete(projectWorkspaces); + await db.delete(projects); + await db.delete(companies); + await Promise.all(tempDirs.splice(0).map((dir) => rm(dir, { recursive: true, force: true }))); + }); + + afterAll(async () => { + await tempDb?.cleanup(); + }); + + async function makeExistingDir(): Promise { + const dir = await mkdtemp(join(tmpdir(), "paperclip-reopen-cwd-")); + tempDirs.push(dir); + return dir; + } + + async function seedCompanyProject() { + const companyId = randomUUID(); + const projectId = randomUUID(); + const projectWorkspaceId = randomUUID(); + await db.insert(companies).values({ + id: companyId, + name: "Paperclip", + issuePrefix: `PAP-${companyId.slice(0, 8)}`, + requireBoardApprovalForNewAgents: false, + }); + await db.insert(projects).values({ + id: projectId, + companyId, + name: "Reopen project", + status: "in_progress", + }); + await db.insert(projectWorkspaces).values({ + id: projectWorkspaceId, + companyId, + projectId, + name: "Primary", + sourceType: "local_path", + cwd: "/tmp/paperclip-reopen-project", + isPrimary: true, + }); + return { companyId, projectId, projectWorkspaceId }; + } + + async function seedClosedWorkspace(input: { + companyId: string; + projectId: string; + projectWorkspaceId: string; + cwd: string; + status?: "archived" | "cleanup_failed" | "active"; + generation?: number; + }) { + const workspaceId = randomUUID(); + const closed = (input.status ?? "archived") !== "active"; + await db.insert(executionWorkspaces).values({ + id: workspaceId, + companyId: input.companyId, + projectId: input.projectId, + projectWorkspaceId: input.projectWorkspaceId, + mode: "isolated_workspace", + // project_primary strategy so the rebuild only checks that the directory + // exists, with no git operation. + strategyType: "project_primary", + name: "reopen-workspace", + status: input.status ?? "archived", + providerType: "local_fs", + cwd: input.cwd, + closedAt: closed ? new Date() : null, + cleanupReason: closed ? "issue_terminal" : null, + cleanupEligibleAt: closed ? new Date() : null, + metadata: { + [EXECUTION_WORKSPACE_LIFECYCLE_GENERATION_METADATA_KEY]: input.generation ?? 1, + }, + }); + return workspaceId; + } + + async function seedIssue(input: { + companyId: string; + projectId: string; + workspaceId: string; + issueNumber: number; + }) { + const issueId = randomUUID(); + await db.insert(issues).values({ + id: issueId, + companyId: input.companyId, + projectId: input.projectId, + identifier: `PAP-${input.issueNumber}`, + issueNumber: input.issueNumber, + title: "Resume me", + status: "todo", + priority: "medium", + executionWorkspaceId: input.workspaceId, + }); + return issueId; + } + + async function readWorkspace(id: string) { + return db + .select() + .from(executionWorkspaces) + .where(eq(executionWorkspaces.id, id)) + .then((rows) => rows[0] ?? null); + } + + it("reopens the archived row in place, keeps the issue link, and raises the generation", async () => { + const { companyId, projectId, projectWorkspaceId } = await seedCompanyProject(); + const cwd = await makeExistingDir(); + const workspaceId = await seedClosedWorkspace({ companyId, projectId, projectWorkspaceId, cwd, generation: 3 }); + const issueId = await seedIssue({ companyId, projectId, workspaceId, issueNumber: 4100 }); + + const svc = executionWorkspaceService(db); + const result = await svc.reopenClosedIsolatedExecutionWorkspaceForIssue({ + workspaceId, + issue: { id: issueId, companyId, projectId }, + actor: { agentId: null, actorType: "user" }, + }); + + expect(result.ok).toBe(true); + if (!result.ok) return; + expect(result.reopened).toBe(true); + + const row = await readWorkspace(workspaceId); + expect(row?.status).toBe("active"); + expect(row?.closedAt).toBeNull(); + expect(row?.cleanupReason).toBeNull(); + expect(row?.cleanupEligibleAt).toBeNull(); + expect(readExecutionWorkspaceLifecycleGeneration(row?.metadata as Record | null)).toBe(4); + + // The reopen never changes the issue-to-workspace link. + const issueRow = await db.select().from(issues).where(eq(issues.id, issueId)).then((rows) => rows[0] ?? null); + expect(issueRow?.executionWorkspaceId).toBe(workspaceId); + }); + + it("keeps access for every issue that shares the reopened row", async () => { + const { companyId, projectId, projectWorkspaceId } = await seedCompanyProject(); + const cwd = await makeExistingDir(); + const workspaceId = await seedClosedWorkspace({ companyId, projectId, projectWorkspaceId, cwd }); + const firstIssueId = await seedIssue({ companyId, projectId, workspaceId, issueNumber: 4101 }); + const secondIssueId = await seedIssue({ companyId, projectId, workspaceId, issueNumber: 4102 }); + + const svc = executionWorkspaceService(db); + const result = await svc.reopenClosedIsolatedExecutionWorkspaceForIssue({ + workspaceId, + issue: { id: firstIssueId, companyId, projectId }, + actor: { agentId: null, actorType: "user" }, + }); + expect(result.ok).toBe(true); + + const rows = await db.select().from(issues); + for (const row of rows) { + expect(row.executionWorkspaceId).toBe(workspaceId); + } + const workspace = await readWorkspace(workspaceId); + expect(workspace?.status).toBe("active"); + expect(firstIssueId).not.toBe(secondIssueId); + }); + + it("fails closed and keeps the row closed when the rebuild fails", async () => { + const { companyId, projectId, projectWorkspaceId } = await seedCompanyProject(); + // A directory that does not exist. The project_primary rebuild returns null. + const missingDir = join(tmpdir(), `paperclip-reopen-missing-${randomUUID()}`); + const workspaceId = await seedClosedWorkspace({ + companyId, + projectId, + projectWorkspaceId, + cwd: missingDir, + generation: 2, + }); + const issueId = await seedIssue({ companyId, projectId, workspaceId, issueNumber: 4103 }); + + const svc = executionWorkspaceService(db); + const result = await svc.reopenClosedIsolatedExecutionWorkspaceForIssue({ + workspaceId, + issue: { id: issueId, companyId, projectId }, + actor: { agentId: null, actorType: "user" }, + }); + + expect(result.ok).toBe(false); + if (result.ok) return; + expect(result.code).toBe("rebuild_failed"); + + const row = await readWorkspace(workspaceId); + // The row stays closed and retryable. The generation still rises so a queued + // cleanup with the old generation does nothing. + expect(row?.status).toBe("archived"); + expect(row?.cleanupReason).toBe(EXECUTION_WORKSPACE_REOPEN_FAILED_REASON); + expect(row?.cleanupEligibleAt).toBeNull(); + expect(readExecutionWorkspaceLifecycleGeneration(row?.metadata as Record | null)).toBe(3); + }); + + it("refuses to reopen a workspace in another company", async () => { + const first = await seedCompanyProject(); + const second = await seedCompanyProject(); + const cwd = await makeExistingDir(); + const workspaceId = await seedClosedWorkspace({ + companyId: first.companyId, + projectId: first.projectId, + projectWorkspaceId: first.projectWorkspaceId, + cwd, + }); + + const svc = executionWorkspaceService(db); + const result = await svc.reopenClosedIsolatedExecutionWorkspaceForIssue({ + workspaceId, + // The issue belongs to a different company than the workspace. + issue: { id: randomUUID(), companyId: second.companyId, projectId: second.projectId }, + actor: { agentId: null, actorType: "user" }, + }); + + expect(result.ok).toBe(false); + if (result.ok) return; + expect(result.code).toBe("not_reopenable"); + + const row = await readWorkspace(workspaceId); + expect(row?.status).toBe("archived"); + }); + + it("reports success without a rebuild when the workspace is already active", async () => { + const { companyId, projectId, projectWorkspaceId } = await seedCompanyProject(); + const cwd = await makeExistingDir(); + const workspaceId = await seedClosedWorkspace({ + companyId, + projectId, + projectWorkspaceId, + cwd, + status: "active", + generation: 5, + }); + const issueId = await seedIssue({ companyId, projectId, workspaceId, issueNumber: 4104 }); + + const svc = executionWorkspaceService(db); + const result = await svc.reopenClosedIsolatedExecutionWorkspaceForIssue({ + workspaceId, + issue: { id: issueId, companyId, projectId }, + actor: { agentId: null, actorType: "user" }, + }); + + expect(result.ok).toBe(true); + if (!result.ok) return; + expect(result.reopened).toBe(false); + + const row = await readWorkspace(workspaceId); + // No reopen ran, so the generation is unchanged. + expect(readExecutionWorkspaceLifecycleGeneration(row?.metadata as Record | null)).toBe(5); + }); + + it("runs the destroy under the fence when the generation matches, and skips it after a reopen", async () => { + const { companyId, projectId, projectWorkspaceId } = await seedCompanyProject(); + const cwd = await makeExistingDir(); + const workspaceId = await seedClosedWorkspace({ companyId, projectId, projectWorkspaceId, cwd, generation: 7 }); + + const svc = executionWorkspaceService(db); + + // The captured generation matches, so the destroy callback runs. + const destroyMatches = vi.fn(async () => "destroyed"); + const matched = await svc.fenceClosedWorkspaceDestruction({ + workspaceId, + capturedGeneration: 7, + destroy: destroyMatches, + }); + expect(matched.skippedReopened).toBe(false); + expect(destroyMatches).toHaveBeenCalledTimes(1); + + // Simulate a reopen: raise the generation and mark the row active. + await db + .update(executionWorkspaces) + .set({ + status: "active", + closedAt: null, + cleanupReason: null, + cleanupEligibleAt: null, + metadata: { [EXECUTION_WORKSPACE_LIFECYCLE_GENERATION_METADATA_KEY]: 8 }, + }) + .where(eq(executionWorkspaces.id, workspaceId)); + + // A stale cleanup captured generation 7. The fence must skip the destroy. + const destroyStale = vi.fn(async () => "destroyed"); + const skipped = await svc.fenceClosedWorkspaceDestruction({ + workspaceId, + capturedGeneration: 7, + destroy: destroyStale, + }); + expect(skipped.skippedReopened).toBe(true); + expect(destroyStale).not.toHaveBeenCalled(); + }); +}); diff --git a/server/src/__tests__/issue-closed-workspace-routes.test.ts b/server/src/__tests__/issue-closed-workspace-routes.test.ts index d9c82cd7d05..a9ae6fcf25f 100644 --- a/server/src/__tests__/issue-closed-workspace-routes.test.ts +++ b/server/src/__tests__/issue-closed-workspace-routes.test.ts @@ -16,6 +16,7 @@ const mockIssueService = vi.hoisted(() => ({ const mockExecutionWorkspaceService = vi.hoisted(() => ({ getById: vi.fn(), + reopenClosedIsolatedExecutionWorkspaceForIssue: vi.fn(), })); const mockAccessService = vi.hoisted(() => ({ @@ -208,30 +209,39 @@ describe.sequential("closed isolated workspace issue routes", () => { vi.clearAllMocks(); mockIssueService.getById.mockResolvedValue(makeIssue()); mockExecutionWorkspaceService.getById.mockResolvedValue(makeClosedWorkspace()); + // The guard reopens a closed isolated workspace and lets the request + // continue. The default is a successful reopen. + mockExecutionWorkspaceService.reopenClosedIsolatedExecutionWorkspaceForIssue.mockResolvedValue({ + ok: true, + reopened: true, + workspace: { ...makeClosedWorkspace(), status: "active", closedAt: null }, + }); }); - it("rejects new issue comments when the linked isolated workspace is closed", async () => { + it("reopens the closed isolated workspace and accepts a new comment", async () => { const res = await request(await createApp()) .post(`/api/issues/${issueId}/comments`) .send({ body: "hello" }); - expect(res.status).toBe(409); - expect(res.body.error).toContain("closed workspace"); - expect(mockIssueService.addComment).not.toHaveBeenCalled(); + expect(mockExecutionWorkspaceService.reopenClosedIsolatedExecutionWorkspaceForIssue).toHaveBeenCalledWith({ + workspaceId: closedWorkspaceId, + issue: { id: issueId, companyId: "company-1", projectId: null }, + actor: expect.objectContaining({ actorType: "user" }), + }); + // The closed-workspace dead end is gone. + expect(res.status).not.toBe(409); }); - it("rejects comment updates when the linked isolated workspace is closed", async () => { + it("reopens the closed isolated workspace and accepts a comment update", async () => { const res = await request(await createApp()) .patch(`/api/issues/${issueId}`) .send({ comment: "hello" }); - expect(res.status).toBe(409); - expect(res.body.error).toContain("closed workspace"); - expect(mockIssueService.update).not.toHaveBeenCalled(); - expect(mockIssueService.addComment).not.toHaveBeenCalled(); + expect(mockExecutionWorkspaceService.reopenClosedIsolatedExecutionWorkspaceForIssue).toHaveBeenCalledTimes(1); + expect(res.status).not.toBe(409); }); - it("rejects checkout when the linked isolated workspace is closed", async () => { + it("reopens the closed isolated workspace and accepts a checkout", async () => { const res = await request(await createApp()) .post(`/api/issues/${issueId}/checkout`) .send({ @@ -239,8 +249,40 @@ describe.sequential("closed isolated workspace issue routes", () => { expectedStatuses: ["todo", "backlog", "blocked"], }); + expect(mockExecutionWorkspaceService.reopenClosedIsolatedExecutionWorkspaceForIssue).toHaveBeenCalledTimes(1); + expect(res.status).not.toBe(409); + }); + + it("returns 409 and blocks the comment when the workspace cannot be reopened", async () => { + mockExecutionWorkspaceService.reopenClosedIsolatedExecutionWorkspaceForIssue.mockResolvedValue({ + ok: false, + code: "not_reopenable", + message: "Execution workspace is not reopenable", + }); + + const res = await request(await createApp()) + .post(`/api/issues/${issueId}/comments`) + .send({ body: "hello" }); + expect(res.status).toBe(409); - expect(res.body.error).toContain("closed workspace"); + expect(mockIssueService.addComment).not.toHaveBeenCalled(); + }); + + it("returns 503 and blocks the checkout when the rebuild fails", async () => { + mockExecutionWorkspaceService.reopenClosedIsolatedExecutionWorkspaceForIssue.mockResolvedValue({ + ok: false, + code: "rebuild_failed", + message: "Failed to rebuild the execution workspace", + }); + + const res = await request(await createApp()) + .post(`/api/issues/${issueId}/checkout`) + .send({ + agentId, + expectedStatuses: ["todo", "backlog", "blocked"], + }); + + expect(res.status).toBe(503); expect(mockIssueService.checkout).not.toHaveBeenCalled(); });