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}