From 8996fb2a49cc8d4307909fd3e7d421604fbaa82e Mon Sep 17 00:00:00 2001 From: Lien Li Date: Fri, 4 Sep 2026 12:43:42 +0800 Subject: [PATCH 1/2] fix: detect invoking shell for init Assisted-By: devx/81468caa-b221-4aa1-a09a-044e49944132 --- spec/command_line.md | 2 +- spec/init_spec.md | 6 +++--- spec/tests/test_14_init_shells.sh | 8 ++++++-- spec/tests/test_34_shell_init.sh | 28 +++++++++++++++++++++++++--- spec/tests/test_36_shell_eval.sh | 4 ++-- try.rb | 8 +++----- 6 files changed, 40 insertions(+), 16 deletions(-) diff --git a/spec/command_line.md b/spec/command_line.md index 34f2de9..25cf793 100644 --- a/spec/command_line.md +++ b/spec/command_line.md @@ -192,7 +192,7 @@ When `HERDR_ENV=1` and `HERDR_PANE_ID` are present, the script includes a guarde | Variable | Description | |----------|-------------| | `HOME` | Used to resolve default tries path (`$HOME/src/tries`) | -| `SHELL` | Used by `init` to detect shell type | +| `SHELL` | Fallback used by `init` when the invoking shell cannot be detected | | `NO_COLOR` | If set, disables colors (equivalent to `--no-colors`) | | `HERDR_ENV`, `HERDR_PANE_ID`, `HERDR_WORKSPACE_ID` | Identify the current Herdr panel and workspace for renaming | | `CMUX_SOCKET_PATH`, `CMUX_BUNDLE_ID` | Identify cmux for tab renaming | diff --git a/spec/init_spec.md b/spec/init_spec.md index 6e61448..7c9faa0 100644 --- a/spec/init_spec.md +++ b/spec/init_spec.md @@ -11,7 +11,7 @@ The shell function wrapper is necessary because: ## Shell Detection -The init command should detect the user's shell via the `$SHELL` environment variable and output the appropriate function syntax. +The init command should detect the shell that invokes it from the parent process and output the appropriate function syntax. If the parent process cannot be determined, it should fall back to the user's configured shell in `$SHELL`. Supported shells: - **Bash/Zsh**: POSIX-compatible function syntax @@ -96,8 +96,8 @@ Test that init produces valid shell syntax: # Test Bash syntax bash -n <(try init) -# Test Fish syntax (if fish is available) -fish -n <(SHELL=/usr/bin/fish try init) +# Test Fish syntax, even when the login shell is different +env SHELL=/bin/zsh fish -c 'try init | fish --no-execute' ``` Test that the wrapper works correctly: diff --git a/spec/tests/test_14_init_shells.sh b/spec/tests/test_14_init_shells.sh index 24d1504..51f44ae 100644 --- a/spec/tests/test_14_init_shells.sh +++ b/spec/tests/test_14_init_shells.sh @@ -18,13 +18,17 @@ else fail "bash function should include --path with specified path" "--path '$TEST_TRIES'" "$output" "init_spec.md" fi -# Test: init with fish shell emits fish function -output=$(SHELL=/usr/bin/fish try_run init "$TEST_TRIES" 2>&1) +# Test: init invoked by fish emits fish function +FAKE_PS_DIR=$(mktemp -d) +printf '#!/bin/sh\nprintf "fish\\n"\n' > "$FAKE_PS_DIR/ps" +chmod +x "$FAKE_PS_DIR/ps" +output=$(PATH="$FAKE_PS_DIR:$PATH" SHELL=/bin/zsh try_run init "$TEST_TRIES" 2>&1) if echo "$output" | grep -q "function try"; then pass else fail "init with fish should emit fish function" "function try" "$output" "init_spec.md" fi +rm -rf "$FAKE_PS_DIR" # Test: init output contains the real, full path to try binary output=$(SHELL=/bin/bash try_run init "$TEST_TRIES" 2>&1) diff --git a/spec/tests/test_34_shell_init.sh b/spec/tests/test_34_shell_init.sh index 127bd8c..1c9f559 100644 --- a/spec/tests/test_34_shell_init.sh +++ b/spec/tests/test_34_shell_init.sh @@ -3,14 +3,36 @@ section "shell-init" -# Test: SHELL=fish emits fish function -output=$(SHELL=/usr/local/bin/fish try_run init "$TEST_TRIES" 2>&1) +# Test: the invoking shell takes precedence over the login shell in $SHELL +FAKE_PS_DIR=$(mktemp -d) +printf '#!/bin/sh\nprintf "fish\\n"\n' > "$FAKE_PS_DIR/ps" +chmod +x "$FAKE_PS_DIR/ps" +output=$(PATH="$FAKE_PS_DIR:$PATH" SHELL=/bin/zsh try_run init "$TEST_TRIES" 2>&1) if echo "$output" | grep -q "function try"; then pass else - fail "SHELL=fish should emit fish function" "function try" "$output" "shell_init" + fail "fish parent should override SHELL=zsh" "function try" "$output" "shell_init" fi +# Test: a non-fish parent also takes precedence over SHELL=fish +printf '#!/bin/sh\nprintf "zsh\\n"\n' > "$FAKE_PS_DIR/ps" +output=$(PATH="$FAKE_PS_DIR:$PATH" SHELL=/usr/local/bin/fish try_run init "$TEST_TRIES" 2>&1) +if echo "$output" | grep -q "try() {"; then + pass +else + fail "zsh parent should override SHELL=fish" "try() {" "$output" "shell_init" +fi + +# Test: SHELL is used when the parent shell cannot be detected +printf '#!/bin/sh\n' > "$FAKE_PS_DIR/ps" +output=$(PATH="$FAKE_PS_DIR:$PATH" SHELL=/usr/local/bin/fish try_run init "$TEST_TRIES" 2>&1) +if echo "$output" | grep -q "function try"; then + pass +else + fail "SHELL=fish should be the fallback" "function try" "$output" "shell_init" +fi +rm -rf "$FAKE_PS_DIR" + # Test: SHELL=zsh emits bash/zsh function output=$(SHELL=/bin/zsh try_run init "$TEST_TRIES" 2>&1) if echo "$output" | grep -q "try() {"; then diff --git a/spec/tests/test_36_shell_eval.sh b/spec/tests/test_36_shell_eval.sh index e19bc3c..28c16a6 100644 --- a/spec/tests/test_36_shell_eval.sh +++ b/spec/tests/test_36_shell_eval.sh @@ -65,7 +65,7 @@ fi if command -v nix-shell >/dev/null 2>&1; then # Test: fish can eval the init output and defines try function - fish_out=$(nix-shell -p fish --run "SHELL=fish fish -c 'eval ($TRY_BIN_PATH init --path $EVAL_DIR | string collect); type try'" 2>&1) + fish_out=$(nix-shell -p fish --run "SHELL=/bin/zsh fish -c 'eval ($TRY_BIN_PATH init --path $EVAL_DIR | string collect); type try'" 2>&1) if echo "$fish_out" | grep -qi "try is a function\|function try"; then pass else @@ -81,7 +81,7 @@ if command -v nix-shell >/dev/null 2>&1; then fi # Test: fish init output is valid fish syntax (no parse errors) - fish_syntax=$(SHELL=fish "$TRY_BIN_PATH" init --path "$EVAL_DIR" 2>&1) + fish_syntax=$(nix-shell -p fish --run "SHELL=/bin/zsh fish -c '$TRY_BIN_PATH init --path $EVAL_DIR'" 2>&1) fish_parse=$(echo "$fish_syntax" | nix-shell -p fish --run "fish --no-execute" 2>&1) if [ $? -eq 0 ]; then pass diff --git a/try.rb b/try.rb index c3b3c0a..324eccf 100755 --- a/try.rb +++ b/try.rb @@ -1642,12 +1642,10 @@ def resolve_unique_name_with_versioning(tries_path, date_prefix, base) end # shell detection for init wrapper - # Check $SHELL first (user's configured shell), then parent process as fallback + # Check the invoking shell first, then fall back to the user's configured shell def fish? - shell = ENV["SHELL"].to_s - if shell.empty? - shell = (`ps c -p #{Process.ppid} -o 'ucomm='`.strip rescue "").to_s - end + shell = (`ps c -p #{Process.ppid} -o 'ucomm=' 2>/dev/null`.strip rescue "").to_s + shell = ENV["SHELL"].to_s if shell.empty? shell.include?('fish') end From 2f7b87201af7c97f3e2029adab03bd42ab92ce29 Mon Sep 17 00:00:00 2001 From: Lien Li Date: Fri, 4 Sep 2026 13:09:42 +0800 Subject: [PATCH 2/2] fix: only trust the parent process when it is a shell A non-shell parent (script runner, make, non-exec wrapper) would otherwise override a correct $SHELL=fish and emit bash syntax. Fall back to $SHELL unless the parent name looks like a shell. Co-Authored-By: Claude Fable 5 Assisted-By: devx/29cc08f5-8976-4c01-bdc7-ff3402fcbfbb --- spec/init_spec.md | 2 +- spec/tests/test_34_shell_init.sh | 9 +++++++++ try.rb | 4 ++-- 3 files changed, 12 insertions(+), 3 deletions(-) diff --git a/spec/init_spec.md b/spec/init_spec.md index 7c9faa0..3ca8439 100644 --- a/spec/init_spec.md +++ b/spec/init_spec.md @@ -11,7 +11,7 @@ The shell function wrapper is necessary because: ## Shell Detection -The init command should detect the shell that invokes it from the parent process and output the appropriate function syntax. If the parent process cannot be determined, it should fall back to the user's configured shell in `$SHELL`. +The init command should detect the shell that invokes it from the parent process and output the appropriate function syntax. If the parent process cannot be determined or is not a shell, it should fall back to the user's configured shell in `$SHELL`. Supported shells: - **Bash/Zsh**: POSIX-compatible function syntax diff --git a/spec/tests/test_34_shell_init.sh b/spec/tests/test_34_shell_init.sh index 1c9f559..544de67 100644 --- a/spec/tests/test_34_shell_init.sh +++ b/spec/tests/test_34_shell_init.sh @@ -31,6 +31,15 @@ if echo "$output" | grep -q "function try"; then else fail "SHELL=fish should be the fallback" "function try" "$output" "shell_init" fi + +# Test: a non-shell parent (e.g. a script runner) is ignored in favor of SHELL +printf '#!/bin/sh\nprintf "ruby\\n"\n' > "$FAKE_PS_DIR/ps" +output=$(PATH="$FAKE_PS_DIR:$PATH" SHELL=/usr/local/bin/fish try_run init "$TEST_TRIES" 2>&1) +if echo "$output" | grep -q "function try"; then + pass +else + fail "non-shell parent should fall back to SHELL=fish" "function try" "$output" "shell_init" +fi rm -rf "$FAKE_PS_DIR" # Test: SHELL=zsh emits bash/zsh function diff --git a/try.rb b/try.rb index 324eccf..07ad351 100755 --- a/try.rb +++ b/try.rb @@ -1642,10 +1642,10 @@ def resolve_unique_name_with_versioning(tries_path, date_prefix, base) end # shell detection for init wrapper - # Check the invoking shell first, then fall back to the user's configured shell + # Check the invoking shell first; fall back to $SHELL when the parent isn't a shell def fish? shell = (`ps c -p #{Process.ppid} -o 'ucomm=' 2>/dev/null`.strip rescue "").to_s - shell = ENV["SHELL"].to_s if shell.empty? + shell = ENV["SHELL"].to_s unless shell.match?(/\A-?\w*sh\z/) shell.include?('fish') end