Leave the branch template variable unset in a detached worktree

[worktrunk-bot]

Problem

In a detached worktree, CommandContext::branch_or_head substituted the literal HEAD for {{ branch }}. HEAD is a non-empty string git happily resolves as a ref, so every guard written around branch passed and the command ran against the wrong thing — the reported case ended in git push origin --delete HEAD. It also disagreed with wt list --format=json, which reports branch: null for the same worktree, and with the hook docs, which say undefined variables error so a template can guard them.

Solution

branch is now absent in a detached worktree rather than falling back. {% if branch %} guards it the way the docs already prescribe for upstream; an unguarded {{ branch }} is an undefined-variable error naming the template and listing the variables that are in scope. This is the first of the two options in #4009 — the one the reporter picked.

The removal path needed the same treatment on its own: spawn_hooks_after_remove took the removed branch as &str and both detached call sites passed the literal HEAD, which PostRemoveContext::extra_vars then applied after the base context — so post-remove still saw branch = HEAD even with the base fix in place. It takes Option<&str> now. That is the hook where the issue's own command belongs, so it's covered by its own regression test (test_user_post_remove_branch_unset_in_detached_worktree, which reproduces branch=[HEAD] without the change).

Riding along:

• The base and target branch names derived from the current branch follow branch and stay unset too — both for a manual wt hook <type> and for the pre-switch hooks wt switch fires, which previously rendered base as an empty string. The directional path vars (base_worktree_path, target_worktree_path) still apply — the worktree exists whether or not it is on a branch.
• vars is now inserted as the empty map when branch is absent, not skipped. Per-branch vars are keyed by branch, so a detached worktree has none either way (wt config state vars set can't even write there — it goes through require_current_branch), but keeping the object defined means {{ vars.key | default('x') }} still renders under SemiStrict instead of erroring on an undefined vars. Same reason list::custom_columns injects an empty map for a branchless row.
• The verbose variables table rendered a detached branch as (unused) under VarScope::Referenced — the label that means "the scope gate saved us the work", which is wrong for a cheap var computed regardless. ALWAYS_COMPUTED_VARS in expansion.rs marks it (unset), the label for a var the operation genuinely couldn't supply.
• The hook long-help sentence about unset base / target scoped them to a manual wt hook; both are now operation-driven cases too (base in a pre-switch from a detached worktree, target in a removal that lands in one), so it's worded generically and the three doc mirrors are regenerated.
• The hook long-help JSON-context example read ctx['branch'] unguarded — the same unguarded case in Python, which now raises KeyError. It uses ctx.get('branch', '') and the section says why.
• target — the branch the user lands on after a removal — followed the same rule on review: PostRemoveContext::new built it with unwrap_or_default(), so a detached primary worktree handed post-remove an empty string and made wt -v print target = instead of target = (unset). It's Option<String> now, pushed only when there is one. commit / short_commit keep their "" shape — they predate this and PostRemoveContext::new documents the choice. The pre-remove half of the same removal built its extra vars by hand and had the same unwrap_or_default(); it goes through TemplateVars::with_target_opt + as_extra_vars now — the builder wt hook pre-remove already used — so both hooks of one removal agree.
• The HEAD literal survives in exactly one place: the background pipeline's log file name (spawn_hook_pipeline_quiet), which needs some string and never reaches a template.

branch_or_head had no callers left, so it's deleted. TemplateVars::with_base takes Option<&str> and a new with_target_opt sits beside with_target, so the optional-branch call sites keep the POSIX path conversion in the builder rather than re-inlining it.

Behavior change worth a look

This is deliberate — it's what the issue asks for — but it turns previously-working shapes into errors, each pinned by a test that this PR updates rather than deletes:

• wt step for-each -- echo '{{ branch }}' now fails at the detached worktree instead of printing HEAD, and (as with any undefined variable) the loop stops there. Not a new failure mode: {{ upstream }} already behaved this way at the first non-tracking worktree. test_for_each_detached_branch_variable_unguarded snapshots it.
• A project pre-remove or post-remove hook that references {{ branch }} unguarded now blocks wt remove on a detached worktree until it's guarded (or --no-hooks). test_pre_remove_hook_branch_expansion_detached_head previously asserted branch=HEAD; it now uses the guarded form and asserts branch=.
• A pre-switch hook run from a detached worktree gets no base rather than an empty one. An unguarded {{ base }} errors where it used to render nothing.

If any of these should instead render an empty string rather than error, that's a different fix — say the word and I'll redo it that way.

Testing

test_alias_branch_unset_in_detached_worktree (tests/integration_tests/step_alias.rs) and test_user_post_remove_branch_unset_in_detached_worktree (tests/integration_tests/user_hooks.rs) are the reproductions: both fail with HEAD before their respective fixes and pass now. test_user_remove_hooks_target_unset_with_detached_primary_worktree (both pre-remove and post-remove of one removal) and test_user_pre_switch_base_unset_in_detached_worktree pin the target and base cases; both use {% if x is defined %} rather than {% if x %}, since an empty string and an absent var are indistinguishable under the plain guard, and each half fails on its pre-fix shape.

cargo run -- hook pre-merge --yes is green apart from three failures that reproduce with these changes stashed, on this branch, in the same sandbox — test_copy_ignored_preserves_file_executable_permissions (expects 0644, gets 0664 under the runner's umask 002) plus test_powershell_skipped_when_installed_no_profile and test_nushell_install_target_is_a_vendor_autoload_dir (the sandbox home's shell state). Clippy --all-targets --all-features and cargo fmt --check are clean.

Manual check against the issue's repro

$ git worktree add --detach ../repo.scratch HEAD && cd ../repo.scratch

$ wt -v guarded          # probe = 'echo "[{% if branch %}{{ branch }}{% endif %}]"'
○ guarded template variables:
  branch                = (unset)
  worktree_path         = /tmp/…/repo.scratch
…
[]

$ wt probe               # probe = 'echo "[{{ branch }}]"'
✗ Failed to expand probe: undefined value @ line 1
  echo "[{{ branch }}]"
↳ Available variables: args, cwd, main_worktree, repo, repo_path, repo_root, worktree, worktree_name, worktree_path

$ cd ../repo && wt probe # unchanged on a branch
[main]

---

Closes #4009 — automated triage

[worktrunk-bot]

The core change reads right: ctx.branch was already Option, so dropping branch_or_head and letting build_hook_context omit the key puts the fix at the level the data already had, and {% if branch %} matches how upstream and target_worktree_path already behave. The vars-as-empty-map decision is the correct call and the list::custom_columns precedent it cites is real. One gap worth fixing before this lands, plus some stale prose.

post-remove hooks still see branch = HEAD

Two spawn_hooks_after_remove call sites hand it the literal "HEAD" rather than an Option:

// src/output/handlers.rs:1841-1842
// Post-remove hooks for detached HEAD use "HEAD" as the branch identifier
spawn_hooks_after_remove(repo, ctx, "HEAD", announcer)?;

// src/output/handlers.rs:2030
spawn_hooks_after_remove(repo, ctx, ctx.branch_name.unwrap_or("HEAD"), announcer)?;

That string reaches the template context twice — PostRemoveContext::extra_vars pushes ("branch", removed_branch), and spawn_hooks_after_remove builds CommandContext::new(repo, &config, Some(removed_branch), ctx.main_path, false) — and build_hook_context applies extra_vars unconditionally at the end, so the new if let Some(branch) = ctx.branch guard is overwritten anyway.

I confirmed it by running a probe against this branch: a project post-remove = "echo 'branch=[{{ branch }}]' > …" fired on wt remove --foreground --force --yes <detached-path> wrote branch=[HEAD].

This is the same failure the PR fixes, on the hook where it matters most: post-remove is the natural home for git push origin --delete {{ branch }}, which is #4009's reported command. test_pre_remove_hook_branch_expansion_detached_head covers pre-remove only, and test_manual_hook_branch_vars_unset_in_detached_worktree (just pushed) covers the manual wt hook path, so nothing in the suite reaches the removal-driven post-remove context. Threading Option<&str> through spawn_hooks_after_remove / PostRemoveContext::extra_vars fits the existing shape — spawn_hook_pipeline_quiet already owns the "HEAD" log-file fallback, so the log path keeps its name for free.

It also makes the PR description's "the HEAD literal survives in exactly one place" inaccurate as written; worth correcting either way.

Stale comments the change falsifies

Two are in a file this PR edits:

// src/commands/hook_commands.rs:188
/// Works in detached HEAD state - `{{ branch }}` template variable will be "HEAD".

// src/commands/hook_commands.rs:379
// fine: `{{ branch }}` expands to "HEAD" there.

And one more in the for-each path, which the PR's own snapshot now contradicts:

// src/commands/for_each.rs:78
// Pass wt.branch directly (not the display string) so detached HEAD maps to None -> "HEAD"

In expansion.rs the block comment above the injection still describes the old condition, while the new comment inside says the opposite:

// src/config/expansion.rs:1188
// {{ vars.config.port }}. When branch is present, always inject (even if
// empty map) so {{ vars.key | default(...) }} works in SemiStrict mode.

The documented JSON-context example breaks in the same scenario

# src/cli/mod.rs:1818 (primary; mirrored in docs/src/content/docs/hook.md and both reference dirs)
if ctx['branch'].startswith('feature/') and 'backend' in ctx['repo']:

Hooks get the same map as JSON on stdin, so this is the {{ branch }} unguarded case in Python: it used to read "HEAD" and now raises KeyError. The new prose adds the {% if branch %} guard for templates but leaves the stdin example unguarded — ctx.get('branch', '') would keep the section consistent with the guidance two sections above it.

Smaller notes

(unused) where (unset) is meant. format_variables_table's docstring (src/config/expansion.rs:310) says cheap vars "are populated unconditionally and always show their value" and that (unused) "fires precisely when the gate saved real work". branch is a cheap var that can now be absent, so under VarScope::Referenced an alias body that doesn't mention it renders branch = (unused) in a detached worktree — reading as "we skipped computing it" rather than "there isn't one". Verified against a build of this branch: wt -v <alias-not-referencing-branch> prints branch = (unused), while the referencing alias correctly prints (unset). Either the docstring's invariant needs amending or format_variables_table needs to treat branch like the other genuinely-optional vars.

build_manual_hook_template_vars re-inlines with_base. The switch arm now hand-rolls the POSIX conversion with_base was doing, purely to route around its &str parameter:

let base_path = to_posix_path(&worktree_path.to_string_lossy());
with_target(TemplateVars::new().with_base_strs(branch, Some(&base_path)))

A with_base_opt(Option<&str>, &Path) alongside a with_target_opt(Option<&str>) on TemplateVars would keep the conversion in one place and drop the local with_target closure, which reads oddly next to the builder chain it wraps. Not blocking.

What looks right

• with_base_strs(branch, Some(&base_path)) is behaviourally identical to the old with_base when branch is Some, and the to_posix_path call count on Windows is unchanged.
• No consumer other than expansion.rs:1212 reads "branch" out of the context map, and branch_or_head has no remaining callers.
• The worktree-path, list custom-column, and commit-prompt templates build their own var maps and are untouched.
• No --help snapshot covers the hook long-help block, so the three doc mirrors are the whole sync surface.

[worktrunk-bot]

Addressed in cafcd5d.

post-remove seeing branch = HEAD. Confirmed and fixed. spawn_hooks_after_remove now takes Option<&str>, and PostRemoveContext::extra_vars only pushes branch when there is one — the extra_vars write happening after the base context is exactly why the base-context guard alone wasn't enough. test_user_post_remove_branch_unset_in_detached_worktree pins it: with Some("HEAD") restored at the detached call site it fails with left: "branch=[HEAD]", and passes as written. The background pipeline's log file keeps its HEAD name through spawn_hook_pipeline_quiet's own fallback, so the PR description's "exactly one place" is accurate again rather than merely rewritten.

Stale comments. All four updated — hook_commands.rs (both), for_each.rs, and the expansion.rs block comment that contradicted the one below it.

The JSON-context example. Switched to ctx.get('branch', ''), with a sentence in the section saying unset variables are absent from the JSON too, so the guidance holds in both directions.

(unused) where (unset) is meant. Took the second option — format_variables_table now treats branch as always-computed (ALWAYS_COMPUTED_VARS) rather than amending the docstring's invariant, since the invariant is the useful thing: (unused) should keep meaning "the scope gate saved real work". test_verbose_variables_table_marks_detached_branch_unset reads the row out of the rendered table; it fails on (unused) with the list emptied.

with_base re-inlining. TemplateVars::with_base now takes Option<&str> itself rather than gaining a third _opt sibling next to with_base_strs, and with_target_opt joins with_target. The local closure and the hand-rolled to_posix_path are gone.

One thing not in the review that the with_base signature surfaced: wt switch was passing current_wt.branch().ok().flatten().unwrap_or_default(), so a pre-switch hook fired from a detached worktree got base = "" — the empty-string insertion format_variables_table's docstring explicitly warns about, and inconsistent with what wt hook pre-switch already renders after the first commit. It's unset now. Flagging it because it's a behavior change beyond the review's asks: an unguarded {{ base }} there errors where it used to render nothing. Happy to split it back out if you'd rather it land separately.

Verification

cargo run -- hook pre-merge --yes: 4745 tests, 2 failures, both reproduced on this PR's parent (926d8e5) in the same sandbox with the working tree stashed — test_copy_ignored_preserves_file_executable_permissions (0664 vs 0644 under the runner's umask 002) and test_powershell_skipped_when_installed_no_profile (this sandbox home already has a nushell autoload installed). Clippy --all-targets --all-features and cargo fmt --check clean. Doc mirrors regenerated by test_docs_are_in_sync.

