You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
nushell wrapper: a failing --execute body aborts before cleanup, leaking temp files and dropping stdout #3733
A --execute body that exits non-zero aborts the nushell wrapper before its cleanup runs: all three temp files leak, and the command's stdout is discarded. Split out of the review on #3732, which fixed the same abort-before-cleanup shape for the rm-alias case; this one is on a supported path rather than an alias-dependent one, and it's pre-existing — cleanup has always sat after the exec call.
The exec-directive call isn't wrapped in try — templates/nushell.nu#L121-L124. Nushell 0.98+ raises ShellError on a non-zero external exit, so ^sh -c $script unwinds the whole def.
Verified
Nushell 0.114.1, the real rendered wrapper, dedicated TMPDIR:
✓ Created branch execfail from main and worktree @ /tmp/nurepo2.execfail
◎ Executing (--execute):
exit 42
REACHED_AFTER_WT never prints — the wrapper aborted mid-function, taking the rest of the calling script with it — and TMPDIR holds all three mktemp files afterwards. Bare mechanism, no wrapper:
def wrapper [] {
let f = (mktemp --tmpdir)
if true { ^sh -c "exit 42" }
print"REACHED_AFTER_EXEC"# never prints"RETVAL"# never returned
}
Why a green test doesn't catch it
test_wrapper_execute_exit_code_propagation already drives this for nu with --execute "exit 42", and passes — but it passes because the abort happens to carry exit 42 out to the shell, not because the wrapper handled it. The assertion the test makes is satisfied by the failure mode. So the leak sits on a path with test coverage.
Proposed fix
Capture the exec exit the way the bash and zsh wrappers do with src_exit, rather than letting the abort carry it:
folded into $exit_code when that's still 0. The end of the function then propagates it through the existing ^sh -c $"exit ($exit_code)" path, which keeps the 42 the test asserts and additionally prints the stdout that's currently dropped, with cleanup running on the way.
Filed as an issue rather than a PR because it changes exit-code propagation: when bothwt and the --execute body fail, something has to decide which code wins, and that's a user-visible contract rather than a mechanical port. Once it lands, test_wrapper_execute_exit_code_propagation should also assert the wrapper is still executing after the failing body — otherwise the next regression passes the same way this one does.
The one open question here was the precedence call — when both wt and the --execute body fail, which code wins — and it turns out it's already decided elsewhere: bash, zsh and fish all guard their src_exit assignment with if $exit_code -eq 0 — first non-zero wins, wt before the body. So the nushell change mirrors an existing contract rather than setting a new one, which is what moved it from an issue to a PR.
The reproduction test asserts the temp dir is empty afterwards, not just the exit code. WT_EXIT:42 is present in the failing run too, so an exit-code assertion alone reproduces the same blind spot test_wrapper_execute_exit_code_propagation has.
Verification
Nushell 0.114.1 (the version CI pins), real rendered wrapper, dedicated TMPDIR:
The new test_nu_wrapper_execute_failure_runs_cleanup fails on the pre-fix template with leaked temp files: ["tmp.zyJKCYvPHq", "tmp.eF7z3IFox0", "tmp.0sJXkEnCXb"] and passes with it.
Note the wrapper's terminal ^sh -c $"exit ($exit_code)" still aborts the caller after the fix — that's how the code propagates, and it's why the driving script in the test wraps the wt call in nushell's own try. "Still executing after the failing body" is only observable inside the wrapper, hence the temp-dir assertion.
Kept separate from #3732: same abort-before-cleanup shape, but that one is the cleanup rm and this is the exec call — neither fixes the other's site. The hunks are adjacent in templates/nushell.nu but don't overlap, so whichever lands second should merge cleanly.
Activity
worktrunk-bot commentedon Aug 4, 2026
Reproduced and fixed in #3734.
The one open question here was the precedence call — when both
wtand the--executebody fail, which code wins — and it turns out it's already decided elsewhere: bash, zsh and fish all guard theirsrc_exitassignment withif $exit_code -eq 0— first non-zero wins, wt before the body. So the nushell change mirrors an existing contract rather than setting a new one, which is what moved it from an issue to a PR.The reproduction test asserts the temp dir is empty afterwards, not just the exit code.
WT_EXIT:42is present in the failing run too, so an exit-code assertion alone reproduces the same blind spottest_wrapper_execute_exit_code_propagationhas.Verification
Nushell 0.114.1 (the version CI pins), real rendered wrapper, dedicated
TMPDIR:--execute "echo BODY; exit 42"wt switch --definitely-not-a-flag(wt itself fails)--execute "echo OK", thenwt config showThe new
test_nu_wrapper_execute_failure_runs_cleanupfails on the pre-fix template withleaked temp files: ["tmp.zyJKCYvPHq", "tmp.eF7z3IFox0", "tmp.0sJXkEnCXb"]and passes with it.Note the wrapper's terminal
^sh -c $"exit ($exit_code)"still aborts the caller after the fix — that's how the code propagates, and it's why the driving script in the test wraps thewtcall in nushell's owntry. "Still executing after the failing body" is only observable inside the wrapper, hence the temp-dir assertion.Kept separate from #3732: same abort-before-cleanup shape, but that one is the cleanup
rmand this is the exec call — neither fixes the other's site. The hunks are adjacent intemplates/nushell.nubut don't overlap, so whichever lands second should merge cleanly.