✨ feat(tui): run the removal lifecycle behind the d key

[kbrdn1]

Description

d in the TUI called worktree::remove and nothing else, so it ran no
[hooks.pre_remove] / [hooks.post_remove] and wrote no undo-journal entry:
an interactive delete was unrecoverable, and a hook written to guard a removal
only held for whoever used the CLI. #484 made that bite harder, since Space

• d can now take out ten worktrees on one keystroke.

The sequence moves out of cli::remove_one into removal::remove_with_lifecycle,
which prints nothing and hands back what each phase produced. The CLI becomes
its renderer; the TUI worker calls it directly. One sequence, two callers.

Closes #521

Type of change

• ✨ Feature (new functionality)
• ♻️ Refactor (no behaviour change)

The issue calls itself a refactor. It is not: hooks now run and the journal is
now written on a path where neither happened. Both boxes are ticked because the
extraction underneath genuinely is one.

Changes

• removal::remove_with_lifecycle owns the sequence: pre_remove hooks,
  destruction, undo-journal entry, post_remove hooks. No printing.
• lifecycle::run_phase_quiet is the phase runner without the abort-time
  print_report, returning the partial report next to the error. run_phase
  is now a wrapper that prints it, so the twelve existing call sites keep their
  output to the character.
• lifecycle::has_steps and trust::evaluate_silent: the phase-level question
  the trust gate should ask, and the no-prompt decision the TUI already
  open-coded for bootstrap, now shared.
• The TUI worker resolves config, worktree list and trust decision once per
  repo a batch spans rather than once per target.
• DeleteBatchOutcome gains warnings for what fails around a removal that
  happened anyway. Those ride the status line and stay out of
  failure_banner, which is what holds the confirm overlay open.
• Docs (EN + FR): hooks page, TUI keybindings, gwm undo reference.

Behaviour changes worth calling out