[worktrunk-bot]

cafcd5d closes every point from the last pass, and I re-derived each rather than taking the summary's word for it: PostRemoveContext::extra_vars now pushes branch only when there is one and spawn_hooks_after_remove threads Option<&str> from all three call sites; the four falsified comments are gone (and expand_template_with really does inject vars unconditionally now, so that block comment is accurate rather than merely reworded); the JSON-context example is ctx.get('branch', '') in the primary and all three mirrors, with no --help snapshot to regenerate; ALWAYS_COMPUTED_VARS gives the (unset) label; and with_base taking Option<&str> drops the hand-rolled to_posix_path along with its import. test_user_post_remove_branch_unset_in_detached_worktree reaches the removal-driven post-remove context, which nothing in the suite did before.

Three things left, none of them blocking the design — details inline.

Not approving

Self-approval aside, this lands in the wt remove path: the diff edits spawn_hooks_after_remove and handle_detached_removed_worktree_output in src/output/handlers.rs, the file that also drives the wt remove -D / --force-delete messaging, and it changes what a user's post-remove hook receives — the hook that runs #4009's git push origin --delete {{ branch }}. That's the surface .claude/skills/running-tend/references/review-pr.md § "Data-Loss Surface: Hold for Human Review" says to hand to a human, so I've requested @max-sixty rather than signing off. The change makes that surface strictly safer — an undefined-variable error instead of a ref git resolves — but the "should these render empty instead of erroring?" question in the PR body is a product call, not a review one.

