Skip to content

fix(terminal): prefer $SHELL over bash for background process spawning (#42203) - #53110

Merged
kshitijk4poor merged 1 commit into
mainfrom
salvage/42203-find-shell-prefer-usershell
Jun 26, 2026
Merged

kshitijk4poor merged 1 commit into
mainfrom
salvage/42203-find-shell-prefer-usershell

Conversation

@kshitijk4poor

Copy link
Copy Markdown
Contributor

Summary

On macOS, terminal(background=true) silently failed — the process returned a session_id and exit_code=0, but the command never ran (empty stdout, no side effects). This fixes it by spawning background processes with the user's actual login shell.

Root cause (#42203)

Two interacting issues:

  1. _find_shell was aliased to _find_bash, which prefers shutil.which("bash")/bin/bash (GNU bash 3.2, still shipped on macOS) over $SHELL (/bin/zsh, the Catalina+ default).
  2. process_registry.spawn_local runs [shell, "-lic", "set +m; <cmd>"] with stdin=/dev/null. bash 3.2 as a login shell (-l) sources ~/.bash_profile, which on many macOS setups contains exec /bin/zsh -l. That exec replaces bash with zsh but drops the -c argument, so the command is silently swallowed (exit 0, no output, no side effects).

Fix

Decouple _find_shell from _find_bash:

  • _find_shell now prefers the user's configured $SHELL on POSIX (the shell they actually log in with), falling back to _find_bash when $SHELL is unset/missing. process_registry uses this for the user-facing background command.
  • _find_bash is unchanged — callers that genuinely need bash (e.g. the _run_bash login-shell environment snapshot) keep bash semantics.

zsh handles -lic correctly even with redirected stdin, so the swallow no longer occurs.

Validation

  • tests/tools/test_find_shell.py9 passed.
  • Bug reproduced directly: system /bin/bash 3.2 invoked -lic with stdin=/dev/null and a ~/.bash_profile containing exec /bin/zsh -lexit 0, file NOT created, empty stdout (the silent swallow); /bin/zsh -lic with the same HOME → file created.
  • Coverage: original 8 unit tests ($SHELL set/unset/missing/empty, Windows-ignores-$SHELL, returns-string, _find_bash-unchanged) + an E2E regression test reproducing the real swallow and asserting the shell _find_shell selects actually executes a -lic background command.
  • Mutation-verified: reverting _find_shell to the = _find_bash alias fails the $SHELL-preference test.
  • ruff clean. (One pre-existing unrelated failure in test_process_registry.py::test_close_stdin_allows_eof_driven_process_to_finish reproduces identically on clean origin/main — not caused by this change.)

Salvaged from #42219 by @liuhao1024 (authorship preserved via cherry-pick; already in AUTHOR_MAP). Added the E2E regression test on top of the original.

Closes #42203. Supersedes #42290 (which changed _find_bash itself — riskier, as bash-dependent callers like _run_bash would then receive $SHELL/zsh).

@github-actions

github-actions Bot commented Jun 26, 2026

Copy link
Copy Markdown
Contributor

🔎 Lint report: salvage/42203-find-shell-prefer-usershell vs origin/main

ruff

Total: 0 on HEAD, 0 on base (➖ 0)

🆕 New issues: none

✅ Fixed issues: none

Unchanged: 0 pre-existing issues carried over.

ty (type checker)

Total: 11488 on HEAD, 11489 on base (✅ -1)

🆕 New issues (2):

Rule Count
invalid-assignment 1
unresolved-import 1
First entries
tests/run_agent/test_credits_notices_toggle.py:76: [invalid-assignment] invalid-assignment: Object of type `None` is not assignable to attribute `_credits_session_start_micros` of type `int`
tests/tools/test_find_shell.py:14: [unresolved-import] unresolved-import: Cannot resolve imported module `pytest`

✅ Fixed issues (2):

Rule Count
unresolved-attribute 2
First entries
tests/run_agent/test_credits_notices_toggle.py:76: [unresolved-attribute] unresolved-attribute: Unresolved attribute `_credits_session_start_micros` on type `AIAgent`
run_agent.py:3002: [unresolved-attribute] unresolved-attribute: Object of type `Self@get_credits_spent_micros` has no attribute `_credits_session_start_micros`

Unchanged: 6033 pre-existing issues carried over.

Diagnostics are surfaced as warnings — this check never fails the build.

@alt-glitch alt-glitch added type/bug Something isn't working comp/tools Tool registry, model_tools, toolsets tool/terminal Terminal execution and process management P1 High — major feature broken, no workaround duplicate This issue or pull request already exists labels Jun 26, 2026
@alt-glitch

Copy link
Copy Markdown
Contributor

This was generated by AI during triage.

Duplicate of #53106 — same author, same files (tools/environments/local.py), same _find_shell/$SHELL fix for #42203, opened ~9 minutes earlier. This PR adds more test coverage but the mechanism is identical. Maintainers: pick one (the earlier #53106 is the canonical entry).

#42203)

On macOS, terminal(background=true) silently failed: the process returned a
session_id and exit_code=0 but the command never ran (empty stdout, no side
effects). Root cause is two interacting issues:

1. _find_shell was aliased to _find_bash, which prefers `shutil.which("bash")`
   → /bin/bash (GNU bash 3.2, still shipped on macOS) over $SHELL (/bin/zsh).
2. process_registry.spawn_local runs [shell, "-lic", "set +m; <cmd>"] with
   stdin=/dev/null. bash 3.2 as a login shell sources ~/.bash_profile, which on
   many macOS setups contains `exec /bin/zsh -l`; that exec replaces bash but
   drops the -c argument, so the command is swallowed (exit 0, no output).

Decouple _find_shell from _find_bash: _find_shell now prefers the user's
configured $SHELL on POSIX (the shell they actually log in with), falling back
to _find_bash when $SHELL is unset/missing. _find_bash is unchanged, so callers
that genuinely need bash (e.g. the _run_bash login-shell snapshot) keep bash
semantics. zsh handles -lic correctly even with redirected stdin.

Salvaged from #42219 by @liuhao1024 (authorship preserved via cherry-pick).
On top of the original (8 unit tests covering $SHELL-set/unset/missing/empty,
Windows-ignores-$SHELL, _find_bash-unchanged), added an E2E regression test
that reproduces the real bash-3.2 login-shell swallow (exit 0 / no file) and
asserts the shell _find_shell selects actually executes a -lic background
command. Mutation-verified: reverting _find_shell to the bash alias fails the
$SHELL-preference test. Bug reproduced directly: /bin/bash 3.2 -lic with a
.bash_profile->exec-zsh creates no file; zsh -lic does.

Closes #42203. Supersedes #42290.
@kshitijk4poor
kshitijk4poor force-pushed the salvage/42203-find-shell-prefer-usershell branch from 2f4e9d1 to d9f1f1a Compare June 26, 2026 15:15
@kshitijk4poor
kshitijk4poor merged commit 5038678 into main Jun 26, 2026
27 checks passed
@kshitijk4poor
kshitijk4poor deleted the salvage/42203-find-shell-prefer-usershell branch June 26, 2026 15:41
waefrebeorn pushed a commit to waefrebeorn/slermes that referenced this pull request Jul 2, 2026
…-find-shell-prefer-usershell

fix(terminal): prefer $SHELL over bash for background process spawning (NousResearch#42203)
habarmc1223-sudo pushed a commit to habarmc1223-sudo/hermes-agent-fluxmem that referenced this pull request Jul 8, 2026
…-find-shell-prefer-usershell

fix(terminal): prefer $SHELL over bash for background process spawning (NousResearch#42203)
santhreal pushed a commit to santhreal/hermes-agent that referenced this pull request Jul 13, 2026
…-find-shell-prefer-usershell

fix(terminal): prefer $SHELL over bash for background process spawning (NousResearch#42203)
Gravezzz pushed a commit to Gravezzz/hermes-agent that referenced this pull request Jul 21, 2026
…-find-shell-prefer-usershell

fix(terminal): prefer $SHELL over bash for background process spawning (NousResearch#42203)
leewenjie pushed a commit to leewenjie/hermes-agent that referenced this pull request Aug 7, 2026
…-find-shell-prefer-usershell

fix(terminal): prefer $SHELL over bash for background process spawning (NousResearch#42203)
melon-xf added a commit to melon-xf/hermes-agent that referenced this pull request Sep 3, 2026
…-find-shell-prefer-usershell

fix(terminal): prefer $SHELL over bash for background process spawning (NousResearch#42203)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp/tools Tool registry, model_tools, toolsets duplicate This issue or pull request already exists P1 High — major feature broken, no workaround tool/terminal Terminal execution and process management type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

macOS: background processes (terminal background=true) silently fail — _find_shell returns bash whose login shell (-l) swallows commands

3 participants