d can now refuse where it always deleted. Running a hook executes code
out of .gwm.toml, so a repo whose config defines remove hooks goes through
the TOFU ledger (#95) first, and an unapproved one refuses rather than silently
skipping the guard hook: the alternate screen has nowhere to host the prompt.
The gate asks about the two remove phases only, so a config whose hooks are all
post_create executes nothing on a delete and is never asked. Without that
narrowing this would refuse deletes that have always worked.

A pre_remove that refuses now refuses in the TUI, for that one target;
the batch carries on and the confirm overlay stays open on what failed, which
is the loop cmd_remove already ran. There is no --skip-hooks in the TUI; to
delete past a hook, use the CLI with --force.

On the CLI, the journal entry moved after the destructive call. It used to
be written before, so a target that then got refused still showed up in
gwm history as something gwm undo would replay. The branch OID is still
captured beforehand, which is what #29 needed. Pinned by
a_batch_refuses_a_target_whose_path_moved_since_it_was_resolved, which I
probed red by putting the write back where it was.

undo stays per-worktree. A batch of ten appends ten entries and each
gwm undo pops one. Giving the journal a notion of a batch is a separate
feature; gwm remove a b c has behaved this way since #484 and the TUI
inherits it rather than creating it.

Tests

• cargo test passes locally
• cargo fmt --check passes
• cargo clippy -- -D warnings passes
• New tests added under tests/

Seven new tests in tests/removal_tests.rs, four of them red before the
implementation (journal entry, both hooks running, a refused pre_remove
stopping everything downstream, the trust gate), plus:

• gwm_undo_restores_what_the_tui_deleted: the round trip the acceptance
  criterion actually asks for. Asserting the entry's fields is not it, since
  undo filters on repo_root verbatim, so an entry canonicalised differently
  than undo resolves it passes every field assertion and is still invisible.
• the_batch_removes_the_path_it_confirmed_not_whatever_the_id_now_names:
  green before and after, as a net. [Task]: the TUI delete skips the undo journal and the remove hooks #521 makes the worker re-resolve its target
  to name the branch in the journal entry, and deriving the expected path from
  that fresh resolution would compare live state with itself, quietly undoing
  the remove_verified guard from PR ✨ feat(tui,cli): multi-row selection and bulk delete (#484) #520. That is why expected_path is a
  separate argument rather than found.path.
• a_failing_post_remove_hook_is_not_reported_as_a_failed_removal: the TUI
  twin of the CLI test from the ✨ feat(tui,cli): multi-row selection and bulk delete (#484) #520 review.
• a_warning_rides_the_status_line_without_becoming_a_failure in
  tui_app_tests.rs.

Two deviations from the acceptance criteria, both deliberate:

1. The AC asks for tests "driven through the App state machine rather than the
   CLI binary". The journal claims cannot hold to that: they rewrite
   GWM_HISTORY_FILE, and the [Bug]: the $HOME guard in config_tests stops at expand_placeholders, and the other test binaries have the same shape #507 env guard derives who must serialise from a
   transitive walk of src/, so a set_var in tui_app_tests.rs would demand
   the lock from every delete test in that binary. The contract tests therefore
   live in their own binary (still driving the App), and the round trip mixes in
   the real binary because nothing else can answer that question.
2. No new test drives delete_branch = true through the TUI. The flag reaches
   the same shared sequence the CLI's remove_with_delete_branch_drops_branch
   already covers, so it is covered by construction rather than by a
   near-duplicate test.

Screenshots / TUI captures

None: nothing changes on screen except the status line, which is asserted in
a_warning_rides_the_status_line_without_becoming_a_failure.

Checklist

• Branch follows <type>/#<issue>-<description>
• Commits follow Gitmoji + Conventional Commits
• CHANGELOG.md updated under ## [Unreleased]
• README updated if CLI surface changed: no CLI surface change
• examples/gwm.toml.example updated if config schema changed: no schema change
• No unwrap() on user-facing paths
• No println! in TUI render code, which is why run_phase_quiet exists:
  run_phase printed its report on an aborting hook, and a worker thread
  would have painted that over the alternate screen. Hook stdout and stderr
  were already captured by command_log::run_logged, so they reach the
  Command Logs modal (3) instead.

Review findings left open

The Codex review loop stopped on convergence, not at zero: the last measured
P0P1P2_COUNT was 4. One of those four was fixed after that measurement and
the count was not taken again, so here is the disposition of all four, by name.

• [P2] Tell a detached HEAD apart from an unreadable one: fixed in
  e8ab738, with a_detached_head_is_recorded_as_no_branch covering it.
• [P1] Load, hash and merge one snapshot of the config bytes: follow-up
  Close the remaining TOCTOU windows in the removal sequence #531. Narrowed here (the two loads are adjacent, with worktree::list no
  longer between them); closing it means trust::evaluate accepting bytes the
  caller already read, which changes the trust surface shared by create,
  bootstrap and review.
• [P1] Drop the provisional journal entry when the removal is refused:
  follow-up Close the remaining TOCTOU windows in the removal sequence #531. Needs a concurrent writer between two adjacent
  statements; the entry that results fails gwm undo visibly rather than
  silently, and the obvious remedy adds a journal write on a failure path.
• [P1] Log the branch the removal actually deletes: follow-up Close the remaining TOCTOU windows in the removal sequence #531.
  Same window shape, and only with --delete-branch; closing it means
  worktree::remove taking the branch as an argument, which changes its
  signature for every caller.

The three deferred ones are one family (repeated observations of the same
state across the sequence) and #531 treats them together, since fixing them
piecemeal is what re-opened a window inside this very PR. None is reachable
without a concurrent writer hitting a gap between two adjacent statements,
which is why none blocks this merge. No test covers them: nobody writes a test
for a finding they chose not to fix, so the green checks say nothing about
them.

Linked issues / docs

• Issue: [Task]: the TUI delete skips the undo journal and the remove hooks #521
• Found while implementing [Feature]: space as toggle the active row as to be "selected" e.g. for bulk cleanup #484 (PR ✨ feat(tui,cli): multi-row selection and bulk delete (#484) #520) and kept out of it on purpose
• Follow-up: Close the remaining TOCTOU windows in the removal sequence #531 (three review findings deferred, see above)

Notes for reviewers

The trust gate is the piece worth a second opinion: it is the only place this
PR makes d do less than it did. RepoBatch::open takes the App's
global_path rather than re-resolving global_config_path(), otherwise the
hooks a delete runs would come from a different layering than the config panel
shows, and the tests that inject None (#194) would have read the runner's
real ~/.config/gwm/config.toml while CI, which sets GWM_NO_GLOBAL_CONFIG=1
workflow-wide, read nothing.

[coderabbitai]

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: ddca1082-be88-4898-ae67-baf3bbdab8c8

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

• 🔍 Trigger review

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[kbrdn1]

Codex review loop: 3 passes, stopped on convergence

pass	findings	P0	P1	P2	what they were about
1	4	0	1	3	the diff: a hook running against an unconfirmed path, the undo anchor lost on a partial failure, on_fail = "warn" silent in the TUI, the trust gate asking the merged config
2	4	0	3	1	3 of 4 on pass 1's own lines: the double id lookup I introduced, the journal naming a branch the removal was not deleting, the config read twice around worktree::list
3	4	0	3	1	4 of 4 on pass 1 and 2's lines: three microsecond-wide inter-process races, plus one real defect in the guard written at pass 2

Nine findings fixed across 2a944da, 382c535 and e8ab738, each with a test
that goes red without its fix, verified by reverting the production change
rather than by reading the test.

Where it stopped being about the diff

Pass 2. From there on, every finding pointed at a line written to fix an
earlier pass. Two of pass 2's were genuine regressions of mine and worth every
minute:

• I reopened the window ✨ feat(tui,cli): multi-row selection and bulk delete (#484) #520 closed. Extracting verify_path and calling
  it from remove_inner meant the worktree id was resolved twice, so another
  process recreating it between the two lookups would have the first handle
  validated and the second one destroyed. The doc comment explaining exactly
  why that check must sit on the prune's own handle was one I deleted in the
  same edit. Both are back.
• The journal named a branch the removal was not deleting. A batch
  resolves its listing once, so a pre_remove on an earlier target can check
  out another branch in a later one without moving it. worktree::remove
  deletes what it reads off HEAD; the entry recorded the stale listing. gwm undo would have restored a ref that was never deleted while the deleted one
  stayed lost.

Pass 3 was not a dry pass either: it found a real, deterministic defect in the
fallback I had written one pass earlier (detached HEAD collapsing into "could
not open the worktree", so the entry named a branch the removal deleted nothing
of). That is fixed too. But it was a defect in my own guard, not in the change
this PR is about, which is what the stop is about.

The gravity contrast

• Pass 1's worst: a pre_remove hook executes against a directory the user
  never confirmed. Reachable with one git worktree move from another
  terminal, and the hook is arbitrary code from .gwm.toml.
• Pass 3's worst: a concurrent process moves the target between two
  consecutive statements, leaving an orphan journal entry that gwm undo
  would fail on visibly, since the worktree is still there.

The three findings left on the table

1. [P1] Load, hash and merge one single snapshot of the config bytes.
   Narrowed at pass 2 (the two reads are now back to back, with the expensive
   worktree::list moved out from between them) but not closed. Closing it
   means having trust::evaluate accept bytes already read instead of opening
   the file itself, which changes the trust surface for create, bootstrap
   and review as well, not just this path. And the threat model does not
   support it: an attacker who can rewrite .gwm.toml at a chosen instant
   already has local write access, against which TOFU was never the defence.
   It defends against a hostile .gwm.toml arriving by clone or fetch.
2. [P1] Undo the provisional journal entry when the removal is refused.
   The window is between two consecutive statements, and the remedy adds a
   journal write on a failure path, which is a new way to fail (what happens
   when the compensating write fails?). The current outcome is an orphan entry
   whose gwm undo fails visibly because the worktree is still on disk, and
   undo only consumes an entry on success, so nothing is silently lost.
3. [P1] Freeze the logged branch with the one actually deleted. Same shape:
   a concurrent checkout between two consecutive statements, and only with
   --delete-branch. The remedy changes worktree::remove's signature for
   every caller to close a window narrower than the one that existed before
   this PR.

One invariant, three check sites

There are now three path checks, and they are not accumulated paranoia. The
rule is one sentence:

Nothing executes, records, or destroys against a path the user did not
confirm.

Checked before anything runs (so a hook cannot act on a moved worktree),
again after the hooks (so the journal cannot claim a removal the hooks just
invalidated), and on the handle the prune acts on (so the destruction and
the verification cannot be about two different worktrees). Delete any one of
them and a specific scenario above comes back.

What is left to validate outside review

None of this has run against a real terminal, which is why the next useful
information is not a fourth pass. Four things only a human at the TUI can
answer:

• d on a single row, the everyday path;
• Space + d on a batch;
• a repo whose .gwm.toml has a pre_remove that refuses, and one that is not
  in the trust ledger;
• gwm undo after a TUI delete, which is the whole point of the issue.