[worktrunk-bot]

a175d73 closes the last pass's three points, and I re-derived each rather than taking the summary's word for it: target_branch is Option<String> on the struct and extra_vars pushes ("target", …) only when there is one; both new tests use x is defined rather than {% if x %}, which is the only guard that separates absent from empty; and the format_variables_table docstring reads as suggested, with the extra_vars clause added. I ran test_user_post_remove_target_unset_with_detached_primary_worktree, test_user_pre_switch_base_unset_in_detached_worktree, and the branch sibling against this head — all three green.

One new finding, plus a doc line the increment makes stale.

pre-remove still hands target the empty string

The sibling hook of the same operation kept the unwrap_or_default() this commit removed from PostRemoveContext::new. In src/output/handlers.rs, execute_pre_remove_hooks_if_needed builds its own extra vars by hand:

let target_branch = repo
    .worktree_at(ctx.main_path)
    .branch()
    .ok()
    .flatten()
    .unwrap_or_default();
let target_path_str = worktrunk::path::to_posix_path(&ctx.main_path.to_string_lossy());
let extra_vars: Vec<(&str, &str)> = vec![
    ("target", &target_branch),
    ("target_worktree_path", &target_path_str),
];

Same repo.worktree_at(ctx.main_path).branch() expression, same detached-primary case, opposite outcome. So a single wt remove on a detached primary worktree now renders target two different ways: pre-remove gets an empty string, post-remove gets nothing — and wt -v prints target = for the first hook's variables block and target = (unset) for the second's, which is the label ALWAYS_COMPUTED_VARS was added to make trustworthy.

