fix: lifecycle-safety follow-ups (#2870)

[max-sixty]

Two follow-ups to #2870 surfaced by a retrospective deep-dive review. The
third item discussed in that review (the two divergent safe-delete semantics
in execute_instant_removal_or_fallback) is being explored as a separate
CAS-based rewrite via git update-ref -d <ref> <sha> and will come in a
follow-up PR — it touches delete_branch_if_safe core logic and warrants
its own review surface.

1. refactor(prune): drop defensive check_lock now that phases are sequenced

The check_lock RwLock<()> in step_prune was carried over from #2808's
Windows .git/config race fix. After #2870 restructured the flow so hook
approval is exact, the phase ordering already serializes integration-check
readers against the rename-failure fallback: the background par_iter is
the only reader, the channel-drain for ... in rx loop exits only once
that thread (and all its readers) has finished, and removals run strictly
after.

The remaining comment admitted the guard was defensive ("preserves the
serialization if this flow is refactored back to overlap checks and
removals") — one mechanism per guarantee per CLAUDE.md, so the lock is
removed. Comments on the par_iter spawn and the SynchronousForNonCurrent
variant now name the real invariants.

Verdict: Removed. Phase ordering is the actual mechanism; the lock was
belt-and-suspenders.

2. fix(remove): warn when background branch deletion silently retains the branch

Both branch-deletion paths in execute_instant_removal_or_fallback
(the fast-path post-prune delete and the SynchronousForNonCurrent
fallback delete) swallowed errors at log::debug! — invisible at default
verbosity. When the worktree was removed but the branch survived (a hook
advanced it, git branch -D errored, refs scan failed), the user got no
signal that the second half of the operation didn't happen.

Promoted to a warning_message with a wt remove --force-delete <branch>
recovery hint. To avoid duplicating the planner's existing "Branch
unmerged" hint, the warning suppresses the Ok(NotDeleted) case when
the planner already predicted retention; race-driven NotDeleted outcomes
still warn. Err always warns. BackgroundRemoval carries a new
planner_expected_retention bit so the helper can make that distinction
without re-deriving planner state.

Tradeoff: a previously silent path gains noise on the rare race. Silent
half-completion of a destructive operation is worse than an occasional
extra warning line.

Tests

cargo run -- hook pre-merge --yes passes: 3834 nextest tests + doctests

• pre-commit lints + clippy + docs.

This was written by Claude Code on behalf of Maximilian Roos

[worktrunk-bot]

Two doc comments still cite the old "serialize .git/config rewrite with integration-check readers" rationale that the rest of this PR replaced — the SynchronousForNonCurrent variant doc and the try_remove comment were updated, but two siblings in src/output/handlers.rs were missed:

• execute_instant_removal_or_fallback's doc, ending ...runs that fallback synchronously for non-current worktrees when the caller needs to keep the fallback's '.git/config' rewrite serialized with other git readers. — the prune caller no longer needs that serialization; the variant doc you updated now grounds the choice in iteration ordering / final summary.
• handle_remove_output's doc, ending prune passes [...SynchronousForNonCurrent] to keep the fallback's '.git/config' rewrite serialized with its parallel integration-check readers. — same stale claim.

Happy to push a follow-up commit aligning both with the new variant doc if you'd like.

codecov/patch is failing with 6 uncovered lines, all in the new warn_if_branch_retained — the Ok(NotDeleted) race-warning arm (lines 303–308) and the Err arm (lines 312–317). Per CLAUDE.md codecov/patch is mandatory despite being non-required, so flagging for your call: the uncovered branches are exactly the rare paths the PR description calls out as acknowledged tradeoffs, and they're not trivially testable without an injected hook / git failure. Up to you whether to add tests or merge with the gap.

Other observations, all non-blocking:

• The lock removal in step_prune looks correct — the for (idx, result) in rx loop only exits once tx is dropped (when the spawned thread returns), which happens after every par_iter worker has finished. Removals run strictly after, so checks and the fallback's .git/config rewrite can't overlap.
• planner_expected_retention = ctx.deletion_mode.should_keep() || safety.integration_reason.is_none() evaluates to true for ForceDelete + integration_reason.is_none(), even though the planner predicted force-deletion in that case. It's a latent inaccuracy that never fires in practice because delete_branch_if_safe(force_delete=true) never returns Ok(NotDeleted) — but the relationship is implicit. Tightening to ... || (!is_force() && integration_reason.is_none()) would make the formula correct independent of delete_branch_if_safe's shape; arguably not worth the extra clause.

[worktrunk-bot]

code-coverage failed on an unrelated test (working_tree_diff_stats_with_untracked_counts_untracked_and_preserves_real_index) — same flake addressed in #2874. The 3 required test (linux|macos|windows) legs all pass. Leaving the approval in place; rerun once #2874 lands, or merge regardless since the failure isn't from this PR's changes.

(Worth noting: this means codecov/patch wasn't computed for this commit. The Err-arm removal + new unit test should resolve the gap from the prior commit's review, but that can't be verified until code-coverage succeeds.)