[max-sixty]

Summary

Switch and merge sites previously each rebuilt Vec<(&str, &str)> of hook template variables ad hoc, with subtly different rules and bespoke borrow gymnastics (notably pr_number_buf: &mut String). The deprecated worktree alias for worktree_path was duplicated at three sites.

This PR introduces commands::template_vars::TemplateVars as the canonical builder and routes all 4 switch sites + 2 merge sites + the picker through it. The struct owns its strings, encodes the deprecated worktree alias once in as_extra_vars(), and offers a for_post_switch constructor that reads directly off SwitchResult / SwitchBranchInfo.

Behavior change

Post-switch hooks fired from the picker now receive target / target_worktree_path like hooks fired from wt switch foo — closing a pre-existing asymmetry where interactive switching exposed strictly fewer template variables than the non-interactive path. There's no design reason interactive switching should hand hooks less context; this looks like an oversight in the original picker integration.

Where to start reviewing

• src/commands/template_vars.rs — the new module (~170 lines of production code, 11 unit tests).
• src/commands/handle_switch.rs — 3 of the migrated sites; biggest reduction (-73 net lines). The switch_extra_vars helper with its pr_number_buf borrow hack is gone.
• src/commands/merge.rs — the post-merge feature_vars capture-before-removal pattern, which was the strongest motivator for owning strings rather than borrowing.
• src/commands/picker/mod.rs — small but the only behavior-changing site.

Out of scope

hook_commands.rs::build_manual_hook_extra_vars is being heavily refactored on hook-dispatch-unify; that branch will land first. After both merge, applying TemplateVars there is a trivial follow-up.

This was written by Claude Code on behalf of @max-sixty

[worktrunk-bot]

Refactor cleanly consolidates the switch/merge template-var assembly. The owned-strings approach removes the pr_number_buf borrow gymnastics and the for_post_switch constructor handles all three SwitchResult variants in one place. Tests cover the directional-pair / Active-override / pr / detached cases. The picker behavior change (now emits target / target_worktree_path) is the right direction and is called out in the PR body.

A few observations:

• src/commands/worktree/switch.rs:913 still references switch_extra_vars ("…SwitchResult — switch_extra_vars then forwards it…"), but that function is removed by this PR. The comment lives just outside the diff hunks; happy to push a one-line follow-up if useful, otherwise it's a trivial inline edit.

• Subtle behavior shift in merge.rs: when env.worktree_path.file_name() is None or non-UTF-8, the old code emitted worktree_name = "unknown" (unwrap_or("unknown")); with_active_worktree now omits the variable instead. In practice env.worktree_path always has a filename, so this is a corner case — but it means a {{ worktree_name }} template would shift from rendering "unknown" to failing as undefined. New behavior is arguably more honest; flagging since it isn't called out in the description.

• PostRemoveContext (src/commands/worktree/hooks.rs:42-106) is a sibling implementation that overlaps meaningfully with TemplateVars (worktree_path + deprecated worktree alias, worktree_name with the unwrap_or("unknown") fallback, commit/short_commit). The new module's docstring positions itself as canonical and the PR's "out of scope" calls out hook_commands.rs but not this one — worth noting whether folding PostRemoveContext into TemplateVars is on the same follow-up runway.

• The lint job is failing on cargo fmt for the post-merge announcer.register call (suggestion below).

[max-sixty]

Thanks for the review. Addressed in the follow-up commit:

• rustfmt: Applied — lint should pass on next run.
• Stale switch_extra_vars comment in worktree/switch.rs:913: Updated to reference TemplateVars::for_post_switch.
• worktree_name fallback: Restored the unwrap_or("unknown") fallback inside with_active_worktree and added a regression test (active_worktree_name_falls_back_to_unknown). You're right that the new behavior was arguably more honest, but parity is the safer call here — env.worktree_path always has a filename in practice, so this is purely defensive.
• PostRemoveContext overlap: Good catch and intentional follow-up. PostRemoveContext (src/commands/worktree/hooks.rs:42-106) covers the same Active-override surface and would fold cleanly into TemplateVars once the hook_commands.rs / hooks.rs refactor on [hook-dispatch-unify] lands. Not in this PR's scope, but worth flagging in the issue tracker.

This was written by Claude Code on behalf of @max-sixty