Confirmed by probe rather than inferred: a [pre-remove] capture of target=[{% if target is defined %}defined:{{ target }}{% else %}unset{% endif %}], with repo.detach_head() on the primary and wt remove feature --force --yes, writes target=[defined:] on this head — the exact pre-fix shape test_user_post_remove_target_unset_with_detached_primary_worktree asserts against for its own hook.

TemplateVars already does this, and hook_commands.rs already routes these same two hook types through it — PreRemove | PostRemove => TemplateVars::new().with_target_opt(branch).with_target_worktree_path(worktree_path). The operation-driven site can say the same thing:

let target_branch = repo.worktree_at(ctx.main_path).branch().ok().flatten();
let extra_vars = TemplateVars::new()
    .with_target_opt(target_branch.as_deref())
    .with_target_worktree_path(ctx.main_path)
    .as_extra_vars();

That line sits outside this PR's hunks, so it needs a commit rather than a suggestion. A sibling of the post-remove test covers it — same fixture, [pre-remove] instead of [post-remove].

The wider point is why it got missed: this is the third hand-rolled "push only when Some" site, and the one that slipped is the only one that never went through TemplateVars. as_extra_vars is where the omit-rather-than-empty rule is structural instead of restated, and the docstring this commit just extended asks every extra_vars layer to hold that rule.

