From b0a25980f89fc42b495d7d6ec17bf879c9b5d5c3 Mon Sep 17 00:00:00 2001 From: Teknium <127238744+teknium1@users.noreply.github.com> Date: Sun, 21 Jun 2026 20:00:06 -0700 Subject: [PATCH] fix(terminal): make hermes install dir reachable in subshell PATH (#50534) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Plugins shelling out to bare `hermes` via the terminal tool hit `command not found` (exit 127) when the gateway was launched without the hermes install dir on PATH (systemd, service managers, cron, desktop launchers) — even though `hermes` works in the user's own interactive terminal, which sources the shell rc that exports that dir. The terminal tool's subshell PATH was the agent process PATH plus a static set of system dirs (_SANE_PATH); it never included wherever the hermes console-script actually lives (~/.local/bin, the venv bin/Scripts, pipx, nix). Resolve that dir once (which/argv0/sys.executable) and prepend-if-missing it so bare `hermes` resolves regardless of launch method. --- tests/tools/test_local_env_blocklist.py | 92 +++++++++++++++++++++++++ tools/environments/local.py | 86 ++++++++++++++++++++++- 2 files changed, 177 insertions(+), 1 deletion(-) diff --git a/tests/tools/test_local_env_blocklist.py b/tests/tools/test_local_env_blocklist.py index 875b8a15ccb..2a016d49f4d 100644 --- a/tests/tools/test_local_env_blocklist.py +++ b/tests/tools/test_local_env_blocklist.py @@ -12,6 +12,8 @@ import os import threading from unittest.mock import MagicMock, patch +import pytest + from tools.environments.local import ( LocalEnvironment, _HERMES_PROVIDER_ENV_BLOCKLIST, @@ -379,6 +381,18 @@ class TestBlocklistCoverage: class TestSanePathIncludesHomebrew: """Verify _SANE_PATH includes macOS Homebrew directories.""" + @pytest.fixture(autouse=True) + def _disable_hermes_bin_injection(self): + """These tests assert the sane-path merge in isolation. Disable the + hermes-install-dir prepend (a separate concern, covered by + TestHermesBinDirOnPath) so a real ``hermes`` on the test runner's PATH + doesn't shift the asserted PATH layout.""" + from tools.environments import local as local_mod + saved = local_mod._HERMES_BIN_DIR + local_mod._HERMES_BIN_DIR = None # resolved -> no dir to inject + yield + local_mod._HERMES_BIN_DIR = saved + def test_sane_path_includes_homebrew_bin(self): from tools.environments.local import _SANE_PATH assert "/opt/homebrew/bin" in _SANE_PATH @@ -471,3 +485,81 @@ class TestSanePathIncludesHomebrew: result = _make_run_env({}) assert result["Path"] == windows_env["Path"] assert "PATH" not in result + + +class TestHermesBinDirOnPath: + """The hermes install dir is reachable in the terminal subshell PATH. + + Plugins shelling out to bare ``hermes`` via the terminal tool must work + even when the gateway was launched without the hermes install dir on + PATH (systemd, service managers, cron). See the discussion that motivated + _resolve_hermes_bin_dir / _prepend_hermes_bin_dir. + """ + + def _reset_cache(self): + from tools.environments import local as local_mod + local_mod._HERMES_BIN_DIR = local_mod._SENTINEL + + def test_resolves_via_which(self, monkeypatch): + from tools.environments import local as local_mod + self._reset_cache() + monkeypatch.setattr(local_mod.shutil, "which", + lambda name: "/opt/hermes/bin/hermes" if name == "hermes" else None) + monkeypatch.setattr(local_mod.os.path, "isdir", lambda p: p == "/opt/hermes/bin") + assert local_mod._resolve_hermes_bin_dir() == "/opt/hermes/bin" + + def test_resolves_via_sys_executable_dir(self, monkeypatch, tmp_path): + from tools.environments import local as local_mod + self._reset_cache() + venv_bin = tmp_path / "venv" / "bin" + venv_bin.mkdir(parents=True) + (venv_bin / "hermes").write_text("#!/bin/sh\n") + monkeypatch.setattr(local_mod.shutil, "which", lambda name: None) + monkeypatch.setattr(local_mod.sys, "argv", ["python"]) + monkeypatch.setattr(local_mod.sys, "executable", str(venv_bin / "python")) + monkeypatch.setattr(local_mod, "_IS_WINDOWS", False) + assert local_mod._resolve_hermes_bin_dir() == str(venv_bin) + + def test_returns_none_when_unresolvable(self, monkeypatch): + from tools.environments import local as local_mod + self._reset_cache() + monkeypatch.setattr(local_mod.shutil, "which", lambda name: None) + monkeypatch.setattr(local_mod.sys, "argv", ["python"]) + monkeypatch.setattr(local_mod.sys, "executable", "/nonexistent/python") + assert local_mod._resolve_hermes_bin_dir() is None + + def test_prepend_adds_missing_dir_at_front(self, monkeypatch): + from tools.environments import local as local_mod + self._reset_cache() + local_mod._HERMES_BIN_DIR = "/opt/hermes/bin" + out = local_mod._prepend_hermes_bin_dir("/usr/bin:/bin") + assert out.split(os.pathsep)[0] == "/opt/hermes/bin" + assert "/usr/bin" in out.split(os.pathsep) + + def test_prepend_is_idempotent(self, monkeypatch): + from tools.environments import local as local_mod + self._reset_cache() + local_mod._HERMES_BIN_DIR = "/opt/hermes/bin" + once = local_mod._prepend_hermes_bin_dir("/usr/bin:/bin") + twice = local_mod._prepend_hermes_bin_dir(once) + assert twice == once + assert once.split(os.pathsep).count("/opt/hermes/bin") == 1 + + def test_prepend_noop_when_unresolved(self, monkeypatch): + from tools.environments import local as local_mod + self._reset_cache() + local_mod._HERMES_BIN_DIR = None + assert local_mod._prepend_hermes_bin_dir("/usr/bin:/bin") == "/usr/bin:/bin" + + def test_make_run_env_injects_hermes_bin_dir(self, monkeypatch): + """A gateway env missing the hermes dir gets it back in the subshell PATH.""" + from tools.environments import local as local_mod + from tools.environments.local import _make_run_env + self._reset_cache() + local_mod._HERMES_BIN_DIR = "/opt/hermes/bin" + monkeypatch.setattr(local_mod, "_IS_WINDOWS", False) + with patch.dict(os.environ, {"PATH": "/usr/bin:/bin"}, clear=True): + result = _make_run_env({}) + entries = result["PATH"].split(os.pathsep) + assert entries[0] == "/opt/hermes/bin" + assert "/usr/bin" in entries diff --git a/tools/environments/local.py b/tools/environments/local.py index b808816ef16..baec8fa2138 100644 --- a/tools/environments/local.py +++ b/tools/environments/local.py @@ -7,6 +7,7 @@ import re import shutil import signal import subprocess +import sys import tempfile import time from pathlib import Path @@ -296,6 +297,85 @@ _SANE_PATH = ( "/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin" ) +# Cached directory containing the ``hermes`` console-script. +# ``_SENTINEL`` distinguishes "not resolved yet" from a resolved ``None``. +_SENTINEL = object() +_HERMES_BIN_DIR: "str | None | object" = _SENTINEL + + +def _resolve_hermes_bin_dir() -> str | None: + """Return the directory holding the ``hermes`` console-script, or None. + + The terminal tool runs in a freshly-spawned subshell whose PATH is the + agent process's PATH plus a static set of system dirs (``_SANE_PATH``). + When the gateway is launched by something that does NOT source the user's + shell rc — systemd, a service manager, a desktop launcher, cron — the + hermes install dir (``~/.local/bin``, the venv ``bin``/``Scripts``, pipx, + nix) is absent from that PATH, so plugins shelling out to bare ``hermes`` + via the terminal tool hit ``command not found`` (exit 127) even though + ``hermes`` works fine in the user's own interactive terminal. + + We resolve the install dir once (it never changes within a process) and + prepend-if-missing it to the subshell PATH so bare ``hermes`` resolves + regardless of how the gateway was started. + + Resolution order (cheap, no heavy imports): + 1. ``shutil.which("hermes")`` — normal PATH-installed shim. + 2. The directory of ``sys.argv[0]`` when it's an absolute path to a + real ``hermes`` executable (covers nix-store / venv wrappers). + 3. The directory of ``sys.executable`` — the running interpreter's + venv ``bin``/``Scripts`` is where its console-scripts live. + """ + global _HERMES_BIN_DIR + if _HERMES_BIN_DIR is not _SENTINEL: + return _HERMES_BIN_DIR # type: ignore[return-value] + + candidate: str | None = None + + which = shutil.which("hermes") + if which: + candidate = os.path.dirname(which) + + if candidate is None: + argv0 = sys.argv[0] if sys.argv else "" + base = os.path.basename(argv0).lower() + if ( + os.path.isabs(argv0) + and (base == "hermes" or base.startswith("hermes.")) + and os.path.isfile(argv0) + ): + candidate = os.path.dirname(argv0) + + if candidate is None: + exe_dir = os.path.dirname(sys.executable) if sys.executable else "" + if exe_dir: + shim = "hermes.exe" if _IS_WINDOWS else "hermes" + if os.path.isfile(os.path.join(exe_dir, shim)): + candidate = exe_dir + + if candidate and not os.path.isdir(candidate): + candidate = None + + _HERMES_BIN_DIR = candidate + return candidate + + +def _prepend_hermes_bin_dir(existing_path: str) -> str: + """Prepend the hermes install dir to ``existing_path`` if it's missing. + + Cross-platform (uses ``os.pathsep``). First-occurrence wins, so a PATH + that already contains the dir is returned unchanged. Returns the input + unchanged when the install dir can't be resolved. + """ + bin_dir = _resolve_hermes_bin_dir() + if not bin_dir: + return existing_path + sep = os.pathsep + entries = [e for e in existing_path.split(sep) if e] if existing_path else [] + if bin_dir in entries: + return existing_path + return sep.join([bin_dir, *entries]) + def _append_missing_sane_path_entries(existing_path: str) -> str: """Return a normalised POSIX PATH with missing sane entries appended. @@ -380,7 +460,11 @@ def _make_run_env(env: dict) -> dict: run_env[k] = v path_key = _path_env_key(run_env) if path_key is not None: - run_env[path_key] = _append_missing_sane_path_entries(run_env.get(path_key, "")) + new_path = _append_missing_sane_path_entries(run_env.get(path_key, "")) + # Ensure the hermes install dir is reachable so plugins can shell out + # to bare ``hermes`` via the terminal tool even when the gateway was + # launched without it on PATH (systemd, service managers, cron, etc.). + run_env[path_key] = _prepend_hermes_bin_dir(new_path) _inject_context_hermes_home(run_env)