You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Operators can now apply the named worktree cleanup plan they reviewed. Existing discard authorization governs application; the plan is claimed once, current ownership and Git facts are rechecked under shared cleanup/resume locks, and a durable receipt records each outcome. Removal retains the local branch and protects active, resumed, session-dependent, dirty, ignored-file and Git-locked trees. Interrupted journaled removals can repair matching handoff facts without deleting a recreated path.
Validation: typecheck, package build, focused cleanup and handoff tests, real executor authorization and external-job resume tests, and independent review passed on Linux Node 24. Disposable Git fixtures cover two-process contention, interrupted receipts, cancellation, drift and retained branches. Focused validation was repeated after rebasing onto 0.75.0. Required exact-head CI and macOS validation remain pending.
CI follow-up: the first Linux/Windows runs exposed a resume-validation ordering regression and a Windows path-identity mismatch when recording removals. Commit 7ac3537a restores the original baseRef rejection before lock admission and reuses the planner's existing filesystem-aware path comparison for handoff bookkeeping, including relative paths. The interrupted-receipt fixture now uses the canonical plan repository root; receipt/hash validation remains unchanged. Typecheck, package build and 36 focused cleanup/resume tests pass on Linux. Updated exact-head CI is running.
Review follow-up: record each removing intent atomically before invoking Git, then update that same receipt entry on success. An interrupted attempt can repair removal facts only after both directory and Git registration are absent; replay preserves recreated trees and newer handoff ownership and does not execute remaining candidates. Recover proven-dead locks with a token-specific atomic directory rename, retaining the nonempty old owner record so an interrupted or delayed reclaimer cannot block maintenance or move a new live lease. Unknown and live owners are still preserved.
Validation: typecheck, package build and 54 focused cleanup/handoff/lock tests pass on Linux, including real child-process interruption at the removal and recovery boundaries. Add a focused macOS cleanup/recovery job; the existing Windows matrix runs the portable regression tests. Checks on this appended commit are pending.
Follow-up head: 06d53c59.
CI update for 06d53c59: 15 of 16 configured checks passed, including the new macOS cleanup/recovery job and the portable unit regressions in the Windows matrix. The remaining failure is background keeps a terminal answer authoritative over an earlier tool timeout in the existing Windows integration suite (job); that test and execution path are unchanged by this follow-up. It passes when run alone on Linux, but the failed Windows check needs a maintainer rerun before merge readiness can be confirmed. The full Linux unit suite passed (3,691 passed, 15 existing skips). The latest code re-review confirms both original findings are addressed.
Maintainer review follow-up (befdd380): record the canonical worktree parent at allocation and carry that proof through handoff and cleanup records. Planning and apply retain paths outside that saved creation directory, and older handoffs without the proof remain kept. Use the full handoff validator during planning and under the handoff write lock before admitting a removal or invoking Git; malformed records stay on disk. Remove docs/design/worktree-retention.md and document the shipped checks in the tool reference.
Validation: typecheck, package build, 116 cleanup/handoff/worktree/lock tests, 9 focused cases through a symlinked temporary-root alias, and both tool-activation smoke tests passed. Disposable Git regressions cover ancestor redirection before and after planning, invalid child indexes and missing patch objects, missing creation-base proof, and propagation through the real handoff/cleanup writers. The previously failing timeout integration test also passed in an isolated Linux run. Fresh checks on this appended head must establish the Windows result.
Integration follow-up (3a429886): merge current main (8fb89814) without rewriting published history, preserving both changelog contributions. Update the two existing integration handoffs to carry creation-time base proof: the external-job follow-up fixture records it immediately after allocation, and the public cleanup-action fixture uses the proof returned by createWorktrees. Both previously failing scenarios now pass and still exercise resume protection and authorized removal. Typecheck, package build and all 116 focused cleanup tests were repeated after integrating main. Fresh exact-head Linux/Windows/macOS checks are pending.
CI update for 3a429886: all 16 configured checks passed, including every Linux/Windows shard, macOS cleanup/recovery and native nested-wait coverage, loader checks, and standalone/clean-install jobs (run). The formerly failing windows-3 shard passed. An independent before/after replay in disposable Git repositories confirmed that both reported cases delete trees on the old head and preserve them after the fix. The latest review reports no outstanding code findings, and all inline review threads are resolved.
[High risk] Adds file-system cleanup execution with locking and plan validation.
No outstanding code finding was identified, but merge readiness requires confirmation of required CI on the exact PR head.
Summary
The PR adds authorized application of reviewed worktree cleanup plans, with locked revalidation, single-use receipts, and conservative recovery of interrupted removals.
Records creation-directory proof and retains trees when ownership, path, session, or Git safety checks fail.
Adds focused cleanup and lock-recovery tests, updates the tool reference, and credits @quifox in the changelog.
Exact-head CI for 8db86e4c was not verified; the supplied passing CI result is for the earlier 3a429886 head.
Diagram
%%{init: {'theme': 'neutral'}}%%
flowchart LR
Plan[Save reviewed plan] --> Auth[Authorize apply]
Auth --> Lock[Acquire repository and handoff locks]
Lock --> Check[Revalidate ownership and Git facts]
Check -->|unsafe or changed| Keep[Record kept outcome]
Check -->|eligible| Intent[Persist removal intent]
Intent --> Git[Remove worktree; retain branch]
Git --> Receipt[Update receipt and handoff]
Intent -->|interrupted| Replay[Reconcile without repeating deletion]
Reviewed at 06d53c59 against #1622. Authority checks, single-use claims, journaling before Git, recovery that preserves recreated paths, and the lock fencing all hold up, and the recovery tests cover the real destructive boundaries. Two keep rules from #1622 still fail, though, and both were reproduced with disposable Git repos on this head. Apply turns these planner gaps, which were harmless while cleanup was plan-only, into deletions:
A symlinked ancestor lets apply delete a tree outside its creation base (worktree-cleanup-apply.ts:114-124, worktree-cleanup-plan.ts:537, 619-636). recordedBaseDir is derived from the candidate's current resolved path, and only the leaf is checked with lstat. Repro: create a managed worktree at base/repo/pi-worktree-review-0 with a valid terminal preserved handoff, move base/repo to outside, replace it with a directory symlink to outside, then plan and apply with the original base config. The plan marks outside/pi-worktree-review-0 safe with recordedBaseDir: outside, and apply removes it (receipt complete). The base needs to come from creation-time metadata that apply compares against, and entries without that proof should be kept.
Malformed handoff metadata is detected only after Git has removed the tree (worktree-cleanup-apply.ts:114-129; full validator in parallel-handoff.ts:95-104 runs only in recordRemoval). Repro: plan a removable entry, then change groups[0].children[0].index to -1 in the handoff and apply the unchanged plan. The worktree is removed, and the receipt then reports partial with the validation error. A child patch object that's missing before planning behaves the same way. The full manifest validation (readParallelHandoffManifest) needs to run under the handoff lock before journaling and Git, and the planner should keep entries it can't validate.
Please drop docs/design/worktree-retention.md. It's a draft (publication pending, the count-retention proposal, future touchpoints), and docs/design/ doesn't exist on main. docs/tool-reference.md already documents the shipped interface.
Please add a regression for each of 1 and 2 using your disposable-Git fixtures. On CI, test (windows-3) failed background keeps a terminal answer authoritative over an earlier tool timeout. I found no causal link to this diff, and it passes locally, but I also didn't find it failing elsewhere, so the next push needs a green Windows run. #2657 is closed separately (see the comment there).
Merged, thanks @quifox. Both deletion fixes held up in a re-review that reran the original reproductions. I folded the follow-up changelog line into the feature entry, since the feature has not been released yet.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Operators can now apply the named worktree cleanup plan they reviewed. Existing discard authorization governs application; the plan is claimed once, current ownership and Git facts are rechecked under shared cleanup/resume locks, and a durable receipt records each outcome. Removal retains the local branch and protects active, resumed, session-dependent, dirty, ignored-file and Git-locked trees. Interrupted journaled removals can repair matching handoff facts without deleting a recreated path.
Validation: typecheck, package build, focused cleanup and handoff tests, real executor authorization and external-job resume tests, and independent review passed on Linux Node 24. Disposable Git fixtures cover two-process contention, interrupted receipts, cancellation, drift and retained branches. Focused validation was repeated after rebasing onto 0.75.0. Required exact-head CI and macOS validation remain pending.
Related: #1622.
CI follow-up: the first Linux/Windows runs exposed a resume-validation ordering regression and a Windows path-identity mismatch when recording removals. Commit
7ac3537arestores the original baseRef rejection before lock admission and reuses the planner's existing filesystem-aware path comparison for handoff bookkeeping, including relative paths. The interrupted-receipt fixture now uses the canonical plan repository root; receipt/hash validation remains unchanged. Typecheck, package build and 36 focused cleanup/resume tests pass on Linux. Updated exact-head CI is running.Review follow-up: record each
removingintent atomically before invoking Git, then update that same receipt entry on success. An interrupted attempt can repair removal facts only after both directory and Git registration are absent; replay preserves recreated trees and newer handoff ownership and does not execute remaining candidates. Recover proven-dead locks with a token-specific atomic directory rename, retaining the nonempty old owner record so an interrupted or delayed reclaimer cannot block maintenance or move a new live lease. Unknown and live owners are still preserved.Validation: typecheck, package build and 54 focused cleanup/handoff/lock tests pass on Linux, including real child-process interruption at the removal and recovery boundaries. Add a focused macOS cleanup/recovery job; the existing Windows matrix runs the portable regression tests. Checks on this appended commit are pending.
Follow-up head:
06d53c59.CI update for
06d53c59: 15 of 16 configured checks passed, including the new macOS cleanup/recovery job and the portable unit regressions in the Windows matrix. The remaining failure isbackground keeps a terminal answer authoritative over an earlier tool timeoutin the existing Windows integration suite (job); that test and execution path are unchanged by this follow-up. It passes when run alone on Linux, but the failed Windows check needs a maintainer rerun before merge readiness can be confirmed. The full Linux unit suite passed (3,691 passed, 15 existing skips). The latest code re-review confirms both original findings are addressed.Maintainer review follow-up (
befdd380): record the canonical worktree parent at allocation and carry that proof through handoff and cleanup records. Planning and apply retain paths outside that saved creation directory, and older handoffs without the proof remain kept. Use the full handoff validator during planning and under the handoff write lock before admitting a removal or invoking Git; malformed records stay on disk. Removedocs/design/worktree-retention.mdand document the shipped checks in the tool reference.Validation: typecheck, package build, 116 cleanup/handoff/worktree/lock tests, 9 focused cases through a symlinked temporary-root alias, and both tool-activation smoke tests passed. Disposable Git regressions cover ancestor redirection before and after planning, invalid child indexes and missing patch objects, missing creation-base proof, and propagation through the real handoff/cleanup writers. The previously failing timeout integration test also passed in an isolated Linux run. Fresh checks on this appended head must establish the Windows result.
Integration follow-up (
3a429886): merge current main (8fb89814) without rewriting published history, preserving both changelog contributions. Update the two existing integration handoffs to carry creation-time base proof: the external-job follow-up fixture records it immediately after allocation, and the public cleanup-action fixture uses the proof returned bycreateWorktrees. Both previously failing scenarios now pass and still exercise resume protection and authorized removal. Typecheck, package build and all 116 focused cleanup tests were repeated after integrating main. Fresh exact-head Linux/Windows/macOS checks are pending.CI update for
3a429886: all 16 configured checks passed, including every Linux/Windows shard, macOS cleanup/recovery and native nested-wait coverage, loader checks, and standalone/clean-install jobs (run). The formerly failingwindows-3shard passed. An independent before/after replay in disposable Git repositories confirmed that both reported cases delete trees on the old head and preserve them after the fix. The latest review reports no outstanding code findings, and all inline review threads are resolved.