Skip to content

Commit 9ace763

Browse files
fix: guard Paperclip worktree cleanup lifecycle (WAT-1777)
Co-Authored-By: Paperclip <noreply@paperclip.ing>
1 parent 5a0c197 commit 9ace763

5 files changed

Lines changed: 157 additions & 17 deletions

File tree

doc/experimental/issue-worktree-support.md

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,45 @@ We are intentionally not shipping the UI for this yet. The runtime code remains
2222
- seeded worktree instances can keep local-encrypted secrets working
2323
- seeded worktree instances can rebind same-repo project workspace paths onto the current git worktree
2424

25+
## Large-repo Worktree Lifecycle Policy
26+
27+
Paperclip-owned issue and review worktrees are execution workspaces, not agent-owned scratch directories. The execution workspace record is the source of truth for:
28+
29+
- owner: the Paperclip company/project plus the issue linked through `sourceIssueId` and `issues.executionWorkspaceId`
30+
- linked work: the issue identifier, branch name, base ref, project workspace, and any PR link carried in the issue thread
31+
- creation and use time: `openedAt`, `createdAt`, and `lastUsedAt`
32+
- cleanup eligibility: `closedAt`, `cleanupEligibleAt`, `cleanupReason`, and workspace `status`
33+
- provider ownership: `providerType: "git_worktree"` plus `metadata.createdByRuntime: true` means Paperclip may remove the derived worktree after close-readiness passes
34+
35+
Retention defaults for large local worktrees:
36+
37+
- active or linked to open issue: keep; report size and readiness only
38+
- clean, idle, Paperclip-created issue worktree: eligible for cleanup after 24 hours
39+
- in review: keep for 7 days unless a reviewer explicitly archives the workspace earlier
40+
- `cleanup_failed`: keep until an operator resolves the reported reason
41+
- shared/project-primary workspace: never delete the underlying project checkout; archive only the execution workspace record
42+
43+
Close and cleanup must use the execution workspace close-readiness report before deleting anything. The report includes linked open issues, runtime services, git dirty state, untracked files, ahead/behind counts, merge status, and planned cleanup actions. Destructive archive is blocked for isolated git worktrees when:
44+
45+
- the workspace is linked to any non-terminal issue
46+
- tracked files are modified
47+
- untracked files are present
48+
- commits are ahead of the base ref and not merged
49+
- git status cannot be inspected
50+
51+
The lower-level cleanup helper also refuses to remove a git worktree when `git status --porcelain --untracked-files=all` is non-empty, so direct callers report dirty worktrees instead of deleting them. If a worktree is clean and Paperclip owns it, cleanup removes the git worktree and then attempts to delete the runtime-created branch with `git branch -d`; unmerged branches are reported and kept.
52+
53+
Safe stale-worktree reporting should list, but not delete, candidate `/tmp` worktrees matching all of:
54+
55+
- provider is `git_worktree`
56+
- workspace path resolves under `/tmp` or the platform temp directory
57+
- status is `idle`, `in_review`, or `cleanup_failed`, or `lastUsedAt` is older than the retention window
58+
- close-readiness is not `ready`
59+
60+
Each row should include workspace id, issue identifier, owner agent if known, path, branch, base ref, opened/last-used time, approximate path size, readiness state, blocking reasons, and planned actions. Deletion is only allowed by the archive/close path after readiness returns `ready` or `ready_with_warnings`; blocked rows remain report-only.
61+
62+
Successful release/completion behavior: when a Paperclip-owned isolated worktree is closed, the server archives the execution workspace record, stops attached runtime services, runs configured cleanup/teardown commands, removes the clean git worktree, and records any cleanup warning in `cleanupReason`. If release/completion does not explicitly archive the workspace, the stale-worktree report is the follow-up mechanism that makes the retention breach visible before manual or scheduled archive.
63+
2564
## Hidden UI entrypoints
2665

2766
These are the current user-facing UI surfaces for the feature, now intentionally disabled:

server/src/__tests__/execution-workspaces-service.test.ts

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -343,7 +343,7 @@ describeEmbeddedPostgres("executionWorkspaceService.getCloseReadiness", () => {
343343
expect(readExecutionWorkspaceConfig(byId.get(untouchedWorkspaceId) ?? null)).toBeNull();
344344
});
345345

346-
it("warns about dirty and unmerged git worktrees and reports cleanup actions", async () => {
346+
it("blocks destructive close for dirty and unmerged git worktrees", async () => {
347347
const repoRoot = await createTempRepo();
348348
tempDirs.add(repoRoot);
349349
const worktreePath = path.join(path.dirname(repoRoot), `paperclip-worktree-${randomUUID()}`);
@@ -416,10 +416,10 @@ describeEmbeddedPostgres("executionWorkspaceService.getCloseReadiness", () => {
416416

417417
expect(readiness).toMatchObject({
418418
workspaceId: executionWorkspaceId,
419-
state: "ready_with_warnings",
419+
state: "blocked",
420420
isSharedWorkspace: false,
421421
isProjectPrimaryWorkspace: false,
422-
isDestructiveCloseAllowed: true,
422+
isDestructiveCloseAllowed: false,
423423
git: {
424424
workspacePath: worktreePath,
425425
branchName: "paperclip-close-check",
@@ -432,6 +432,10 @@ describeEmbeddedPostgres("executionWorkspaceService.getCloseReadiness", () => {
432432
isMergedIntoBase: false,
433433
},
434434
});
435+
expect(readiness?.blockingReasons).toEqual(expect.arrayContaining([
436+
"This git worktree has 1 untracked file; archive it after committing, stashing, or removing the file.",
437+
"This git worktree has 1 unmerged commit ahead of main; push or merge it before archive cleanup.",
438+
]));
435439
expect(readiness?.warnings).toEqual(expect.arrayContaining([
436440
"The workspace has 1 untracked file.",
437441
"This workspace is 1 commit ahead of main and is not merged.",

server/src/__tests__/workspace-runtime.test.ts

Lines changed: 61 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1976,6 +1976,67 @@ describe("realizeExecutionWorkspace", () => {
19761976
});
19771977
}, 10_000);
19781978

1979+
it("reports dirty git worktrees instead of removing them during cleanup", async () => {
1980+
const repoRoot = await createTempRepo();
1981+
1982+
const workspace = await realizeExecutionWorkspace({
1983+
base: {
1984+
baseCwd: repoRoot,
1985+
source: "project_primary",
1986+
projectId: "project-1",
1987+
workspaceId: "workspace-1",
1988+
repoUrl: null,
1989+
repoRef: "HEAD",
1990+
},
1991+
config: {
1992+
workspaceStrategy: {
1993+
type: "git_worktree",
1994+
branchTemplate: "{{issue.identifier}}-{{slug}}",
1995+
},
1996+
},
1997+
issue: {
1998+
id: "issue-1",
1999+
identifier: "PAP-452",
2000+
title: "Keep dirty worktree",
2001+
},
2002+
agent: {
2003+
id: "agent-1",
2004+
name: "Codex Coder",
2005+
companyId: "company-1",
2006+
},
2007+
});
2008+
2009+
await fs.writeFile(path.join(workspace.cwd, "scratch.txt"), "still here\n", "utf8");
2010+
2011+
const cleanup = await cleanupExecutionWorkspaceArtifacts({
2012+
workspace: {
2013+
id: "execution-workspace-1",
2014+
cwd: workspace.cwd,
2015+
providerType: "git_worktree",
2016+
providerRef: workspace.worktreePath,
2017+
branchName: workspace.branchName,
2018+
repoUrl: workspace.repoUrl,
2019+
baseRef: workspace.repoRef,
2020+
projectId: workspace.projectId,
2021+
projectWorkspaceId: workspace.workspaceId,
2022+
sourceIssueId: "issue-1",
2023+
metadata: {
2024+
createdByRuntime: true,
2025+
},
2026+
},
2027+
projectWorkspace: {
2028+
cwd: repoRoot,
2029+
cleanupCommand: null,
2030+
},
2031+
});
2032+
2033+
expect(cleanup.cleaned).toBe(false);
2034+
expect(cleanup.warnings).toEqual(expect.arrayContaining([
2035+
`Skipped removing git worktree "${workspace.cwd}" because it has uncommitted or untracked files.`,
2036+
]));
2037+
await expect(fs.stat(path.join(workspace.cwd, "scratch.txt"))).resolves.toBeTruthy();
2038+
});
2039+
19792040
it("records teardown and cleanup operations when a recorder is provided", async () => {
19802041
const repoRoot = await createTempRepo();
19812042
const { recorder, operations } = createWorkspaceOperationRecorderDouble();

server/src/services/execution-workspaces.ts

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -591,20 +591,41 @@ export function executionWorkspaceService(db: Db) {
591591
? "The workspace has 1 modified tracked file."
592592
: `The workspace has ${git.dirtyEntryCount} modified tracked files.`,
593593
);
594+
if (!isSharedWorkspace && executionWorkspace.providerType === "git_worktree") {
595+
blockingReasons.push(
596+
git.dirtyEntryCount === 1
597+
? "This git worktree has 1 modified tracked file; archive it after committing, stashing, or reverting the file."
598+
: `This git worktree has ${git.dirtyEntryCount} modified tracked files; archive it after committing, stashing, or reverting the files.`,
599+
);
600+
}
594601
}
595602
if (git?.hasUntrackedFiles) {
596603
warnings.push(
597604
git.untrackedEntryCount === 1
598605
? "The workspace has 1 untracked file."
599606
: `The workspace has ${git.untrackedEntryCount} untracked files.`,
600607
);
608+
if (!isSharedWorkspace && executionWorkspace.providerType === "git_worktree") {
609+
blockingReasons.push(
610+
git.untrackedEntryCount === 1
611+
? "This git worktree has 1 untracked file; archive it after committing, stashing, or removing the file."
612+
: `This git worktree has ${git.untrackedEntryCount} untracked files; archive it after committing, stashing, or removing the files.`,
613+
);
614+
}
601615
}
602616
if (git?.aheadCount && git.aheadCount > 0 && git.isMergedIntoBase === false) {
603617
warnings.push(
604618
git.aheadCount === 1
605619
? `This workspace is 1 commit ahead of ${git.baseRef ?? "the base ref"} and is not merged.`
606620
: `This workspace is ${git.aheadCount} commits ahead of ${git.baseRef ?? "the base ref"} and is not merged.`,
607621
);
622+
if (!isSharedWorkspace && executionWorkspace.providerType === "git_worktree") {
623+
blockingReasons.push(
624+
git.aheadCount === 1
625+
? `This git worktree has 1 unmerged commit ahead of ${git.baseRef ?? "the base ref"}; push or merge it before archive cleanup.`
626+
: `This git worktree has ${git.aheadCount} unmerged commits ahead of ${git.baseRef ?? "the base ref"}; push or merge them before archive cleanup.`,
627+
);
628+
}
608629
}
609630
if (git?.behindCount && git.behindCount > 0) {
610631
warnings.push(

server/src/services/workspace-runtime.ts

Lines changed: 29 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1377,22 +1377,37 @@ export async function cleanupExecutionWorkspaceArtifacts(input: {
13771377
if (!repoRoot) {
13781378
warnings.push(`Could not resolve git repo root for "${workspacePath}".`);
13791379
} else {
1380+
let gitStatus: string | null = null;
13801381
try {
1381-
await recordGitOperation(input.recorder, {
1382-
phase: "worktree_cleanup",
1383-
args: ["worktree", "remove", "--force", workspacePath],
1384-
cwd: repoRoot,
1385-
metadata: {
1386-
workspaceId: input.workspace.id,
1387-
workspacePath,
1388-
branchName: input.workspace.branchName,
1389-
cleanupAction: "worktree_remove",
1390-
},
1391-
successMessage: `Removed git worktree ${workspacePath}\n`,
1392-
failureLabel: `git worktree remove ${workspacePath}`,
1393-
});
1382+
gitStatus = await runGit(["status", "--porcelain=v1", "--untracked-files=all"], workspacePath);
13941383
} catch (err) {
1395-
warnings.push(err instanceof Error ? err.message : String(err));
1384+
warnings.push(
1385+
`Skipped removing git worktree "${workspacePath}" because Paperclip could not inspect git status: ${
1386+
err instanceof Error ? err.message : String(err)
1387+
}`,
1388+
);
1389+
}
1390+
1391+
if (gitStatus && gitStatus.length > 0) {
1392+
warnings.push(`Skipped removing git worktree "${workspacePath}" because it has uncommitted or untracked files.`);
1393+
} else if (gitStatus === "") {
1394+
try {
1395+
await recordGitOperation(input.recorder, {
1396+
phase: "worktree_cleanup",
1397+
args: ["worktree", "remove", "--force", workspacePath],
1398+
cwd: repoRoot,
1399+
metadata: {
1400+
workspaceId: input.workspace.id,
1401+
workspacePath,
1402+
branchName: input.workspace.branchName,
1403+
cleanupAction: "worktree_remove",
1404+
},
1405+
successMessage: `Removed git worktree ${workspacePath}\n`,
1406+
failureLabel: `git worktree remove ${workspacePath}`,
1407+
});
1408+
} catch (err) {
1409+
warnings.push(err instanceof Error ? err.message : String(err));
1410+
}
13961411
}
13971412
}
13981413
}

0 commit comments

Comments
 (0)