The docs still scope unset base / target to a manual wt hook

Inline. Worth noting it renders into docs/src/content/docs/hook.md and both reference mirrors, so test_docs_are_in_sync regenerates them.

---

Not approving: self-authored, and the hold from the last pass stands — this edits the wt remove hook path, which .claude/skills/running-tend/references/review-pr.md § "Data-Loss Surface: Hold for Human Review" hands to a human. @max-sixty is already on it.

[worktrunk-bot]

b7ca619 closes both points from the last pass, and I re-derived each rather than reading the summary. execute_pre_remove_hooks_if_needed no longer builds its own vec — it's TemplateVars::new().with_target_opt(…).with_target_worktree_path(ctx.main_path).as_extra_vars(), which emits exactly the two pairs the hand-rolled version did, in the same order, with the same to_posix_path conversion, and omits target when the primary worktree is detached. The renamed test_user_remove_hooks_target_unset_with_detached_primary_worktree now asserts target=[unset] for both halves of one removal, so the pre-remove / post-remove disagreement is pinned rather than merely fixed. The hook long-help sentence is the suggested wording verbatim, check-docs is green on the three mirrors, and test (linux) passed the full suite at this head.

No new findings — nothing in the increment, and a fresh pass over the whole diff turned up nothing the earlier rounds missed.

One observation, outside the diff and not something I'd change here. wt list's custom columns are the other place a user writes {{ branch }}, and they resolve it the opposite way: expand_custom_columns does let branch = item.branch().unwrap_or("") and inserts that unconditionally, so a detached row renders an empty cell where a hook now errors. That's defensible on its own terms — an undefined-variable error in a table cell just blanks the cell anyway — and the new expansion.rs comment cites that function only for the empty-vars-map decision, which is accurate. But it does mean "branch is unset in a detached worktree" is true of hooks, aliases, for-each, and --format=json, and not of [list.columns]. Worth knowing when deciding whether the erroring behavior is the one to standardize on; not worth widening this PR for.

Not approving: self-authored, and the hold from the earlier passes stands — @max-sixty is already on it.

[max-sixty]

why does this output change?

[worktrunk-bot]

Because I changed the test's own command, not just its result — the args line at the top of the snapshot goes from Branch: {{ branch }} to Branch: [{% if branch %}{{ branch }}{% endif %}]. The brackets are why every row moved (Branch: main → Branch: [main]), even the four worktrees the fix doesn't touch; the one substantive line is the detached row, Branch: HEAD → Branch: [].

I rewrote it rather than left it because the unguarded form no longer reaches the end of the loop: it's an undefined-variable error at the detached worktree and for-each stops there, so the original command would have stopped covering "visits all five worktrees". That case didn't get dropped — it's test_for_each_detached_branch_variable_unguarded, whose new snapshot keeps the original {{ branch }} command and captures the error plus the aborted loop.

In hindsight the two are the wrong way round for review: had I kept the unguarded command on the existing test, its snapshot diff would have shown Branch: HEAD → the error directly, and the guarded walk-all-worktrees case would be the new file. Same coverage, but the behavior change would be visible in the diff instead of hidden behind a changed command. Happy to swap them if you'd like — it's test-arrangement only, no production change.

The unguarded snapshot

----- stderr -----
◎ Running in main...
Branch: main
◎ Running in repo.detached-test (detached)...
✗ Failed to expand for-each argument: undefined value @ line 1
  Branch: {{ branch }}
↳ Available variables: commit, cwd, default_branch, main_worktree, main_worktree_path, primary_worktree_path, remote, remote_url, repo, repo_path, repo_root, short_commit, worktree, worktree_name, worktree_path

exit_code: 1, and feature-a/-b/-c never run — the loop aborts on an undefined variable the same way {{ upstream }} already did at the first non-tracking worktree.