From 5a26938aa502ae172a6e6d90ab60ac3fe89c1ad3 Mon Sep 17 00:00:00 2001 From: Teknium <127238744+teknium1@users.noreply.github.com> Date: Thu, 23 Apr 2026 05:15:37 -0700 Subject: [PATCH] fix(terminal): auto-source ~/.profile and ~/.bash_profile so n/nvm PATH survives (#14534) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The environment-snapshot login shell was auto-sourcing only ~/.bashrc when building the PATH snapshot. On Debian/Ubuntu the default ~/.bashrc starts with a non-interactive short-circuit: case $- in *i*) ;; *) return;; esac Sourcing it from a non-interactive shell returns before any PATH export below that guard runs. Node version managers like n and nvm append their PATH line under that guard, so Hermes was capturing a PATH without ~/n/bin — and the terminal tool saw 'node: command not found' even when node was on the user's interactive shell PATH. Expand the auto-source list (when auto_source_bashrc is on) to: ~/.profile → ~/.bash_profile → ~/.bashrc ~/.profile and ~/.bash_profile have no interactivity guard — installers that write their PATH there (n's n-install, nvm's curl installer on most setups) take effect. ~/.bashrc still runs last to preserve behaviour for users who put PATH logic there without the guard. Added two tests covering the new behaviour plus an E2E test that spins up a real LocalEnvironment with a guard-prefixed ~/.bashrc and a ~/.profile PATH export, and verifies the captured snapshot PATH contains the profile entry. --- hermes_cli/config.py | 20 +++-- tests/tools/test_local_shell_init.py | 110 ++++++++++++++++++++++++++- tools/environments/local.py | 20 ++++- 3 files changed, 138 insertions(+), 12 deletions(-) diff --git a/hermes_cli/config.py b/hermes_cli/config.py index 91232dc0d3..6d4c49fd4b 100644 --- a/hermes_cli/config.py +++ b/hermes_cli/config.py @@ -394,17 +394,23 @@ DEFAULT_CONFIG = { # (bash doesn't source bashrc in non-interactive login mode) or # zsh-specific files like ``~/.zshrc`` / ``~/.zprofile``. # Paths support ``~`` / ``${VAR}``. Missing files are silently - # skipped. When empty, Hermes auto-appends ``~/.bashrc`` if the + # skipped. When empty, Hermes auto-sources ``~/.profile``, + # ``~/.bash_profile``, and ``~/.bashrc`` (in that order) if the # snapshot shell is bash (this is the ``auto_source_bashrc`` # behaviour — disable with that key if you want strict login-only # semantics). "shell_init_files": [], - # When true (default), Hermes sources ``~/.bashrc`` in the login - # shell used to build the environment snapshot. This captures - # PATH additions, shell functions, and aliases defined in the - # user's bashrc — which a plain ``bash -l -c`` would otherwise - # miss because bash skips bashrc in non-interactive login mode. - # Turn this off if you have a bashrc that misbehaves when sourced + # When true (default), Hermes sources the user's shell rc files + # (``~/.profile``, ``~/.bash_profile``, ``~/.bashrc``) in the + # login shell used to build the environment snapshot. This + # captures PATH additions, shell functions, and aliases — which a + # plain ``bash -l -c`` would otherwise miss because bash skips + # bashrc in non-interactive login mode, and because a default + # Debian/Ubuntu ``~/.bashrc`` short-circuits on non-interactive + # sources. ``~/.profile`` and ``~/.bash_profile`` are tried first + # because ``n`` / ``nvm`` / ``asdf`` installers typically write + # their PATH exports there without an interactivity guard. Turn + # this off if your rc files misbehave when sourced # non-interactively (e.g. one that hard-exits on TTY checks). "auto_source_bashrc": True, "docker_image": "nikolaik/python-nodejs:python3.11-nodejs20", diff --git a/tests/tools/test_local_shell_init.py b/tests/tools/test_local_shell_init.py index 96e26e7357..7dabaadf12 100644 --- a/tests/tools/test_local_shell_init.py +++ b/tests/tools/test_local_shell_init.py @@ -34,8 +34,59 @@ class TestResolveShellInitFiles: assert resolved == [str(bashrc)] + def test_auto_sources_profile_when_present(self, tmp_path, monkeypatch): + """~/.profile is where ``n`` / ``nvm`` installers typically write + their PATH export on Debian/Ubuntu, and it has no interactivity + guard so a non-interactive source actually runs it. + """ + profile = tmp_path / ".profile" + profile.write_text('export PATH="$HOME/n/bin:$PATH"\n') + monkeypatch.setenv("HOME", str(tmp_path)) + + with patch( + "tools.environments.local._read_terminal_shell_init_config", + return_value=([], True), + ): + resolved = _resolve_shell_init_files() + + assert resolved == [str(profile)] + + def test_auto_sources_bash_profile_when_present(self, tmp_path, monkeypatch): + bash_profile = tmp_path / ".bash_profile" + bash_profile.write_text('export MARKER=bp\n') + monkeypatch.setenv("HOME", str(tmp_path)) + + with patch( + "tools.environments.local._read_terminal_shell_init_config", + return_value=([], True), + ): + resolved = _resolve_shell_init_files() + + assert resolved == [str(bash_profile)] + + def test_auto_sources_profile_before_bashrc(self, tmp_path, monkeypatch): + """Both files present: profile runs first so PATH exports in + profile take effect even if bashrc short-circuits on the + non-interactive ``case $- in *i*) ;; *) return;; esac`` guard. + """ + profile = tmp_path / ".profile" + profile.write_text('export FROM_PROFILE=1\n') + bash_profile = tmp_path / ".bash_profile" + bash_profile.write_text('export FROM_BASH_PROFILE=1\n') + bashrc = tmp_path / ".bashrc" + bashrc.write_text('export FROM_BASHRC=1\n') + monkeypatch.setenv("HOME", str(tmp_path)) + + with patch( + "tools.environments.local._read_terminal_shell_init_config", + return_value=([], True), + ): + resolved = _resolve_shell_init_files() + + assert resolved == [str(profile), str(bash_profile), str(bashrc)] + def test_skips_bashrc_when_missing(self, tmp_path, monkeypatch): - # No bashrc written. + # No rc files written. monkeypatch.setenv("HOME", str(tmp_path)) with patch( @@ -49,6 +100,8 @@ class TestResolveShellInitFiles: def test_auto_source_bashrc_off_suppresses_default(self, tmp_path, monkeypatch): bashrc = tmp_path / ".bashrc" bashrc.write_text('export MARKER=seen\n') + profile = tmp_path / ".profile" + profile.write_text('export MARKER=p\n') monkeypatch.setenv("HOME", str(tmp_path)) with patch( @@ -160,3 +213,58 @@ class TestSnapshotEndToEnd: output = result.get("output", "") assert "PROBE=probe-ok" in output assert "/opt/shell-init-probe/bin" in output + + def test_profile_path_export_survives_bashrc_interactive_guard( + self, tmp_path, monkeypatch + ): + """Reproduces the Debian/Ubuntu + ``n``/``nvm`` case. + + Setup: + - ``~/.bashrc`` starts with ``case $- in *i*) ;; *) return;; esac`` + (the default on Debian/Ubuntu) and would happily export a PATH + entry below that guard — but never gets there because a + non-interactive source short-circuits. + - ``~/.profile`` exports ``$HOME/fake-n/bin`` onto PATH, no guard. + + Expectation: auto-sourced rc list picks up ``~/.profile`` before + ``~/.bashrc``, so the snapshot ends up with ``fake-n/bin`` on PATH + even though the bashrc export is silently skipped. + """ + fake_n_bin = tmp_path / "fake-n" / "bin" + fake_n_bin.mkdir(parents=True) + + profile = tmp_path / ".profile" + profile.write_text( + f'export PATH="{fake_n_bin}:$PATH"\n' + 'export FROM_PROFILE=profile-ok\n' + ) + bashrc = tmp_path / ".bashrc" + bashrc.write_text( + 'case $- in\n' + ' *i*) ;;\n' + ' *) return;;\n' + 'esac\n' + 'export FROM_BASHRC=bashrc-should-not-appear\n' + ) + + monkeypatch.setenv("HOME", str(tmp_path)) + + with patch( + "tools.environments.local._read_terminal_shell_init_config", + return_value=([], True), + ): + env = LocalEnvironment(cwd=str(tmp_path), timeout=15) + try: + result = env.execute( + 'echo "PATH=$PATH"; ' + 'echo "FROM_PROFILE=$FROM_PROFILE"; ' + 'echo "FROM_BASHRC=$FROM_BASHRC"' + ) + finally: + env.cleanup() + + output = result.get("output", "") + assert "FROM_PROFILE=profile-ok" in output + assert str(fake_n_bin) in output + # bashrc short-circuited on the interactive guard — its export never ran + assert "FROM_BASHRC=bashrc-should-not-appear" not in output diff --git a/tools/environments/local.py b/tools/environments/local.py index e4ef27829a..4aa6b64e2d 100644 --- a/tools/environments/local.py +++ b/tools/environments/local.py @@ -247,10 +247,22 @@ def _resolve_shell_init_files() -> list[str]: if explicit: candidates.extend(explicit) elif auto_bashrc and not _IS_WINDOWS: - # Bash's login-shell invocation does NOT source ~/.bashrc by default, - # so tools like nvm / asdf / pyenv that self-install there stay - # invisible to the snapshot without this nudge. - candidates.append("~/.bashrc") + # Build a login-shell-ish source list so tools like n / nvm / asdf / + # pyenv that self-install into the user's shell rc land on PATH in + # the captured snapshot. + # + # ~/.profile and ~/.bash_profile run first because they have no + # interactivity guard — installers like ``n`` and ``nvm`` append + # their PATH export there on most distros, and a non-interactive + # ``. ~/.profile`` picks that up. + # + # ~/.bashrc runs last. On Debian/Ubuntu the default bashrc starts + # with ``case $- in *i*) ;; *) return;; esac`` and exits early + # when sourced non-interactively, which is why sourcing bashrc + # alone misses nvm/n PATH additions placed below that guard. We + # still include it so users who put PATH logic in bashrc (and + # stripped the guard, or never had one) keep working. + candidates.extend(["~/.profile", "~/.bash_profile", "~/.bashrc"]) resolved: list[str] = [] for raw in candidates: