diff --git a/server/src/__tests__/execution-workspaces-routes.test.ts b/server/src/__tests__/execution-workspaces-routes.test.ts index be7deacb2e2..af1d4dbfd90 100644 --- a/server/src/__tests__/execution-workspaces-routes.test.ts +++ b/server/src/__tests__/execution-workspaces-routes.test.ts @@ -10,6 +10,8 @@ const mockExecutionWorkspaceService = vi.hoisted(() => ({ listSummaries: vi.fn(), getById: vi.fn(), getCloseReadiness: vi.fn(), + archiveWorkspaceUnderLifecycleLock: vi.fn(), + fenceClosedWorkspaceDestruction: vi.fn(), reconcileExecutionWorkspaceBranch: vi.fn(), update: vi.fn(), })); @@ -389,4 +391,34 @@ describe.sequential("execution workspace routes", () => { }), })); }); + + it("returns 409 and skips destructive cleanup when the archive hits a reopen-pending workspace", async () => { + // A reopen published the workspace active while its source issue is still + // terminal. The archive control must return 409 before any lease teardown, + // runtime-service stop, or artifact cleanup, so it never removes the rebuilt + // worktree. + mockExecutionWorkspaceService.getById.mockResolvedValue({ + id: "workspace-1", + companyId: "company-1", + sourceIssueId: "issue-1", + status: "active", + mode: "isolated_workspace", + }); + mockExecutionWorkspaceService.getCloseReadiness.mockResolvedValue({ + state: "ready", + blockingReasons: [], + }); + mockExecutionWorkspaceService.archiveWorkspaceUnderLifecycleLock.mockResolvedValue({ + outcome: "reopen_pending", + }); + + const res = await request(createApp()) + .patch("/api/execution-workspaces/workspace-1") + .send({ status: "archived" }); + + expect(res.status).toBe(409); + expect(mockExecutionWorkspaceService.archiveWorkspaceUnderLifecycleLock).toHaveBeenCalledTimes(1); + // The destruction fence never runs, so no worktree is removed. + expect(mockExecutionWorkspaceService.fenceClosedWorkspaceDestruction).not.toHaveBeenCalled(); + }); }); diff --git a/server/src/__tests__/execution-workspaces-service.test.ts b/server/src/__tests__/execution-workspaces-service.test.ts index 3ef97f2a983..44199572450 100644 --- a/server/src/__tests__/execution-workspaces-service.test.ts +++ b/server/src/__tests__/execution-workspaces-service.test.ts @@ -1022,6 +1022,56 @@ describeEmbeddedPostgres("executionWorkspaceService.getCloseReadiness", () => { await expect(fs.access(seeded.worktreePath)).resolves.toBeUndefined(); }); + it("refuses to archive a reopen-pending workspace and leaves the row unchanged", async () => { + // Close the second destructive path. The archive route calls + // archiveWorkspaceUnderLifecycleLock. A reopen published this row active with + // the reopen-pending flag while the source issue is still terminal. The + // archive must not close or clear the flag, so the destruction fence never + // removes the rebuilt worktree during the reopen consumption window. + const seeded = await seedTerminalWorkspace({ mergedPr: true }); + await db + .update(executionWorkspaces) + .set({ + status: "active", + closedAt: null, + cleanupReason: null, + cleanupEligibleAt: null, + metadata: { + createdByRuntime: true, + [EXECUTION_WORKSPACE_LIFECYCLE_GENERATION_METADATA_KEY]: 4, + [EXECUTION_WORKSPACE_REOPEN_PENDING_METADATA_KEY]: true, + }, + }) + .where(eq(executionWorkspaces.id, seeded.executionWorkspaceId)); + + const result = await svc.archiveWorkspaceUnderLifecycleLock({ + id: seeded.executionWorkspaceId, + patch: {}, + closedAt: new Date(), + }); + + expect(result).toEqual({ outcome: "reopen_pending" }); + + const [workspace] = await db + .select({ + status: executionWorkspaces.status, + closedAt: executionWorkspaces.closedAt, + metadata: executionWorkspaces.metadata, + }) + .from(executionWorkspaces) + .where(eq(executionWorkspaces.id, seeded.executionWorkspaceId)); + + // The row stays active, keeps the flag, and keeps its generation. + expect(workspace?.status).toBe("active"); + expect(workspace?.closedAt).toBeNull(); + expect(metadataHasReopenPendingConsumption(workspace?.metadata as Record | null)).toBe(true); + expect( + (workspace?.metadata as Record | null)?.[EXECUTION_WORKSPACE_LIFECYCLE_GENERATION_METADATA_KEY], + ).toBe(4); + // The rebuilt worktree is intact. + await expect(fs.access(seeded.worktreePath)).resolves.toBeUndefined(); + }); + it("holds Git index and ref locks across terminal cleanup", async () => { const seeded = await seedTerminalWorkspace({ mergedPr: true }); await db.update(executionWorkspaces).set({ diff --git a/server/src/routes/execution-workspaces.ts b/server/src/routes/execution-workspaces.ts index 097472c884e..22900b8eea2 100644 --- a/server/src/routes/execution-workspaces.ts +++ b/server/src/routes/execution-workspaces.ts @@ -651,6 +651,17 @@ export function executionWorkspaceRoutes(db: Db, opts: { pluginWorkerManager?: P res.status(404).json({ error: "Execution workspace not found" }); return; } + if (archiveResult.outcome === "reopen_pending") { + // A reopen published this workspace as active while its source issue is + // still terminal. A caller will consume the rebuilt worktree. Refuse the + // archive and return before any lease teardown, runtime-service stop, or + // artifact cleanup, so the archive control never removes the rebuilt + // worktree during the reopen consumption window. + res.status(409).json({ + error: "Execution workspace was reopened and cannot be archived right now", + }); + return; + } workspace = archiveResult.workspace; const capturedGeneration = archiveResult.capturedGeneration; diff --git a/server/src/services/execution-workspaces.ts b/server/src/services/execution-workspaces.ts index 60e905210d0..66e8f911783 100644 --- a/server/src/services/execution-workspaces.ts +++ b/server/src/services/execution-workspaces.ts @@ -2730,11 +2730,23 @@ export function executionWorkspaceService(db: Db, opts: ExecutionWorkspaceServic // reopen from publishing an active row between the status re-check and the // archive write. The archive runs only while the row is still open (not // already archived by a race) and clears the reopen-pending flag. + // + // The method refuses to archive a row that carries the reopen-pending flag. + // A reopen sets that flag when it publishes a rebuilt worktree as active + // while the source issue is still terminal. The method returns a distinct + // "reopen_pending" outcome for that row. It does not clear the flag and does + // not archive. The route maps that outcome to HTTP 409 and returns before any + // destructive cleanup, so the archive control never removes a rebuilt + // worktree during the reopen consumption window. archiveWorkspaceUnderLifecycleLock: async (input: { id: string; patch: Partial; closedAt: Date; - }): Promise<{ workspace: ExecutionWorkspace; capturedGeneration: number } | null> => { + }): Promise< + | { outcome: "archived"; workspace: ExecutionWorkspace; capturedGeneration: number } + | { outcome: "reopen_pending" } + | null + > => { return db.transaction(async (tx) => { await acquireExecutionWorkspaceLifecycleLock(tx, input.id); const fresh = await tx @@ -2747,6 +2759,13 @@ export function executionWorkspaceService(db: Db, opts: ExecutionWorkspaceServic // archive again. return null; } + if (metadataHasReopenPendingConsumption(fresh.metadata as Record | null)) { + // A reopen published this row as active while its source issue is still + // terminal. A caller will consume the rebuilt worktree. Refuse the + // archive and keep the flag, so the destructive path never removes the + // rebuilt worktree. The route maps this to HTTP 409. + return { outcome: "reopen_pending" }; + } const baseMetadata = (input.patch.metadata as Record | null | undefined) ?? (fresh.metadata as Record | null); @@ -2763,11 +2782,19 @@ export function executionWorkspaceService(db: Db, opts: ExecutionWorkspaceServic metadata: archiveMetadata, updatedAt: new Date(), }) - .where(eq(executionWorkspaces.id, input.id)) + .where(and( + eq(executionWorkspaces.id, input.id), + // Defense in depth: never archive a reopen-pending row even if the + // flag appears between the read above and this write. The lifecycle + // lock already serializes reopen and archive, so this predicate only + // adds a second, authoritative guard at the write. + sql`(${executionWorkspaces.metadata} ->> ${EXECUTION_WORKSPACE_REOPEN_PENDING_METADATA_KEY}) IS DISTINCT FROM 'true'`, + )) .returning() .then((rows) => rows[0] ?? null); if (!archived) return null; return { + outcome: "archived", workspace: toExecutionWorkspace(archived), capturedGeneration: readExecutionWorkspaceLifecycleGeneration(archiveMetadata), };