Repository navigation
Conversation
…moval-final-checks # Conflicts: # src/git/remove.rs
…ntity # Conflicts: # src/git/repository/tests.rs # src/git/test.rs
Remove the duplicate per-worktree fsmonitor config reader and early cleanliness check. Force still checks captured registration and locks once after shutdown; non-force full-scans through the canonical CleanCheckMode API and rechecks ownership after status. Disable fsmonitor for direct and detached fallback worktree removal and the submodule cleanliness gate. Drop unsupported operation-state refusal per identity review, and reduce shutdown regressions to untracked-work/force and forced ownership/lock contrasts with exact failure assertions. Non-force removal now pays a full scan to protect writes during shutdown; this tradeoff is proposed for review, not recorded as accepted. Source-only repair. Existing command-builder inline snapshots need Insta regeneration before validation.
…ntity # Conflicts: # src/git/repository/tests.rs
…al-checks # Conflicts: # src/git/remove.rs
…ree-containment # Conflicts: # src/git/remove.rs
Atomically move unregistered metadata into wt/trash and use one complete disposal inventory. Restore the state table and snapshots to main, remove the extra metadata prefix/root and payload rename reservation machinery, and trim their duplicate fixtures. Keep cancellation at the transaction mutation boundary and document preserved-removal errors. Keep git worktree repair of the retained checkout: moving it blindly back can touch a concurrent original-path occupant. Extend the existing recovery contrast to cover that occupant. Fallback fixtures now block the initial retained move, rather than later trash promotion. Source-only repair; no runtime validation. The ENOTEMPTY-only remaining-directory diagnostic and contrast are unchanged.
…t-registration-identity # Conflicts: # src/git/repository/tests.rs
…al-checks # Conflicts: # src/commands/process.rs
…n-identity # Conflicts: # src/git/repository/tests.rs # src/git/repository/working_tree.rs # src/git/repository/worktrees.rs # src/git/test.rs
# Conflicts: # src/commands/process.rs # src/git/repository/tests.rs # src/git/repository/working_tree.rs # src/git/test.rs # src/output/handlers.rs # tests/integration_tests/remove.rs
…worktree-containment
worktrunk-bot
left a comment
There was a problem hiding this comment.
The synchronous fallback now reaches Repository::remove_worktree and its final gates before Git deletes the checkout; stage_worktree_removal has a separate final gate before the trash rename. These checks decide whether --force can destroy a registered checkout or Git storage inside the removal target. The data-loss review rule requires maintainer review of changed deletion gates. I am withholding approval pending @max-sixty's signoff on both mutation paths.
…worktree-containment
worktrunk-bot
left a comment
There was a problem hiding this comment.
When staging fails, spawn_fallback queues the removal worker, but the caller then starts post-remove hooks without waiting for its result. For removal from the current checkout, the worker deliberately waits one second; a hook can therefore run while that checkout still exists. If the worker's final topology or cleanliness check refuses removal, the hook has run even though the checkout remains. The hook documentation says the active worktree is gone at post-remove, and teardown hooks can act on that assumption. Please keep those hooks behind successful worker removal on this path.
Removing a checkout must preserve registered checkouts and this repository's Git storage inside it, including with --force. Put the final containment, captured-registration, lock, and cancellation gates at both actual mutation boundaries: trash staging and direct Git removal. Registry synchronization keeps the decisive cleanliness and topology checks clear of concurrent teardown, with no duplicate scanner.
If staging fails, a private Rust worker preserves background execution while using the same guarded removal owner. Native checkout and captured-registration paths travel as argv; typed branch intent uses the existing detached stdin transport. The owner rechecks the captured registration after its registry wait, then checks containment, locks, and cancellation before deletion. This replaces the unchecked raw-Git child and separate branch-deletion tail. Noncurrent prune fallback remains synchronous so its summary reports an observed result; current-checkout removal can release its working directory before the worker deletes it.
The worker also owns the deferred post-remove event. The parent renders the already approved hook plan without dispatching it and sends that frozen batch through the same transport. After confirmed removal, the worker finishes the branch decision and dispatches post-remove through the existing hook runner, even if branch deletion failed. A refused removal starts no post-remove pipeline. An interrupted branch operation preserves its interruption without starting hooks; shared dispatch checks cancellation before announcing and before each new pipeline. Post-switch remains with the parent's actual directory change. The worker never selects or approves hooks again, and failed preparation cannot flush a partial batch through Drop.
Includes the registration-identity work from #4454 and recovery transactions from #4459. Landing order is #4454, then #4459, then this PR. The final staging checks, checkout move, and atomic unregister share one registry write lock; failed unregister restores the checkout or retains both payloads if restoration is blocked. Disposal preparation runs after releasing the lock. A committed transaction whose cleanup is deferred remains a successful removal and never falls through to another deletion attempt.
Also reuses the canonical separate-Git-store discovery correction reviewed in #4461, after reproducing an actual containment escape. An unknown primary checkout now requires Git repair rather than supplying a guessed root. No user-facing command, flag, or configuration is added. Deferred branch outcomes remain meaningful for delegated deletion; later worker failures go to its log, and preparation failures do not claim removal already completed.
Review scope: the whole stack is 47 files, +3,991/-1,319. The incremental diff after #4459 is 34 files, +2,335/-931. Most added lines are regression tests and captured output; most production churn is in the shared removal and output owners, including replacing the unchecked fallback with the native worker. The final test-only repair adds five actual deferred-output goldens (394 lines) while preserving the existing staged goldens.
Validation: the unchanged production code passed the full 5,507-test project gate (one skipped), plus formatting, Clippy, docs, doctests, and lockfile checks. All 253 focused combined safety tests and 12 final cancellation-contract tests pass, as does the 18-page documentation build. The subsequent test-only follow-up passes all 10 staged/deferred hook cases and seven docs/snapshot checks. It exercises deferred removal on every platform, verifies actual cleanup/notification/merge hook logs and checkout removal, and retains staged coverage on Unix. Tests run the actual worker to check refusal without hook dispatch, captured branch tips and targets, post-remove after checkout and branch deletion, post-remove despite ordinary branch failure, and approved hooks surviving config mutation. Native SIGTERM tests cover worker, public foreground, and public background removal; cancellation starts no new hook pipelines. Fallback fixtures occupy the transaction reservation or staging root to force failure before checkout movement on all platforms. Independent review of the combined source and test repairs found no remaining actionable issues.
Final-head CI: all Windows, macOS, Linux, advisory affected tests, lint, docs, features, and coverage-workflow jobs pass. Codecov project passes at 98.60%; mandatory patch coverage remains below target, 97.62% against 98.62%. The completed current-head review confirms the hook-ordering fix and no new actionable issue; it deliberately retains the existing maintainer deletion-safety hold without posting a duplicate review. No target merge has been performed.