From 81951535e9d07e54884fe9cb3546d0dcd1500080 Mon Sep 17 00:00:00 2001 From: Priya Raman Date: Thu, 13 Aug 2026 01:18:02 +0000 Subject: [PATCH] feat(server): reopen an archived isolated execution workspace in place Make an archived isolated execution workspace usable again, so a terminal issue can resume without a dead end. Reopen the same workspace row: flip it back to active and rebuild the worktree. Do not create a new row and do not change any issue-to-workspace link, so every issue that shares the row keeps access. Add reopenClosedIsolatedExecutionWorkspaceForIssue in the execution workspace service. It binds the workspace to the issue company and project, accepts only an isolated workspace in a closed state, and fails closed with no workspace detail on a mismatch. It rebuilds the worktree first and publishes active only on success. A rebuild failure keeps the row closed and returns an error, so the caller never dispatches a run against a broken workspace. Fence a cleanup against a concurrent reopen. Every archive and every reopen raises a lifecycle generation stored in the workspace metadata. Reopen and the destructive cleanup paths take the same transaction-scoped Postgres advisory lock. A cleanup captures the generation it archived at and re-checks it under the lock before it deletes the worktree. A reopen that ran in between raises the generation, so the stale cleanup does nothing. The terminal-workspace reaper and the archive route both use this fence. Wire the reopen at the resume, checkout, and comment guards. Each guard runs after the route authorizes the request on the issue. A closed isolated workspace now reopens and the request continues, instead of a 409 dead end. A non-reopenable workspace returns 409 and a rebuild failure returns 503. Enable the comment composer for a reopened issue and show a hint that the next comment or resume rebuilds the worktree. Co-authored-by: Paperclip --- server/src/routes/execution-workspaces.ts | 77 ++-- server/src/routes/issues.ts | 44 ++- server/src/services/execution-workspaces.ts | 410 +++++++++++++++++++- ui/src/pages/IssueDetail.tsx | 24 +- 4 files changed, 504 insertions(+), 51 deletions(-) diff --git a/server/src/routes/execution-workspaces.ts b/server/src/routes/execution-workspaces.ts index 2d4b592dba3..04d7abee55d 100644 --- a/server/src/routes/execution-workspaces.ts +++ b/server/src/routes/execution-workspaces.ts @@ -13,7 +13,12 @@ import { import type { WorkspaceRuntimeDesiredState, WorkspaceRuntimeServiceStateMap } from "@paperclipai/shared"; import { validate } from "../middleware/validate.js"; import { accessService, executionWorkspaceService, heartbeatService, logActivity, workspaceOperationService } from "../services/index.js"; -import { mergeExecutionWorkspaceConfig, readExecutionWorkspaceConfig } from "../services/execution-workspaces.js"; +import { + bumpExecutionWorkspaceLifecycleGeneration, + mergeExecutionWorkspaceConfig, + readExecutionWorkspaceConfig, + readExecutionWorkspaceLifecycleGeneration, +} from "../services/execution-workspaces.js"; import { parseProjectExecutionWorkspacePolicy } from "../services/execution-workspace-policy.js"; import { readProjectWorkspaceRuntimeConfig } from "../services/project-workspace-runtime-config.js"; import { @@ -634,17 +639,25 @@ export function executionWorkspaceRoutes(db: Db, opts: { pluginWorkerManager?: P } const closedAt = new Date(); + // Raise the lifecycle generation on archive so the destruction below (and + // any queued reaper cleanup) can detect a reopen that runs in between. + const archiveMetadata = bumpExecutionWorkspaceLifecycleGeneration( + ((patch.metadata as Record | null | undefined) + ?? (existing.metadata as Record | null)) ?? null, + ); const archivedWorkspace = await svc.update(id, { ...patch, status: "archived", closedAt, cleanupReason: null, + metadata: archiveMetadata, }); if (!archivedWorkspace) { res.status(404).json({ error: "Execution workspace not found" }); return; } workspace = archivedWorkspace; + const capturedGeneration = readExecutionWorkspaceLifecycleGeneration(archiveMetadata); await environmentRuntime.destroyReusableSandboxLeases({ companyId: existing.companyId, @@ -668,11 +681,6 @@ export function executionWorkspaceRoutes(db: Db, opts: { pluginWorkerManager?: P } try { - await stopRuntimeServicesForExecutionWorkspace({ - db, - executionWorkspaceId: existing.id, - workspaceCwd: existing.cwd, - }); const projectWorkspace = existing.projectWorkspaceId ? await db .select({ @@ -697,26 +705,45 @@ export function executionWorkspaceRoutes(db: Db, opts: { pluginWorkerManager?: P .where(and(eq(projects.id, existing.projectId), eq(projects.companyId, existing.companyId))) .then((rows) => parseProjectExecutionWorkspacePolicy(rows[0]?.executionWorkspacePolicy)) : null; - const cleanupResult = await cleanupExecutionWorkspaceArtifacts({ - workspace: existing, - projectWorkspace, - teardownCommand: configForCleanup?.teardownCommand ?? projectPolicy?.workspaceStrategy?.teardownCommand ?? null, - cleanupCommand: configForCleanup?.cleanupCommand ?? null, - recorder: workspaceOperationsSvc.createRecorder({ - companyId: existing.companyId, - executionWorkspaceId: existing.id, - }), + // Destroy under the lifecycle lock. If a resume reopened the workspace in + // the meantime, the fence skips destruction and keeps the reopened row. + const fenced = await svc.fenceClosedWorkspaceDestruction({ + workspaceId: id, + capturedGeneration, + destroy: async () => { + await stopRuntimeServicesForExecutionWorkspace({ + db, + executionWorkspaceId: existing.id, + workspaceCwd: existing.cwd, + }); + return cleanupExecutionWorkspaceArtifacts({ + workspace: existing, + projectWorkspace, + teardownCommand: configForCleanup?.teardownCommand ?? projectPolicy?.workspaceStrategy?.teardownCommand ?? null, + cleanupCommand: configForCleanup?.cleanupCommand ?? null, + recorder: workspaceOperationsSvc.createRecorder({ + companyId: existing.companyId, + executionWorkspaceId: existing.id, + }), + }); + }, }); - cleanupWarnings = cleanupResult.warnings; - const cleanupPatch: Record = { - closedAt, - cleanupReason: cleanupWarnings.length > 0 ? cleanupWarnings.join(" | ") : null, - }; - if (!cleanupResult.cleaned) { - cleanupPatch.status = "cleanup_failed"; - } - if (cleanupResult.warnings.length > 0 || !cleanupResult.cleaned) { - workspace = (await svc.update(id, cleanupPatch)) ?? workspace; + if (fenced.skippedReopened) { + // A resume reopened the workspace. Return the current (active) row. + workspace = (await svc.getById(id)) ?? workspace; + } else { + const cleanupResult = fenced.result; + cleanupWarnings = cleanupResult.warnings; + const cleanupPatch: Record = { + closedAt, + cleanupReason: cleanupWarnings.length > 0 ? cleanupWarnings.join(" | ") : null, + }; + if (!cleanupResult.cleaned) { + cleanupPatch.status = "cleanup_failed"; + } + if (cleanupResult.warnings.length > 0 || !cleanupResult.cleaned) { + workspace = (await svc.update(id, cleanupPatch)) ?? workspace; + } } } catch (error) { const failureReason = error instanceof Error ? error.message : String(error); diff --git a/server/src/routes/issues.ts b/server/src/routes/issues.ts index dbdfe47064c..5bac26a24e7 100644 --- a/server/src/routes/issues.ts +++ b/server/src/routes/issues.ts @@ -66,7 +66,6 @@ import { updateDocumentAnnotationThreadSchema, upsertIssueDocumentSchema, updateIssueSchema, - getClosedIsolatedExecutionWorkspaceMessage, isClosedIsolatedExecutionWorkspace, isUuidLike, normalizeIssueIdentifier as normalizeIssueReferenceIdentifier, @@ -5018,14 +5017,30 @@ export function issueRoutes( return workspace; } - function respondClosedIssueExecutionWorkspace( + // Reopen the closed isolated workspace that a guard found, so the request can + // continue. Return true when the workspace is open again. Return false after + // this function sends an error response, so the caller must stop. The reopen is + // scoped to the issue company and project inside the service, and it runs only + // after the route already authorized the request on the issue. + async function reopenClosedIssueExecutionWorkspaceOrRespond( + req: Request, res: Response, - workspace: Pick, - ) { - res.status(409).json({ - error: getClosedIsolatedExecutionWorkspaceMessage(workspace), - executionWorkspace: workspace, + issue: { id: string; companyId: string; projectId?: string | null }, + workspace: Pick, + ): Promise { + const actor = getActorInfo(req); + const result = await executionWorkspacesSvc.reopenClosedIsolatedExecutionWorkspaceForIssue({ + workspaceId: workspace.id, + issue: { id: issue.id, companyId: issue.companyId, projectId: issue.projectId ?? null }, + actor: { agentId: actor.agentId, actorType: actor.actorType }, }); + if (result.ok) return true; + if (result.code === "not_reopenable") { + res.status(409).json({ error: "This issue is linked to a closed workspace that cannot be reopened." }); + } else { + res.status(503).json({ error: "Could not reopen the workspace for this issue. Please try again." }); + } + return false; } async function destroyReusableSandboxLeasesForTerminalIssue(issue: { @@ -8738,8 +8753,9 @@ export function issueRoutes( (Object.keys(updateFields).length > 0 || reviewRequest !== undefined || hiddenAtRaw !== undefined); if (closedExecutionWorkspace && (commentBody || isAgentWorkUpdate)) { - respondClosedIssueExecutionWorkspace(res, closedExecutionWorkspace); - return; + if (!(await reopenClosedIssueExecutionWorkspaceOrRespond(req, res, existing, closedExecutionWorkspace))) { + return; + } } if ( isAgentWorkUpdate && @@ -10120,8 +10136,9 @@ export function issueRoutes( const closedExecutionWorkspace = await getClosedIssueExecutionWorkspace(issue); if (closedExecutionWorkspace) { - respondClosedIssueExecutionWorkspace(res, closedExecutionWorkspace); - return; + if (!(await reopenClosedIssueExecutionWorkspaceOrRespond(req, res, issue, closedExecutionWorkspace))) { + return; + } } const checkoutRunId = requireAgentRunId(req, res); @@ -11110,8 +11127,9 @@ export function issueRoutes( })) return; const closedExecutionWorkspace = await getClosedIssueExecutionWorkspace(issue); if (closedExecutionWorkspace) { - respondClosedIssueExecutionWorkspace(res, closedExecutionWorkspace); - return; + if (!(await reopenClosedIssueExecutionWorkspaceOrRespond(req, res, issue, closedExecutionWorkspace))) { + return; + } } const actor = getActorInfo(req); diff --git a/server/src/services/execution-workspaces.ts b/server/src/services/execution-workspaces.ts index 37b5d438b44..c3781256a82 100644 --- a/server/src/services/execution-workspaces.ts +++ b/server/src/services/execution-workspaces.ts @@ -35,6 +35,7 @@ import type { } from "@paperclipai/shared"; import { deriveProjectUrlKey, WORKSPACE_OVERVIEW_LINKED_ISSUE_LIMIT } from "@paperclipai/shared"; import { conflict, notFound, unprocessable } from "../errors.js"; +import { logger } from "../middleware/logger.js"; import { applyIssueExecutionPolicyTransition, normalizeIssueExecutionPolicy, @@ -61,12 +62,73 @@ import { type ExecutionWorkspaceRow = typeof executionWorkspaces.$inferSelect; type WorkspaceRuntimeServiceRow = typeof workspaceRuntimeServices.$inferSelect; type RuntimeServiceReadDb = Pick; +type DbTransaction = Parameters[0]>[0]; const execFileAsync = promisify(execFile); const TERMINAL_ISSUE_STATUSES = new Set(["done", "cancelled"]); const WORKSPACE_BRANCH_INCOHERENCE_REASON = "git_worktree_branch_incoherence"; const WORKSPACE_VALIDATION_RECOVERY_CAUSE = "workspace_validation_failed"; export const ISSUE_TERMINAL_WORKSPACE_CLEANUP_REASON = "issue_terminal"; +// The closed statuses a workspace holds after an archive or a failed cleanup. +// A reopen restores one of these rows back to "active". +const CLOSED_EXECUTION_WORKSPACE_STATUSES = ["archived", "cleanup_failed"] as const; + +// The reopen-failure reason kept on the row when a rebuild does not finish. The +// value is sanitized: it never contains a repository URL, a host path, or git +// output. +export const EXECUTION_WORKSPACE_REOPEN_FAILED_REASON = "reopen_failed"; + +// The metadata key that holds the workspace lifecycle generation. The generation +// is a monotonic integer. Every archive and every reopen increases it by one. A +// destructive cleanup captures the generation it archived at, then re-reads the +// generation under the lifecycle lock immediately before it deletes the worktree. +// A reopen that ran in between raises the generation, so the stale cleanup finds +// a mismatch and does nothing. This fences a queued or in-flight cleanup against +// a workspace that a reopen already restored. +export const EXECUTION_WORKSPACE_LIFECYCLE_GENERATION_METADATA_KEY = "lifecycleGeneration"; + +export function readExecutionWorkspaceLifecycleGeneration( + metadata: Record | null | undefined, +): number { + const raw = metadata?.[EXECUTION_WORKSPACE_LIFECYCLE_GENERATION_METADATA_KEY]; + return typeof raw === "number" && Number.isInteger(raw) && raw >= 0 ? raw : 0; +} + +// Return a metadata object with the lifecycle generation increased by one. The +// caller keeps every other metadata key. +export function bumpExecutionWorkspaceLifecycleGeneration( + metadata: Record | null | undefined, +): Record { + return { + ...(metadata ?? {}), + [EXECUTION_WORKSPACE_LIFECYCLE_GENERATION_METADATA_KEY]: + readExecutionWorkspaceLifecycleGeneration(metadata) + 1, + }; +} + +function isClosedExecutionWorkspaceStatus(status: string | null | undefined): boolean { + return status === "archived" || status === "cleanup_failed"; +} + +// Acquire the per-workspace, transaction-scoped Postgres advisory lock. Postgres +// releases the lock when the transaction that holds `tx` commits or rolls back. +// Both the reopen path and the destructive cleanup path acquire the same lock, +// so they never run against the same workspace at the same time, even on +// different server processes. +async function acquireExecutionWorkspaceLifecycleLock( + tx: DbTransaction, + workspaceId: string, +): Promise { + await tx.execute( + sql`select pg_advisory_xact_lock(hashtextextended(${`execution_workspace_lifecycle:${workspaceId}`}, 0))`, + ); +} + +export type ReopenClosedIsolatedExecutionWorkspaceResult = + | { ok: true; workspace: ExecutionWorkspace; reopened: true } + | { ok: true; workspace: ExecutionWorkspace; reopened: false } + | { ok: false; code: "not_reopenable" | "rebuild_failed"; message: string }; + export type ExecutionWorkspaceServiceOptions = { resolvePullRequestDetails?: PullRequestMergeDetailsResolver; now?: () => Date; @@ -1279,7 +1341,52 @@ export function executionWorkspaceService(db: Db, opts: ExecutionWorkspaceServic return active.length > 0; } - async function cleanupTerminalWorkspace(workspace: ExecutionWorkspaceRow, expectedHeadSha: string | null) { + async function cleanupTerminalWorkspace( + workspace: ExecutionWorkspaceRow, + expectedHeadSha: string | null, + capturedGeneration: number, + ): Promise<{ cleaned: boolean; warnings: string[]; skippedReopened?: boolean }> { + return db.transaction(async (tx) => { + // Hold the per-workspace lifecycle lock across the destructive actions. A + // reopen takes the same lock, so a reopen cannot rebuild the worktree while + // this cleanup runs, and this cleanup cannot delete a worktree that a + // reopen already restored. The advisory lock (not a row FOR UPDATE) gives + // the exclusion, so the cleanup body can still update the same row on the + // pooled connection without a self-block. + await acquireExecutionWorkspaceLifecycleLock(tx, workspace.id); + const fresh = await tx + .select({ status: executionWorkspaces.status, metadata: executionWorkspaces.metadata }) + .from(executionWorkspaces) + .where(eq(executionWorkspaces.id, workspace.id)) + .then((rows) => rows[0] ?? null); + const currentGeneration = fresh + ? readExecutionWorkspaceLifecycleGeneration(fresh.metadata as Record | null) + : null; + if ( + !fresh + || !isClosedExecutionWorkspaceStatus(fresh.status) + || currentGeneration !== capturedGeneration + ) { + // A reopen restored this workspace after it was archived. Do not destroy + // the rebuilt worktree. + logger.info( + { + event: "execution_workspace.cleanup_skipped", + reason: "reopened", + executionWorkspaceId: workspace.id, + capturedGeneration, + currentGeneration, + currentStatus: fresh?.status ?? null, + }, + "execution workspace cleanup skipped because it was reopened", + ); + return { cleaned: false, warnings: [], skippedReopened: true }; + } + return runTerminalWorkspaceCleanup(workspace, expectedHeadSha); + }); + } + + async function runTerminalWorkspaceCleanup(workspace: ExecutionWorkspaceRow, expectedHeadSha: string | null) { const [ { acquireGitWorktreeCleanupLock, @@ -2069,6 +2176,7 @@ export function executionWorkspaceService(db: Db, opts: ExecutionWorkspaceServic skippedNonTerminalTree: 0, skippedUndelivered: 0, skippedRace: 0, + skippedReopened: 0, }; } terminalSweepInProgress = true; @@ -2130,6 +2238,7 @@ export function executionWorkspaceService(db: Db, opts: ExecutionWorkspaceServic skippedNonTerminalTree: 0, skippedUndelivered: 0, skippedRace: 0, + skippedReopened: 0, }; for (const workspace of candidates) { @@ -2157,6 +2266,12 @@ export function executionWorkspaceService(db: Db, opts: ExecutionWorkspaceServic } result.eligible += 1; const closedAt = now(); + // Raise the lifecycle generation on archive. The cleanup below captures + // this generation and re-checks it before it deletes the worktree, so a + // reopen that runs in between fences the cleanup off. + const archivedMetadata = bumpExecutionWorkspaceLifecycleGeneration( + workspace.metadata as Record | null, + ); const archived = await db .update(executionWorkspaces) .set({ @@ -2164,6 +2279,7 @@ export function executionWorkspaceService(db: Db, opts: ExecutionWorkspaceServic closedAt, cleanupEligibleAt: workspace.cleanupEligibleAt ?? closedAt, cleanupReason: ISSUE_TERMINAL_WORKSPACE_CLEANUP_REASON, + metadata: archivedMetadata, updatedAt: closedAt, }) .where(and( @@ -2229,13 +2345,19 @@ export function executionWorkspaceService(db: Db, opts: ExecutionWorkspaceServic }, }); + const capturedGeneration = readExecutionWorkspaceLifecycleGeneration( + archived.metadata as Record | null, + ); try { - const cleanup = await cleanupTerminalWorkspace(archived, assessment.workspaceHeadSha); - if (!cleanup.cleaned) result.cleanupFailed += 1; + const cleanup = await cleanupTerminalWorkspace(archived, assessment.workspaceHeadSha, capturedGeneration); + if (cleanup.skippedReopened) result.skippedReopened += 1; + else if (!cleanup.cleaned) result.cleanupFailed += 1; else result.archived += 1; } catch (error) { result.cleanupFailed += 1; const failure = error instanceof Error ? error.message : String(error); + // Mark cleanup_failed only while the row is still closed. A reopen that + // raced after the failure leaves the row active; do not clobber it. await db .update(executionWorkspaces) .set({ @@ -2243,7 +2365,10 @@ export function executionWorkspaceService(db: Db, opts: ExecutionWorkspaceServic cleanupReason: `${ISSUE_TERMINAL_WORKSPACE_CLEANUP_REASON} | ${failure}`, updatedAt: now(), }) - .where(eq(executionWorkspaces.id, archived.id)); + .where(and( + eq(executionWorkspaces.id, archived.id), + inArray(executionWorkspaces.status, [...CLOSED_EXECUTION_WORKSPACE_STATUSES]), + )); await logActivity(db, { companyId: archived.companyId, actorType: "system", @@ -2280,6 +2405,283 @@ export function executionWorkspaceService(db: Db, opts: ExecutionWorkspaceServic return row ? toExecutionWorkspace(row) : null; }, + // Reopen one closed isolated execution workspace so an authorized issue can + // use it again. The caller must first authorize the request on the issue. + // The whole operation runs under the per-workspace lifecycle lock: it reads + // the row in the issue scope, rebuilds the worktree, and only then publishes + // the row as "active". A rebuild failure keeps the row closed and returns an + // error, so the caller never dispatches a run against a broken workspace. + reopenClosedIsolatedExecutionWorkspaceForIssue: async (input: { + workspaceId: string; + issue: { id: string; companyId: string; projectId: string | null }; + actor: { agentId: string | null; actorType: string }; + }): Promise => { + const { issue, actor } = input; + // Bind the workspace to the issue company and project. A null project on + // the issue must match a null project on the row (IS NOT DISTINCT FROM). + const projectIdCondition = + issue.projectId == null + ? sql`${executionWorkspaces.projectId} IS NULL` + : eq(executionWorkspaces.projectId, issue.projectId); + + return db.transaction(async (tx): Promise => { + await acquireExecutionWorkspaceLifecycleLock(tx, input.workspaceId); + const row = await tx + .select() + .from(executionWorkspaces) + .where(and( + eq(executionWorkspaces.id, input.workspaceId), + eq(executionWorkspaces.companyId, issue.companyId), + projectIdCondition, + eq(executionWorkspaces.mode, "isolated_workspace"), + )) + .then((rows) => rows[0] ?? null); + if (!row) { + // Wrong company, wrong project, wrong mode, or missing. Fail closed and + // disclose no workspace detail. + return { ok: false, code: "not_reopenable", message: "Execution workspace is not reopenable" }; + } + if (!isClosedExecutionWorkspaceStatus(row.status)) { + // A concurrent reopen already restored the row. Report success without a + // second rebuild so the caller continues normally. + return { ok: true, reopened: false, workspace: toExecutionWorkspace(row) }; + } + + const [{ ensurePersistedExecutionWorkspaceAvailable }, { workspaceOperationService }] = + await Promise.all([ + import("./workspace-runtime.js"), + import("./workspace-operations.js"), + ]); + const [projectWorkspace, projectPolicy] = await Promise.all([ + row.projectWorkspaceId + ? db + .select({ cwd: projectWorkspaces.cwd }) + .from(projectWorkspaces) + .where(and( + eq(projectWorkspaces.companyId, row.companyId), + eq(projectWorkspaces.id, row.projectWorkspaceId), + )) + .then((rows) => rows[0] ?? null) + : null, + db + .select({ executionWorkspacePolicy: projects.executionWorkspacePolicy }) + .from(projects) + .where(and(eq(projects.companyId, row.companyId), eq(projects.id, row.projectId))) + .then((rows) => parseProjectExecutionWorkspacePolicy(rows[0]?.executionWorkspacePolicy)), + ]); + const config = readExecutionWorkspaceConfig(row.metadata as Record | null); + const nextGeneration = readExecutionWorkspaceLifecycleGeneration( + row.metadata as Record | null, + ) + 1; + const nextMetadata = bumpExecutionWorkspaceLifecycleGeneration( + row.metadata as Record | null, + ); + const recorder = workspaceOperationService(db).createRecorder({ + companyId: row.companyId, + executionWorkspaceId: row.id, + }); + + let rebuildError: string | null = null; + try { + const realized = await ensurePersistedExecutionWorkspaceAvailable({ + db: tx as unknown as Db, + base: { + baseCwd: projectWorkspace?.cwd ?? row.cwd ?? "", + source: "task_session", + projectId: row.projectId, + workspaceId: row.projectWorkspaceId, + repoUrl: row.repoUrl, + repoRef: row.baseRef, + }, + workspace: { + id: row.id, + mode: row.mode, + strategyType: row.strategyType, + cwd: row.cwd, + providerRef: row.providerRef, + projectId: row.projectId, + projectWorkspaceId: row.projectWorkspaceId, + repoUrl: row.repoUrl, + baseRef: row.baseRef, + branchName: row.branchName, + metadata: row.metadata as Record | null, + config: { + ...config, + provisionCommand: + config?.provisionCommand + ?? projectPolicy?.workspaceStrategy?.provisionCommand + ?? null, + }, + }, + issue: row.sourceIssueId + ? { id: row.sourceIssueId, identifier: null, title: row.name } + : null, + agent: { + id: actor.agentId ?? null, + name: actor.actorType === "user" ? "Board" : "Agent", + companyId: row.companyId, + }, + recorder, + }); + if (!realized) { + rebuildError = "Execution workspace could not be rebuilt"; + } + } catch (error) { + rebuildError = error instanceof Error ? error.message : String(error); + } + + if (rebuildError) { + // The rebuild failed. Keep the row closed and retryable. Raise the + // generation so a queued cleanup that captured the old generation does + // nothing. Clear cleanupEligibleAt so the reaper does not destroy the + // half-built worktree while a later reopen retries. + await tx + .update(executionWorkspaces) + .set({ + cleanupReason: EXECUTION_WORKSPACE_REOPEN_FAILED_REASON, + cleanupEligibleAt: null, + metadata: nextMetadata, + updatedAt: new Date(), + }) + .where(eq(executionWorkspaces.id, row.id)); + // The server log carries the underlying cause for diagnosis. The audit + // event and the returned message stay free of repo URLs, host paths, + // and git output. + logger.warn( + { + event: "execution_workspace.reopen", + outcome: "rebuild_failed", + executionWorkspaceId: row.id, + issueId: issue.id, + companyId: row.companyId, + actorType: actor.actorType, + actorAgentId: actor.agentId ?? null, + generation: nextGeneration, + error: rebuildError, + }, + "execution workspace reopen rebuild failed", + ); + await logActivity(tx as unknown as Db, { + companyId: row.companyId, + actorType: actor.actorType === "user" ? "user" : "agent", + actorId: actor.agentId ?? "system", + agentId: actor.actorType === "user" ? null : actor.agentId, + action: "execution_workspace.reopen_failed", + entityType: "execution_workspace", + entityId: row.id, + details: { + issueId: issue.id, + outcome: "rebuild_failed", + generation: nextGeneration, + }, + }); + return { ok: false, code: "rebuild_failed", message: "Failed to rebuild the execution workspace" }; + } + + // The rebuild succeeded. Publish the row as active in one write, and clear + // the closed markers. + const activeRow = await tx + .update(executionWorkspaces) + .set({ + status: "active", + closedAt: null, + cleanupReason: null, + cleanupEligibleAt: null, + metadata: nextMetadata, + lastUsedAt: new Date(), + updatedAt: new Date(), + }) + .where(eq(executionWorkspaces.id, row.id)) + .returning() + .then((rows) => rows[0] ?? null); + if (!activeRow) { + return { ok: false, code: "rebuild_failed", message: "Failed to rebuild the execution workspace" }; + } + logger.info( + { + event: "execution_workspace.reopen", + outcome: "reopened", + executionWorkspaceId: row.id, + issueId: issue.id, + companyId: row.companyId, + actorType: actor.actorType, + actorAgentId: actor.agentId ?? null, + generation: nextGeneration, + }, + "execution workspace reopened", + ); + await logActivity(tx as unknown as Db, { + companyId: row.companyId, + actorType: actor.actorType === "user" ? "user" : "agent", + actorId: actor.agentId ?? "system", + agentId: actor.actorType === "user" ? null : actor.agentId, + action: "execution_workspace.reopened", + entityType: "execution_workspace", + entityId: row.id, + details: { + issueId: issue.id, + outcome: "reopened", + generation: nextGeneration, + }, + }); + return { ok: true, reopened: true, workspace: toExecutionWorkspace(activeRow) }; + }); + }, + + // Read the lifecycle generation of a workspace row. The archive route captures + // this before it destroys, so it can hand the value to the destruction fence. + readLifecycleGeneration: async (id: string): Promise => { + const row = await db + .select({ metadata: executionWorkspaces.metadata }) + .from(executionWorkspaces) + .where(eq(executionWorkspaces.id, id)) + .then((rows) => rows[0] ?? null); + return row ? readExecutionWorkspaceLifecycleGeneration(row.metadata as Record | null) : null; + }, + + // Run a destructive workspace cleanup under the per-workspace lifecycle lock. + // The caller passes the generation it captured at archive time. If a reopen + // raised the generation or restored the row to an open status, the fence skips + // the destroy callback, so a cleanup never deletes a worktree that a reopen + // rebuilt. + fenceClosedWorkspaceDestruction: async (input: { + workspaceId: string; + capturedGeneration: number; + destroy: () => Promise; + }): Promise<{ skippedReopened: true } | { skippedReopened: false; result: T }> => { + return db.transaction(async (tx) => { + await acquireExecutionWorkspaceLifecycleLock(tx, input.workspaceId); + const fresh = await tx + .select({ status: executionWorkspaces.status, metadata: executionWorkspaces.metadata }) + .from(executionWorkspaces) + .where(eq(executionWorkspaces.id, input.workspaceId)) + .then((rows) => rows[0] ?? null); + const currentGeneration = fresh + ? readExecutionWorkspaceLifecycleGeneration(fresh.metadata as Record | null) + : null; + if ( + !fresh + || !isClosedExecutionWorkspaceStatus(fresh.status) + || currentGeneration !== input.capturedGeneration + ) { + logger.info( + { + event: "execution_workspace.cleanup_skipped", + reason: "reopened", + executionWorkspaceId: input.workspaceId, + capturedGeneration: input.capturedGeneration, + currentGeneration, + currentStatus: fresh?.status ?? null, + }, + "execution workspace cleanup skipped because it was reopened", + ); + return { skippedReopened: true as const }; + } + const result = await input.destroy(); + return { skippedReopened: false as const, result }; + }); + }, + reconcileExecutionWorkspaceBranch: async ( id: string, input: { diff --git a/ui/src/pages/IssueDetail.tsx b/ui/src/pages/IssueDetail.tsx index f35c552f600..4a620be8fc8 100644 --- a/ui/src/pages/IssueDetail.tsx +++ b/ui/src/pages/IssueDetail.tsx @@ -208,7 +208,6 @@ import { import { Badge } from "@/components/ui/badge"; import { deriveOriginatingActor, - getClosedIsolatedExecutionWorkspaceMessage, isClosedIsolatedExecutionWorkspace, ISSUE_CONTINUATION_SUMMARY_DOCUMENT_KEY, ONBOARDING_FIRST_TASK_ORIGIN_KIND, @@ -1759,12 +1758,16 @@ export function IssueDetail() { }); const resolvedCompanyId = issue?.companyId ?? selectedCompanyId; const externalObjectsState = useIssueExternalObjects(issue?.id ?? null); - const commentComposerDisabledReason = useMemo(() => { - if (!issue?.currentExecutionWorkspace || !isClosedIsolatedExecutionWorkspace(issue.currentExecutionWorkspace)) { - return null; - } - return getClosedIsolatedExecutionWorkspaceMessage(issue.currentExecutionWorkspace); - }, [issue?.currentExecutionWorkspace]); + // A closed isolated workspace no longer blocks the composer. The server reopens + // the workspace when the next comment or resume arrives, so the composer stays + // enabled and a hint tells the user what happens. + const closedIsolatedWorkspaceReopenPending = useMemo( + () => Boolean( + issue?.currentExecutionWorkspace + && isClosedIsolatedExecutionWorkspace(issue.currentExecutionWorkspace), + ), + [issue?.currentExecutionWorkspace], + ); const { data: commentPages, @@ -4333,7 +4336,10 @@ export function IssueDetail() { : "Assign an agent to wake them for triage while the subtree remains paused." ) : null; - const composerHint = pausedComposerHint; + const reopenComposerHint = closedIsolatedWorkspaceReopenPending + ? "This issue's isolated workspace was archived. Your next comment or resume reopens it and rebuilds the worktree." + : null; + const composerHint = pausedComposerHint ?? reopenComposerHint; const queuedCommentReason: "hold" | "active_run" | "other" = activePauseHold ? "hold" : "active_run"; const canApplyTreeControl = Boolean(treeControlPreview) @@ -5244,7 +5250,7 @@ export function IssueDetail() { currentAssigneeValue={actualAssigneeValue} suggestedAssigneeValue={suggestedAssigneeValue} mentions={mentionOptions} - composerDisabledReason={commentComposerDisabledReason} + composerDisabledReason={null} composerHint={composerHint} queuedCommentReason={queuedCommentReason} onVote={handleCommentVote}