From 34bf34bd5b49b9caba6bee4328b563136cde1d64 Mon Sep 17 00:00:00 2001 From: Andrew Stilliard Date: Wed, 12 Aug 2026 22:25:03 +0100 Subject: [PATCH] fix: don't name shell variables "path" - zsh ties it to $PATH In zsh the `path` array is tied to `PATH`, so a `local path` (or a bare `read -r path`) empties PATH for that function's scope and everything it calls. `wt merged --rm` hit this at `local main_wt path branch`: every subsequent git/awk call failed with "command not found", _wt_resolve returned nothing, and removal aborted with "no worktree matching". Rename to wt_path at all four sites - declaration and every use. Renaming only the declaration would be worse: the leftover `read -r path` would then write into zsh's tied array and clobber PATH permanently rather than for the function's scope. WT_PATH, the hook env var, is not a special name and is unchanged. Also drop the claim that the `claude` CLI mutates the caller's PATH - a misdiagnosis of this same bug (a subprocess cannot alter its parent's environment). The up-front binary resolution stays; the one-shot session fetch is still worth it. Tests run under bash, where this bug class is invisible, so add a behavioural zsh test (skipped when zsh is absent) plus a static grep guard. Both fail against the pre-fix wt.sh. Co-Authored-By: Claude Opus 5 --- test/helpers.bash | 3 ++- test/zsh.bats | 28 ++++++++++++++++++++++++++++ wt.sh | 36 +++++++++++++++++++----------------- 3 files changed, 49 insertions(+), 18 deletions(-) create mode 100644 test/zsh.bats diff --git a/test/helpers.bash b/test/helpers.bash index 4f63c34..b3dff5d 100644 --- a/test/helpers.bash +++ b/test/helpers.bash @@ -25,7 +25,8 @@ wt_common_setup() { while [ ! -f "$dir/wt.sh" ] && [ "$dir" != "/" ]; do dir=$(dirname "$dir") done - source "$dir/wt.sh" + WT_SH="$dir/wt.sh" + source "$WT_SH" } wt_common_teardown() { diff --git a/test/zsh.bats b/test/zsh.bats new file mode 100644 index 0000000..49aac10 --- /dev/null +++ b/test/zsh.bats @@ -0,0 +1,28 @@ +load helpers + +setup() { wt_common_setup; } +teardown() { wt_common_teardown; } + +# zsh ties the `path` array to $PATH, so a `local path` (or a bare +# `read -r path`) inside a function empties PATH for that function and +# everything it calls - git/awk stop resolving mid-run. bash ignores this +# entirely, so the behavioural test has to run under a real zsh. + +@test "wt merged --rm works under zsh (PATH not clobbered by a 'path' local)" { + command -v zsh >/dev/null || skip "zsh not installed" + git -C "$TEST_REPO" merge -q feature + + run zsh -c "cd '$TEST_REPO'; source '$WT_SH'; wt merged --rm -y" + [ "$status" -eq 0 ] + [[ "$output" != *"git: command not found"* ]] + [[ "$output" != *"command not found: git"* ]] + [[ "$output" != *"no worktree matching"* ]] + [ ! -d "$TEST_REPO-feature" ] +} + +# static guard: catches new occurrences in any function, including ones with +# no zsh test coverage. WT_PATH (the hook env var) is not a special name. +@test "no shell variable is named 'path'" { + run grep -nE '(^|[[:space:];])(local|typeset)[^=]*[[:space:]]path([[:space:]=]|$)|read([[:space:]]+-[^[:space:]]+)*[[:space:]]+path([[:space:]]|$)' "$WT_SH" + [ "$status" -ne 0 ] +} diff --git a/wt.sh b/wt.sh index 4b21a75..f57c78f 100644 --- a/wt.sh +++ b/wt.sh @@ -44,9 +44,8 @@ _wt_ls() { } # resolve claude/jq/column to absolute paths up front and fetch the full -# session list once. Some versions of `claude` mutate the calling shell's -# PATH as a side effect, so every lookup must happen before the first call -# to it - sets _WT_CLAUDE_BIN / _WT_JQ_BIN / _WT_COLUMN_BIN / _WT_CLAUDE_JSON. +# session list once - sets _WT_CLAUDE_BIN / _WT_JQ_BIN / _WT_COLUMN_BIN / +# _WT_CLAUDE_JSON. # Returns 0 on success, 1 if `claude` is missing (non-fatal, caller falls # back to its normal output), 2 if `jq` is missing (fatal). _wt_claude_init() { @@ -96,13 +95,14 @@ _wt_claude_table() { local main_wt main_wt=$(git worktree list --porcelain | awk '/^worktree /{print $2; exit}') - local path branch display_branch sessions rows + # NB: never name a shell variable "path" - zsh ties it to $PATH + local wt_path branch display_branch sessions rows rows="BRANCH"$'\t'"SESSION"$'\t'"NAME"$'\t'"STATE"$'\n' - while IFS=$'\t' read -r path branch; do - [ -z "$path" ] && continue + while IFS=$'\t' read -r wt_path branch; do + [ -z "$wt_path" ] && continue display_branch="$branch" - [ "$path" = "$main_wt" ] && display_branch="(main, branch varies)" - sessions=$(printf '%s' "$_WT_CLAUDE_JSON" | "$_WT_JQ_BIN" -r --arg wt "$path" \ + [ "$wt_path" = "$main_wt" ] && display_branch="(main, branch varies)" + sessions=$(printf '%s' "$_WT_CLAUDE_JSON" | "$_WT_JQ_BIN" -r --arg wt "$wt_path" \ '.[] | select(.cwd == $wt) | [.id, (.name // "-"), .state] | @tsv') if [ -z "$sessions" ]; then rows+="$display_branch"$'\t'"-"$'\t'"-"$'\t'"-"$'\n' @@ -123,8 +123,8 @@ _wt_claude_table() { # delete the Claude Code sessions recorded against a worktree path (claude rm); # expects _wt_claude_init to have been run already _wt_claude_rm_sessions() { - local path="$1" rc=0 ids id - ids=$(printf '%s' "$_WT_CLAUDE_JSON" | "$_WT_JQ_BIN" -r --arg wt "$path" \ + local wt_path="$1" rc=0 ids id + ids=$(printf '%s' "$_WT_CLAUDE_JSON" | "$_WT_JQ_BIN" -r --arg wt "$wt_path" \ '.[] | select(.cwd == $wt) | .id // empty') [ -z "$ids" ] && return 0 while IFS= read -r id; do @@ -158,13 +158,13 @@ _wt_run_hook() { # run an ad-hoc hook script passed via --pre-hook / --post-hook _wt_run_adhoc_hook() { - local file="$1" branch="$2" path="$3" + local file="$1" branch="$2" wt_path="$3" [ -n "$file" ] || return 0 [ -e "$file" ] || { echo "wt: hook file not found: $file" >&2; return 1; } if [ -x "$file" ]; then - WT_BRANCH="$branch" WT_PATH="$path" "$file" + WT_BRANCH="$branch" WT_PATH="$wt_path" "$file" else - WT_BRANCH="$branch" WT_PATH="$path" bash "$file" + WT_BRANCH="$branch" WT_PATH="$wt_path" bash "$file" fi } @@ -330,11 +330,13 @@ _wt_merged() { fi # never remove the main working tree, even if it's on a merged branch - local main_wt path branch failed=0 + # NB: never name a shell variable "path" - zsh ties it to $PATH, so a + # `local path` (or a bare `read -r path`) wipes PATH for everything below + local main_wt wt_path branch failed=0 main_wt=$(git worktree list --porcelain | awk '/^worktree /{print $2; exit}') - while IFS=$'\t' read -r path branch; do - [ -z "$path" ] && continue - if [ "$path" = "$main_wt" ]; then + while IFS=$'\t' read -r wt_path branch; do + [ -z "$wt_path" ] && continue + if [ "$wt_path" = "$main_wt" ]; then echo "wt: skipping main worktree [$branch]" >&2 continue fi