fix(worktree): prevent zombie worktree and map leak on partial cleanup failure

[IBondarenko-iwg]

Linked issue

Closes #537

Summary of changes

Four bugs in Dotbot.Worktree.psm1 that cause orphan worktree partial setup to corrupt the filesystem on retry:

Bug A — Remove-OrphanWorktrees deleted map entry even when the worktree directory survived removal. Next startup: dir not in map → invisible to retry machinery → permanent zombie.

Bug B — No Remove-Item fallback after git worktree remove --force in Remove-OrphanWorktrees. On Windows, open file handles cause silent failure. Added fallback + git worktree prune. Map entry now kept only when dir actually survives both attempts.

Bug C — New-TaskWorktree called Remove-Item -ErrorAction SilentlyContinue on stale leftover dir but never checked the result. Fell through to git worktree add, which failed with a path-collision error. Added early return with success=$false when removal fails.

Bug D — Complete-TaskWorktree had the same pattern as Bug A: removed map entry even when dir survived. Added Remove-Item fallback + conditional map removal. Also fixed misleading return message ("cleaned up" now only emitted when dir is actually gone) and corrected a false comment claiming Remove-OrphanWorktrees would retry done-status tasks on next startup (it doesn't — done is in $activeIds).

Assert-PathWithinBounds calls at new fallback sites use the canonical worktree parent dir derived from $ProjectRoot, matching the pre-existing pattern in New-TaskWorktree.

Testing notes

1. Create a task worktree. Lock a file inside it to prevent git worktree remove --force from succeeding.
2. Trigger Remove-OrphanWorktrees (restart the bot process). Verify:
   • Log emits Error level: "Orphan worktree removal incomplete — path still exists"
   • Map entry is retained (not deleted)
3. Release the file handle. Restart again. Verify:
   • Dir removed, map entry purged, branch deleted
   • No "Worktree already exists" or path-collision errors on next task pick-up
4. Run existing dispatch regression tests (Test-ProcessDispatch.ps1) — must pass clean.

Checklist

• Linked issue exists
• Follows the contribution guide

[carlospedreira]

@IBondarenko-iwg this cleanup path still feels too path-driven.

First, the fallback reconstructs the current dotbot worktree layout as <repo-parent>/worktrees/<repo-name>. That makes the cleanup guard fragile: if dotbot changes where it creates worktrees later, this check can drift from the real ownership model.

Second, I do not think Remove-Item -Recurse -Force should be the first recovery mechanism here. Git already tracks registered worktrees and their branches. Before removing anything directly, dotbot can inspect git worktree list --porcelain and verify that the mapped worktree_path is a registered worktree whose branch is exactly refs/heads/$branchName. If that matches, Git has proved this is the expected task worktree, even if it lives outside the current default folder layout, and git worktree remove --force <path> should be the cleanup operation.

If Git cannot remove it, I am not sure whether dotbot should stop and ask for manual cleanup or use a narrow filesystem fallback. Either way, ownership should come from Git, not from the raw filesystem path. Git should confirm this is the registered worktree for the expected task branch before dotbot does any fallback cleanup.

[IBondarenko-iwg]

@carlospedreira Thanks for the push on this. Turns out there were a couple of issues.

On the path formula: it looks hardcoded but it's the same derivation New-TaskWorktree uses to create the worktree in the first place (lines 1386–1389). It's not a second copy of the layout that can drift independently — if we ever move where worktrees live, New-TaskWorktree changes and this follows it. And it's not a new pattern either; the other four Assert-PathWithinBounds callsites in the module all derive ownership the same way, so these two are consistent with everything around them.

On git worktree remove --force — it's already there, right before the fallback. But it only fires when junctions are clean (1889/2209). When Remove-Junctions fails we intentionally drop to non-force removal (1896/2216), because forcing a remove while junctions are still live can take out shared linked content with it. So, it's a deliberate gate, not a missing step.

You're right the fallback was too blunt, though. Remove-Item -Recurse -Force ran unconditionally when the dir survived — and on Windows recurse follows junctions, so it could've deleted the link targets, not just the worktree. Both sites are now gated on the same junctions-clean check as --force: if junctions survive, skip the delete, keep the entry for retry.

In Remove-OrphanWorktrees the path-formula Assert could throw on a non-canonical/legacy map entry — and because that loop had no try/catch, a single bad entry would abort the entire orphan sweep and skip the map prune. I've wrapped the per-entry removal in try/catch (~2218): a bad entry now degrades to a logged skip instead of bricking cleanup.

On git worktree list --porcelain as the ownership source — I think it's a fair model, but right now all six Assert-PathWithinBounds callsites use path-formula ownership, and switching just these two over would leave us with two competing models in one module. If we want registry-based ownership to be the canonical thing, I'd rather make that its own PR that converts all six together.
Happy to open it as a follow-up.

[carlospedreira]

Looks good to me now. The concrete cleanup risk is addressed, and the remaining ownership-model idea can be follow-up work.