Skip to content

nushell wrapper calls sh unconditionally, including on Windows #3944

Closed
divoxx/gentoo-overlay
#126

Description

@worktrunk-bot

Surfaced by the nightly survey of templates/nushell.nu. Not reproduced — I have no Windows+nushell host in CI, so this is a code-reading finding with the evidence below rather than an observed failure.

The inconsistency

templates/nushell.nu branches on the platform for its temp-file cleanup, so Windows is clearly a platform the wrapper expects to run on:

# branches so cleanup can never abort the wrapper. `hide rm` is not an
# option: it's a parse-time keyword that leaks out of this function into
# the user's session, silently unbinding their alias.
if $nu.os-info.family == "windows" {
try { rm -f $cd_file $exec_file $stdout_file }

But two other spots in the same file call sh unconditionally:

  • L129 — try { ^sh -c $script; 0 }, running an --execute body.
  • L167 — ^sh -c $"exit ($exit_code)", the exit-code propagation path, reached whenever wt exits non-zero.

L167 is the wider path of the two: it fires on any failing wt command, not just --execute.

Why it's reachable on Windows

Shell::Nushell isn't platform-gated. It's in Shell::all(), and configure_shell.rs offers it purely on Shell::Nushell.is_installed() — no Windows exclusion. So wt config shell install on a Windows box with nu on PATH writes this wrapper.

The test that covers the failure path does include nushell — test_wrapper_handles_command_failure has a #[case("nu")] — but it sits inside mod unix_tests under #[cfg(unix)], so it only ever runs where sh exists. That's consistent with the gap never having shown up in CI.

For contrast, templates/powershell.ps1 needs no POSIX shell for either job: it uses Invoke-Expression $script for exec bodies and sets $global:LASTEXITCODE for propagation.

The design question

L167 looks mechanically replaceable with something nushell-native. L129 is the harder one, and it's why this is an issue rather than a PR: --execute bodies are POSIX snippets by construction — ShellEscapeMode::Posix is documented as "the wrapper-independent default for bash, zsh, and nushell". So on Windows there may be no correct shell to hand them to, and the options (require sh, refuse to install the nu wrapper on Windows, document the limitation, or something else) are a maintainer call rather than a mechanical fix.

Happy to implement whichever direction you'd prefer.

Possibly relevant: #2860 proposes moving wt switch --execute to an argv input model. If exec bodies stop being POSIX snippets, the L129 half of this may dissolve on its own, leaving only L167 to fix.

Activity

  1. worktrunk-bot commented on Aug 28, 2026

    @worktrunk-bot
    CollaboratorAuthor

    Reproduced the L167 half and opened #3945 for it.

    The mechanism doesn't need Windows — restricting PATH to nu and git gives the same failure on Linux, since "no POSIX shell to spawn" is the only property of Windows this path depends on. Two things go wrong on every failing wt command, and the second wasn't in the original report: the wrapper prints its own source at the user, and the real exit code is flattened to 1 (a wt that exited 3 exits 1, because nushell's own not-found error replaces it).

    The fix is ^$nu.current-exe --no-config-file -c $"exit ($exit_code)" rather than a $nu.os-info.family branch keeping ^sh on Unix. One command on every platform makes the path Unix CI exercises the same path Windows runs — which is precisely the gap that let this exist. It costs ~7 ms per failing command on Unix (0.67 ms for sh vs 7.5 ms for nu --no-config-file), on a path that has finished its git work and is handing control back to the prompt.

    L129 is untouched — the design question stands as written, so this issue stays open for it. The template now carries an inline note pointing here, so the file doesn't read as sh-free. If #2860 lands first and exec bodies stop being POSIX snippets, that note is what should be revisited.

    Reproduction and verification

    Against the unmodified template, with PATH holding only nu and git:

    ✗ Branch existing already exists
    ↳ To switch to the existing branch, run without --create: wt switch existing
    Error: nu::shell::external_command
    
      × External command failed
         ╭─[source:171:14]
     170 │             if ($output | is-not-empty) { print -n $output }
     171 │             ^sh -c $"exit ($exit_code)"
         ·              ─┬
         ·               ╰── Command `sh` not found
     172 │         } else if ($output | is-not-empty) {
         ╰────
      help: Did you mean `hash`?
    

    Exit-code fidelity checked separately in a bare nu script: ^sh -c "exit 3" under that PATH exits 1, ^$nu.current-exe --no-config-file -c "exit 3" exits 3 with and without sh present.

    test_nushell_wrapper_failure_needs_no_posix_shell (in tests/integration_tests/shell_wrapper.rs) pins this: it drives the real wrapper through a PTY with PATH restricted to symlinks for nu and git. Fails on the old template, passes on the new one. It lives in mod unix_tests deliberately — the assertion is about the absence of a POSIX shell, not about Windows, so a Unix runner proves it.

    All required checks on #3945 are green: test (linux), test (macos), test (windows), fast-checks, plus codecov/patch and code-coverage.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions