mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-10-26 10:26:26 +01:00
Use non-forced workspace close cleanup
Co-Authored-By: Paperclip <noreply@paperclip.ing>
This commit is contained in:
1 parent
75e58b71e8
commit
e8a59ca78f
4 files changed
+162
-7
No files matched your search
@@ -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);
|
||||
});
|
||||
@@ -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();
|
||||
|
||||
@@ -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}`,
|
||||
});
|
||||
}
|
||||
|
||||
|
||||
@@ -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,
|
||||
|
||||
Reference in new issue
Block a user