diff --git a/server/src/__tests__/execution-workspaces-service.test.ts b/server/src/__tests__/execution-workspaces-service.test.ts index d4a50bdccee..1adb85248b4 100644 --- a/server/src/__tests__/execution-workspaces-service.test.ts +++ b/server/src/__tests__/execution-workspaces-service.test.ts @@ -221,7 +221,7 @@ describeEmbeddedPostgres("executionWorkspaceService.getCloseReadiness", () => { ])); }); - it("warns about dirty and unmerged git worktrees and reports cleanup actions", async () => { + it("warns about unmerged git worktrees and reports cleanup actions", async () => { const repoRoot = await createTempRepo(); tempDirs.add(repoRoot); const worktreePath = path.join(path.dirname(repoRoot), `paperclip-worktree-${randomUUID()}`); @@ -232,7 +232,6 @@ describeEmbeddedPostgres("executionWorkspaceService.getCloseReadiness", () => { await fs.writeFile(path.join(worktreePath, "feature.txt"), "hello\n", "utf8"); await runGit(worktreePath, ["add", "feature.txt"]); await runGit(worktreePath, ["commit", "-m", "Feature commit"]); - await fs.writeFile(path.join(worktreePath, "untracked.txt"), "left behind\n", "utf8"); const companyId = randomUUID(); const projectId = randomUUID(); @@ -304,14 +303,13 @@ describeEmbeddedPostgres("executionWorkspaceService.getCloseReadiness", () => { baseRef: "main", createdByRuntime: true, hasDirtyTrackedFiles: false, - hasUntrackedFiles: true, + hasUntrackedFiles: false, aheadCount: 1, behindCount: 0, isMergedIntoBase: false, }, }); expect(readiness?.warnings).toEqual(expect.arrayContaining([ - "The workspace has 1 untracked file.", "This workspace is 1 commit ahead of main and is not merged.", ])); expect(readiness?.plannedActions.map((action) => action.kind)).toEqual(expect.arrayContaining([ @@ -322,4 +320,84 @@ describeEmbeddedPostgres("executionWorkspaceService.getCloseReadiness", () => { "git_branch_delete", ])); }, 20_000); + + it("blocks closing dirty git worktrees now that close is non-forced", async () => { + const repoRoot = await createTempRepo(); + tempDirs.add(repoRoot); + const worktreePath = path.join(path.dirname(repoRoot), `paperclip-worktree-${randomUUID()}`); + tempDirs.add(worktreePath); + + await runGit(repoRoot, ["branch", "paperclip-dirty-check"]); + await runGit(repoRoot, ["worktree", "add", worktreePath, "paperclip-dirty-check"]); + await fs.writeFile(path.join(worktreePath, "untracked.txt"), "left behind\n", "utf8"); + + const companyId = randomUUID(); + const projectId = randomUUID(); + const projectWorkspaceId = randomUUID(); + const executionWorkspaceId = randomUUID(); + + await db.insert(companies).values({ + id: companyId, + name: "Paperclip", + issuePrefix: "PAP", + requireBoardApprovalForNewAgents: false, + }); + await db.insert(projects).values({ + id: projectId, + companyId, + name: "Workspaces", + status: "in_progress", + executionWorkspacePolicy: { + enabled: true, + workspaceStrategy: { + type: "git_worktree", + }, + }, + }); + await db.insert(projectWorkspaces).values({ + id: projectWorkspaceId, + companyId, + projectId, + name: "Primary", + sourceType: "git_repo", + isPrimary: true, + cwd: repoRoot, + }); + await db.insert(executionWorkspaces).values({ + id: executionWorkspaceId, + companyId, + projectId, + projectWorkspaceId, + mode: "isolated_workspace", + strategyType: "git_worktree", + name: "Dirty workspace", + status: "active", + providerType: "git_worktree", + cwd: worktreePath, + providerRef: worktreePath, + branchName: "paperclip-dirty-check", + baseRef: "main", + metadata: { + createdByRuntime: true, + }, + }); + + const readiness = await svc.getCloseReadiness(executionWorkspaceId); + + expect(readiness).toMatchObject({ + workspaceId: executionWorkspaceId, + state: "blocked", + isDestructiveCloseAllowed: false, + git: { + hasDirtyTrackedFiles: false, + hasUntrackedFiles: true, + }, + }); + expect(readiness?.blockingReasons).toContain( + "This git worktree still has untracked files. Clean, commit, stash, or move them before closing.", + ); + expect(readiness?.plannedActions.find((action) => action.kind === "git_worktree_remove")).toMatchObject({ + command: `git worktree remove ${worktreePath}`, + }); + }, 20_000); }); diff --git a/server/src/__tests__/workspace-runtime.test.ts b/server/src/__tests__/workspace-runtime.test.ts index f158a5e9e63..05fe2e9b823 100644 --- a/server/src/__tests__/workspace-runtime.test.ts +++ b/server/src/__tests__/workspace-runtime.test.ts @@ -888,6 +888,72 @@ describe("realizeExecutionWorkspace", () => { }); }); + it("refuses to remove a dirty git worktree during cleanup", async () => { + const repoRoot = await createTempRepo(); + + const workspace = await realizeExecutionWorkspace({ + base: { + baseCwd: repoRoot, + source: "project_primary", + projectId: "project-1", + workspaceId: "workspace-1", + repoUrl: null, + repoRef: "HEAD", + }, + config: { + workspaceStrategy: { + type: "git_worktree", + branchTemplate: "{{issue.identifier}}-{{slug}}", + }, + }, + issue: { + id: "issue-1", + identifier: "PAP-1100", + title: "Dirty cleanup stays put", + }, + agent: { + id: "agent-1", + name: "Codex Coder", + companyId: "company-1", + }, + }); + + await fs.writeFile(path.join(workspace.cwd, "left-behind.txt"), "still dirty\n", "utf8"); + + const cleanup = await cleanupExecutionWorkspaceArtifacts({ + workspace: { + id: "execution-workspace-1", + cwd: workspace.cwd, + providerType: "git_worktree", + providerRef: workspace.worktreePath, + branchName: workspace.branchName, + repoUrl: workspace.repoUrl, + baseRef: workspace.repoRef, + projectId: workspace.projectId, + projectWorkspaceId: workspace.workspaceId, + sourceIssueId: "issue-1", + metadata: { + createdByRuntime: true, + }, + }, + projectWorkspace: { + cwd: repoRoot, + cleanupCommand: null, + }, + }); + + expect(cleanup.cleaned).toBe(false); + expect(cleanup.warnings).toHaveLength(2); + expect(cleanup.warnings[0]).toContain("use --force to delete it"); + expect(cleanup.warnings[1]).toContain(`Skipped deleting branch "${workspace.branchName}"`); + await expect(fs.stat(workspace.cwd)).resolves.toBeTruthy(); + await expect( + execFileAsync("git", ["branch", "--list", workspace.branchName!], { cwd: repoRoot }), + ).resolves.toMatchObject({ + stdout: expect.stringContaining(workspace.branchName!), + }); + }); + it("records teardown and cleanup operations when a recorder is provided", async () => { const repoRoot = await createTempRepo(); const { recorder, operations } = createWorkspaceOperationRecorderDouble(); diff --git a/server/src/services/execution-workspaces.ts b/server/src/services/execution-workspaces.ts index a1d3b41d69a..5b497b2e68e 100644 --- a/server/src/services/execution-workspaces.ts +++ b/server/src/services/execution-workspaces.ts @@ -508,6 +508,17 @@ export function executionWorkspaceService(db: Db) { ); } + if (!isSharedWorkspace && executionWorkspace.providerType === "git_worktree" && git) { + const dirtyConditions: string[] = []; + if (git.hasDirtyTrackedFiles) dirtyConditions.push("modified tracked files"); + if (git.hasUntrackedFiles) dirtyConditions.push("untracked files"); + if (dirtyConditions.length > 0) { + blockingReasons.push( + `This git worktree still has ${dirtyConditions.join(" and ")}. Clean, commit, stash, or move them before closing.`, + ); + } + } + const plannedActions: ExecutionWorkspaceCloseAction[] = [ { kind: "archive_record", @@ -562,8 +573,8 @@ export function executionWorkspaceService(db: Db) { plannedActions.push({ kind: "git_worktree_remove", label: "Remove git worktree", - description: `Paperclip will run git worktree cleanup for ${workspacePath}.`, - command: `git worktree remove --force ${workspacePath}`, + description: `Paperclip will remove the git worktree at ${workspacePath} without forcing dirty checkouts.`, + command: `git worktree remove ${workspacePath}`, }); } diff --git a/server/src/services/workspace-runtime.ts b/server/src/services/workspace-runtime.ts index a100242e665..1aae25634e8 100644 --- a/server/src/services/workspace-runtime.ts +++ b/server/src/services/workspace-runtime.ts @@ -816,7 +816,7 @@ export async function cleanupExecutionWorkspaceArtifacts(input: { try { await recordGitOperation(input.recorder, { phase: "worktree_cleanup", - args: ["worktree", "remove", "--force", workspacePath], + args: ["worktree", "remove", workspacePath], cwd: repoRoot, metadata: { workspaceId: input.workspace.id,