[max-sixty]

Today wt merge (with removal) emits two or three separate ◎ Running … lines — one for post-remove + post-switch (already batched), then another for post-merge. Collapse them into one combined announce so a single wt command produces a single status line for its background hooks.

# before
◎ Running post-remove: user:cleanup; post-switch: user:notify
◎ Running post-merge: user:sync

# after
◎ Running post-remove: user:cleanup; post-switch: user:notify; post-merge: user:sync

Approach

HookAnnouncer (in src/commands/hooks.rs) registers pipelines from multiple phases and flushes once. It stores owned PendingPipeline data so registration sites at different points in the command's lifecycle can pass short-lived CommandContexts without lifetime gymnastics; flush() rebuilds contexts from owned data and delegates to the existing combined formatter.

A Drop impl flushes pending hooks on early-return errors. Without it, a later registration failure would silently swallow earlier-registered pipelines — a regression from the prior fire-and-forget pattern. On the success path, explicit flush() runs first so Drop sees empty pending and is a no-op.

Wiring

• handle_remove_output gains an announcer: Option<&mut HookAnnouncer<'_>> parameter, threaded through the internal handlers (handle_named_removed_worktree_*, handle_detached_removed_worktree_output, spawn_hooks_after_remove). When Some, pipelines register on the announcer instead of self-spawning.
• wt merge constructs one announcer, passes it through the remove block, registers post-merge after, calls flush() once before json output.
• Standalone wt remove, prune, and the picker remove path pass None — output unchanged.
• wt switch --create (already one-line via announce_and_spawn_background_hooks) is unchanged. Migrating it to HookAnnouncer for consistency is a follow-up.

Testing

• test_merge_combines_post_remove_post_switch_post_merge snapshots the combined announce line.
• test_merge_drops_pending_hooks_when_post_merge_fails covers the Drop fallback: post-merge template prep errors after post-remove + post-switch register, and the hooks still fire via Drop.

codecov/patch reports 96.40% vs 96.73% target. The five missed lines are the inner eprintln! arm of the Drop fallback (fires only when flush() itself errors during Drop — requires spawn_hook_pipeline_quiet to fail) and a few trailing )?; lines that llvm-cov misattributes. Defensive logging path, not meaningfully testable; merging with the gap.

[worktrunk-bot]

codecov/patch failed

The patch fails coverage at 91.89% on src/commands/hooks.rs — the Drop impl's error-recovery path is uncovered:

L377-383: } if let Err(err) = self.flush() { eprintln!(warning_message(...))

This is reachable when an earlier phase registers pipelines on the announcer and a later phase errors before flush() runs (e.g., post-remove succeeds, then announcer.register(HookType::PostMerge, …) fails because of a malformed template, dropping the announcer with non-empty pending). It's the path the docstring on Drop calls out as "preserves prior fire-and-forget behavior on early-return errors" — codecov correctly flags that there's no test for it.

A test could mirror the new test_merge_combines_post_remove_post_switch_post_merge but with a post-merge config that fails template rendering (e.g. an undefined variable like {{ does_not_exist }}). The post-remove pipelines would be queued, post-merge prep would error, and the Drop path would fire and emit the warning.

A smaller secondary gap: extend() line 312 (continue for empty steps) is unreachable because all callers source from prepare_background_hooks, which already filters out empty groups. Could be dropped, since announce_and_spawn_background_hooks also filters defensively before calling announce_and_spawn_pipelines.

codecov/patch failed — Drop error-recovery path uncovered (see follow-up review)

[worktrunk-bot]

CI is failing on Windows on the new test. I missed this when approving — apologies.

test_merge_drops_pending_hooks_when_post_merge_fails writes the marker paths into TOML basic strings (double-quoted). On Windows the temp path looks like D:\tmp\.tmphrqBat\drop_postremove_marker.txt; in a TOML basic string \t becomes a tab and \. is an invalid escape, so config load fails before HookAnnouncer is even constructed. The merge exits non-success (which the test still happens to assert) but neither hook ever registers, so the post-remove marker stays empty and wait_for_file_content times out at 60s. Linux/macOS pass because their paths use forward slashes.

Fix: switch the two embedded-path lines to TOML literal strings (single-quoted), which don't interpret escapes. The format-level {{{{ does_not_exist }}}} still resolves to a literal {{ does_not_exist }} in the rendered TOML, which is what we want for the template-prep failure.

Happy to push the fix if you'd like.

CI failed on Windows — see follow-up review for analysis and inline fix suggestions

[worktrunk-bot]

CI is still red on Windows on the same test, despite the literal-string fix. Apologies — I missed the second half of the path-handling problem when I approved.

The literal-string change made the TOML parse clean, but the rendered shell command still embeds the raw Windows path with backslashes (D:\tmp\.tmpXXXX\drop_postremove_marker.txt). On Windows wt shells via Git Bash (Cmd::shell per src/shell_exec.rs:687-693), and in unquoted bash \. and \t are eaten as escape-of-next-char, mangling the path. So echo writes to a different filename and wait_for_file_content times out at 60s on the path the test is watching.

The project convention for embedding paths into shell commands in tests is path_slash::PathExt::to_slash_lossy() (and CLAUDE.md mandates it under "Use Existing Dependencies"). The pattern is already used in tests/integration_tests/remove.rs (e.g. marker_file.to_slash_lossy() at line 1603). Two .display() calls need swapping; you'll also need a use path_slash::PathExt as _;.

Suggestions inline. Happy to push the fix if you'd like.

(Codecov is also flagging 96.40% vs 96.73% target — five missed lines that are mostly trailing )?; lines plus the eprintln! body inside the Drop fallback. The Drop path itself is now exercised; the remaining miss is just the inner format! call when flush() itself errors. Probably not worth chasing.)

CI failed on Windows — path with backslashes still mangled by Git Bash; see follow-up review

codecov/patch failing — see follow-up comment

[worktrunk-bot]

CI is green across Linux/macOS/Windows — the to_slash_lossy() fix worked.

codecov/patch is still red at 96.40% vs 96.73% target. Same five misses I flagged on the previous commit: the HookAnnouncer::new/extend/register/flush body lines plus the eprintln! arm inside Drop (when flush() itself errors). The Drop path is now exercised by test_merge_drops_pending_hooks_when_post_merge_fails; the inner-error miss is what remains. Per CLAUDE.md the failure needs your sign-off before merging — flagging again so the merge decision is explicit.