From 8689d49596048345342949dd23572c5d2cddfaba Mon Sep 17 00:00:00 2001 From: Hermes Agent service user Date: Fri, 25 Sep 2026 08:28:52 -0300 Subject: [PATCH] fix(kanban): use install-bound CLI launchers --- gateway/run.py | 27 ++-- hermes_cli/_launchers.py | 11 ++ hermes_cli/kanban_db_dispatch.py | 49 ++++--- hermes_cli/relaunch.py | 33 +++-- .../test_kanban_worker_relaunch_provider.py | 133 ++++++++++++++++++ tests/gateway/test_update_command.py | 27 ++-- tests/hermes_cli/test_kanban_db.py | 32 ++--- .../test_kanban_worker_spawn_toolsets.py | 15 ++ tests/hermes_cli/test_relaunch.py | 132 ++++++++--------- 9 files changed, 301 insertions(+), 158 deletions(-) create mode 100644 tests/e2e/core/kanban/test_kanban_worker_relaunch_provider.py diff --git a/gateway/run.py b/gateway/run.py index e60de6e07d156..169bedcf42313 100644 --- a/gateway/run.py +++ b/gateway/run.py @@ -2910,21 +2910,22 @@ def _get_channel_override( def _resolve_hermes_bin() -> Optional[list[str]]: - """Hermes update/restart argv: the running interpreter's ``python -m hermes_cli.main`` - (exactly this install), else ``hermes`` on PATH, else None. The module argv must win: a - PATH-first lookup lets an attacker-planted ``hermes`` shadow the running install when - /update or /restart re-execs it (#111569).""" + """Build an installation-bound Hermes update/restart command. + + Re-enter the current managed checkout through its durable launcher or + isolated bootstrap before consulting PATH. A plain ``python -m`` loses the + source/dependency roots that the launcher activated, especially after the + process changes CWD or starts from an isolated child environment. + """ try: - import importlib.util - if importlib.util.find_spec("hermes_cli") is not None: - return [sys.executable, "-m", "hermes_cli.main"] + from hermes_cli._launchers import current_installation_command + + return current_installation_command() except Exception: - pass - import shutil - hermes_bin = shutil.which("hermes") - if hermes_bin: - return [hermes_bin] - return None + import shutil + + hermes_bin = shutil.which("hermes") + return [hermes_bin] if hermes_bin else None _PROFILE_ID_KEY_RE = re.compile(r"^[a-z0-9][a-z0-9_-]{0,63}$") diff --git a/hermes_cli/_launchers.py b/hermes_cli/_launchers.py index 8c6464c9ae69c..99490ab221fd7 100644 --- a/hermes_cli/_launchers.py +++ b/hermes_cli/_launchers.py @@ -76,6 +76,17 @@ def installation_command(repo_root: Path, args=(), *, module: str = "hermes_cli. prefix = [] if module == "hermes_cli.main" else ["--run-module", module] return [str(root / ".hermes" / "bin" / "hermes"), *prefix, *args] + +def current_installation_command(args=(), *, module: str = "hermes_cli.main") -> list[str]: + """Build an argv for this imported Hermes checkout without consulting CWD or PATH. + + Managed source installs use their durable local launcher; developer/Nix + checkouts use ``runtime_command``'s isolated bootstrap with this module's + absolute source root. Both paths survive a Kanban workspace CWD and ``-I``. + """ + root = Path(__file__).resolve().parents[1] + return installation_command(root, args, module=module) + #: Launcher command names — keep in lockstep with scripts/install.ps1 #: Publish-UserCommand and hermes_cli/_install_repair.py. WINDOWS_BIN_LAUNCHERS = ("hermes", "hermes-acp") diff --git a/hermes_cli/kanban_db_dispatch.py b/hermes_cli/kanban_db_dispatch.py index 7b71be5569ef0..ba307ebd86122 100644 --- a/hermes_cli/kanban_db_dispatch.py +++ b/hermes_cli/kanban_db_dispatch.py @@ -10,6 +10,7 @@ import contextlib import os import re +import shutil import signal import sqlite3 import subprocess @@ -2442,10 +2443,11 @@ def _rotate_worker_log( pass -def _module_hermes_argv() -> list[str]: - """Interpreter-bound Hermes CLI invocation (``hermes_cli.main`` is the - console-script target — there is no top-level ``hermes`` package).""" - return [sys.executable, "-m", "hermes_cli.main"] +def _current_installation_argv() -> list[str]: + """Bind a child to this Hermes checkout, not the dispatcher's CWD/PATH.""" + from hermes_cli._launchers import current_installation_command + + return current_installation_command() def _absolute_hermes_path(path: str) -> str: @@ -2500,27 +2502,23 @@ def _safe_which_no_cwd(command: str) -> Optional[str]: def _hermes_path_argv(path: str) -> list[str]: """argv for a resolved Hermes executable path. Windows batch shims (``.cmd``/``.bat``) are unsafe as argv[0] because the argument vector - includes task-derived values; prefer the module form.""" + includes task-derived values; use the installation-bound bootstrap instead. + """ if _kb._IS_WINDOWS and _is_windows_batch_shim(path): - return _module_hermes_argv() + return _current_installation_argv() return [_absolute_hermes_path(path)] def _resolve_hermes_argv() -> list[str]: - """Resolve the ``hermes`` invocation as argv for ``Popen``: ``$HERMES_BIN`` - (path-like -> absolute; bare names keep PATH semantics, never a - same-directory file), then the running interpreter's ``sys.executable -m - hermes_cli.main`` (exactly this install; also covers shim-less cron, - systemd ``User=``, launchd), then ``which("hermes")`` (Windows: safe PATH - search, batch shims fall back to the module form) only when ``hermes_cli`` - is not importable. The module argv must win over PATH: a PATH-first lookup - lets an attacker-planted ``hermes`` shadow the running install (#111569). - Mirrors ``gateway.run._resolve_hermes_bin``; local because ``hermes_cli`` - sits below ``gateway`` in the dependency order. + """Resolve the installed Hermes CLI for a dispatcher-spawned worker. + + A valid explicit ``HERMES_BIN`` remains an operator override. Otherwise + use the current source install's durable launcher or ``-I`` bootstrap; + unlike ``sys.executable -m hermes_cli.main``, that preserves import roots + activated by the launcher when the child starts from a Kanban worktree. + PATH is only a last-resort compatibility path when the install bootstrap + cannot be constructed. """ - import importlib.util - import shutil - env_bin = os.environ.get("HERMES_BIN", "").strip() if env_bin: if _looks_like_path(env_bin): @@ -2528,18 +2526,19 @@ def _resolve_hermes_argv() -> list[str]: resolved_env_bin = _safe_which_no_cwd(env_bin) if resolved_env_bin: return _hermes_path_argv(resolved_env_bin) - return _module_hermes_argv() + install_error = None try: - if importlib.util.find_spec("hermes_cli") is not None: - return _module_hermes_argv() - except Exception: - pass + return _current_installation_argv() + except Exception as exc: + install_error = exc hermes_bin = _safe_which_no_cwd("hermes") if _kb._IS_WINDOWS else shutil.which("hermes") if hermes_bin: return _hermes_path_argv(hermes_bin) - return _module_hermes_argv() + raise RuntimeError( + "cannot resolve an installation-bound Hermes CLI; set HERMES_BIN to an executable path" + ) from install_error def _worker_terminal_timeout_env( diff --git a/hermes_cli/relaunch.py b/hermes_cli/relaunch.py index 790ca1026eee3..07dec29f3e3c1 100644 --- a/hermes_cli/relaunch.py +++ b/hermes_cli/relaunch.py @@ -3,7 +3,6 @@ import os import pathlib -import shutil import sys from typing import Optional, Sequence @@ -60,13 +59,14 @@ def _extract_inherited_flags(argv: Sequence[str]) -> list[str]: def resolve_hermes_bin() -> Optional[str]: - """Hermes entry point: ``sys.argv[0]`` if a real executable, else ``which hermes``, else ``None`` - (caller falls back to ``python -m hermes_cli.main``). + """Return an absolute, safe ``sys.argv[0]`` executable, or ``None``. + ``None`` makes relaunch use :func:`hermes_cli._launchers.current_installation_command`, + which is independent of the current directory and inherited PATH/Python paths. Python launchers are never returned: on Windows a ``.py`` can't be exec'd directly, and on POSIX the git installer runs the extensionless source launcher under the managed venv interpreter while its ``#!/usr/bin/env python3`` shebang would pick the system Python and - lose the venv. Falling through to ``sys.executable -m hermes_cli.main`` keeps the venv. + lose the venv. """ argv0 = sys.argv[0] _is_windows = sys.platform == "win32" @@ -83,28 +83,27 @@ def _is_unsafe_python_launcher(p: str) -> bool: return _needs_interpreter(pathlib.Path(p)) - # Absolute executable (nix store, venv wrappers, …), then relative-to-CWD, then PATH. + # Never interpret a bare argv[0] relative to the worker CWD: that directory + # is often an unrelated Kanban worktree which may contain its own `hermes`. if ( os.path.isabs(argv0) and os.path.isfile(argv0) and os.access(argv0, os.X_OK) and not _is_unsafe_python_launcher(argv0) ): return argv0 - if not argv0.startswith("-") and os.path.isfile(argv0): - abs_path = os.path.abspath(argv0) - if os.access(abs_path, os.X_OK) and not _is_unsafe_python_launcher(abs_path): - return abs_path - path_bin = shutil.which("hermes") - if path_bin and not _is_unsafe_python_launcher(path_bin): - return path_bin return None def build_relaunch_argv( extra_args: Sequence[str], *, preserve_inherited: bool = True, original_argv: Optional[Sequence[str]] = None ) -> list[str]: - """Construct an argv list for replacing the current process with hermes.""" + """Construct an argv list for replacing the current process with this Hermes installation.""" bin_path = resolve_hermes_bin() - argv = [bin_path] if bin_path else [sys.executable, "-m", "hermes_cli.main"] + if bin_path: + argv = [bin_path] + else: + from hermes_cli._launchers import current_installation_command + + argv = current_installation_command() src = list(original_argv) if original_argv is not None else list(sys.argv[1:]) if preserve_inherited: argv.extend(_extract_inherited_flags(src)) @@ -129,12 +128,12 @@ def relaunch( except KeyboardInterrupt: sys.exit(130) except OSError as exc: - # Raw ``[Errno 8] Exec format error`` is cryptic; usual causes are ``hermes`` not on - # PATH yet (install hasn't propagated User PATH into this shell) or a stale shim. + # A missing install-bound launcher or a damaged runtime bootstrap is more likely than PATH + # here: managed relaunches no longer depend on ambient command discovery. print( f"\nHermes relaunch failed: {exc}\n" f"Command: {' '.join(new_argv)}\n" - f"Fix: open a new terminal so PATH picks up, then re-run hermes.", + f"Fix: repair the Hermes installation, then re-run the command.", file=sys.stderr, ) sys.exit(1) diff --git a/tests/e2e/core/kanban/test_kanban_worker_relaunch_provider.py b/tests/e2e/core/kanban/test_kanban_worker_relaunch_provider.py new file mode 100644 index 0000000000000..3e340e10d6bab --- /dev/null +++ b/tests/e2e/core/kanban/test_kanban_worker_relaunch_provider.py @@ -0,0 +1,133 @@ +"""A dispatcher retry must keep its model/provider pin and launch from the task workspace. + +The first OpenAI Codex request returns 429; a fresh worker must use the same route, +complete the card, and persist ``billing_provider=openai-codex``. HERMES_BIN is +cleared and PATH contains no Hermes executable so dispatch uses the install-bound +launcher/bootstrap rather than an ambient command. +""" +from __future__ import annotations + +import base64 +import json +import sqlite3 +import sys +import time + +import pytest + +from tests.e2e.core.kanban._helpers import Board +from tests.fakes.fake_llm_provider import Error, FakeLLMServer +from tests.fakes.providers.openai_responses import ( + FakeResponsesServer, FunctionCall, HttpError, Message, Turn, +) + +pytestmark = [ + pytest.mark.skipif(not sys.platform.startswith("linux"), reason="worker liveness uses /proc"), + pytest.mark.live_system_guard_bypass, +] + +MODEL = "gpt-6-luna" +PROVIDER = "openai-codex" + + +def _codex_store() -> tuple[dict, str]: + """One local fake OAuth grant with enough TTL to avoid contacting the token endpoint.""" + def segment(value: dict) -> str: + return base64.urlsafe_b64encode(json.dumps(value).encode()).rstrip(b"=").decode() + + exp = int(time.time()) + 8 * 60 * 60 + access_token = ( + f"{segment({'alg': 'none'})}." + f"{segment({'exp': exp, 'sub': 'e2e-codex-user', 'https://api.openai.com/auth': {'chatgpt_account_id': 'e2e-codex-account'}})}.sig" + ) + tokens = {"access_token": access_token, "refresh_token": "e2e-refresh-not-used"} + refreshed = time.strftime("%Y-%m-%dT%H:%M:%SZ", time.gmtime(time.time() - 60)) + store = { + "version": 1, + "active_provider": PROVIDER, + "providers": {PROVIDER: {"tokens": tokens, "last_refresh": refreshed, "auth_mode": "chatgpt"}}, + "credential_pool": {PROVIDER: [{ + "id": "e2e-codex-login", "label": "device_code", "auth_type": "oauth", + "priority": 0, "source": "device_code", **tokens, "last_refresh": refreshed, + }]}, + } + return store, access_token + + +def _session_routes(board: Board) -> list[tuple[str, str]]: + path = board.hermes_home / "state.db" + if not path.is_file(): + return [] + conn = sqlite3.connect(f"file:{path}?mode=ro", uri=True, timeout=5) + try: + rows = conn.execute( + """SELECT DISTINCT u.model, u.billing_provider + FROM session_model_usage AS u + JOIN sessions AS s ON s.id = u.session_id + WHERE s.source = 'kanban'""" + ).fetchall() + return [(str(model), str(provider)) for model, provider in rows] + finally: + conn.close() + + +def test_worker_relaunch_and_rate_limit_retry_keep_pinned_provider(tmp_path) -> None: + auth_store, bearer = _codex_store() + codex_steps = [ + HttpError(status=429, message="scripted Codex rate limit", retry_after=0), + Turn([FunctionCall("kanban_complete", {"summary": "Codex route survived retry"})]), + Turn([Message("card completed through OpenAI Codex")]), + ] + + def default_route_must_not_be_used(_record: dict): + return Error(429, "default provider was used instead of the task override", retry_after=0) + + with FakeLLMServer(default_route_must_not_be_used) as default_server, \ + FakeResponsesServer(codex_steps, api_key=bearer) as codex_server: + board = Board( + tmp_path, + default_server.base_url, + env_extra={ + "HERMES_BIN": "", + "HERMES_CODEX_BASE_URL": codex_server.base_url, + "PATH": "/usr/bin:/bin", + }, + ) + board.hermes_home.joinpath(".env").write_text( + f"OPENAI_API_KEY=sk-fake-default\nHERMES_CODEX_BASE_URL={codex_server.base_url}\n", + encoding="utf-8", + ) + board.hermes_home.joinpath("auth.json").write_text(json.dumps(auth_store), encoding="utf-8") + try: + task_id = board.create( + "pinned Codex retry probe", + "--model", MODEL, + "--provider", PROVIDER, + ) + + first_tick = board.dispatch() + assert task_id in [row["task_id"] for row in first_tick.get("spawned", [])], board.diag(task_id) + first_pid = int(board.task(task_id)["worker_pid"]) + board.wait_worker_exit(task_id, first_pid) + + second_tick = board.dispatch() + assert task_id in [row["task_id"] for row in second_tick.get("spawned", [])], board.diag(task_id) + second_pid = int(board.task(task_id)["worker_pid"]) + assert second_pid != first_pid + board.wait_worker_exit(task_id, second_pid) + + assert board.task(task_id)["status"] == "done", board.diag(task_id) + outcomes = [run["outcome"] for run in board.runs(task_id)] + assert outcomes[:2] == ["rate_limited", "completed"], board.diag(task_id) + assert default_server.main_requests() == [], "task override fell through to the profile provider" + requests = codex_server.main_requests() + assert len(requests) == 3, board.diag(task_id) + assert codex_server.invalid_requests() == [], codex_server.invalid_requests() + assert all(request.get("model") == MODEL for request in requests), requests + assert all( + request["headers"].get("authorization") == f"Bearer {bearer}" + for request in codex_server.requests if request["kind"] == "main" + ), codex_server.requests + assert (MODEL, PROVIDER) in _session_routes(board), board.diag(task_id) + finally: + board.kill_workers() diff --git a/tests/gateway/test_update_command.py b/tests/gateway/test_update_command.py index 2bc8aa1eba882..79e74dbd9cff0 100644 --- a/tests/gateway/test_update_command.py +++ b/tests/gateway/test_update_command.py @@ -92,32 +92,29 @@ class FakePath(type(Path())): @pytest.mark.asyncio - async def test_resolve_hermes_bin_module_argv(self): - """_resolve_hermes_bin uses the running interpreter's module argv when hermes_cli is - importable, even when PATH also offers a ``hermes`` binary (#111569: a PATH-first - lookup would re-exec an attacker-planted executable on /update and /restart).""" - import sys + async def test_resolve_hermes_bin_prefers_installation_command(self): + """A PATH binary must not shadow the current install on /update or /restart.""" + from hermes_cli import _launchers from gateway.run import _resolve_hermes_bin - fake_spec = MagicMock() - with patch("shutil.which", return_value="/tmp/attacker/hermes"), \ - patch("importlib.util.find_spec", return_value=fake_spec): + with patch.object(_launchers, "current_installation_command", return_value=["/install/hermes"]), \ + patch("shutil.which", return_value="/tmp/attacker/hermes"): result = _resolve_hermes_bin() - assert result == [sys.executable, "-m", "hermes_cli.main"] + assert result == ["/install/hermes"] @pytest.mark.asyncio async def test_resolve_hermes_bin_falls_back_to_path_then_none(self): - """Without an importable hermes_cli the argv degrades to PATH, then to None — never a - bare ``hermes`` string that a hostile PATH entry could shadow.""" + """PATH is consulted only when the install-bound launcher/bootstrap fails.""" + from hermes_cli import _launchers from gateway.run import _resolve_hermes_bin - with patch("shutil.which", return_value="/usr/local/bin/hermes"), \ - patch("importlib.util.find_spec", return_value=None): + with patch.object(_launchers, "current_installation_command", side_effect=RuntimeError), \ + patch("shutil.which", return_value="/usr/local/bin/hermes"): assert _resolve_hermes_bin() == ["/usr/local/bin/hermes"] - with patch("shutil.which", return_value=None), \ - patch("importlib.util.find_spec", side_effect=ImportError): + with patch.object(_launchers, "current_installation_command", side_effect=RuntimeError), \ + patch("shutil.which", return_value=None): assert _resolve_hermes_bin() is None diff --git a/tests/hermes_cli/test_kanban_db.py b/tests/hermes_cli/test_kanban_db.py index c9a8b172cbb9e..c1c3135c16254 100644 --- a/tests/hermes_cli/test_kanban_db.py +++ b/tests/hermes_cli/test_kanban_db.py @@ -1540,45 +1540,29 @@ def test_connect_heals_reduced_tasks_schema_seeded_by_external_harness(kanban_ho # --------------------------------------------------------------------------- # Dispatcher spawn invocation — _resolve_hermes_argv() # -# Workers spawned by the dispatcher must use a `hermes` invocation that does -# not depend on PATH being set up correctly. cron jobs, systemd User= services, -# launchd jobs, and other detached processes routinely run with a stripped -# $PATH that doesn't include the venv's bin/, so a bare `["hermes", ...]` -# spawn fails with FileNotFoundError and the task gets stuck. The resolver -# prefers the interpreter-bound module form (exactly this install; a PATH -# shim could be attacker-planted or belong to another install, #111569) and -# only falls back to the PATH shim when ``hermes_cli`` is not importable. +# Workers must use an installation-bound launcher/bootstrap that survives a +# stripped PATH, an isolated Python child, and a task-workspace CWD. An +# attacker-planted PATH shim must never shadow the currently imported install. # --------------------------------------------------------------------------- -def test_resolve_hermes_argv_prefers_module_form_over_path_shim(monkeypatch): - """A `hermes` on PATH must not shadow the running install (#111569): - the module argv wins whenever ``hermes_cli`` is importable; only an - explicit ``$HERMES_BIN`` overrides it.""" +def test_resolve_hermes_argv_prefers_installation_bootstrap_over_path_shim(monkeypatch): + """A PATH executable cannot shadow this install; explicit HERMES_BIN still wins.""" import shutil - import sys from hermes_cli import kanban_db_dispatch as kbd monkeypatch.delenv("HERMES_BIN", raising=False) monkeypatch.setattr(shutil, "which", lambda name: "/tmp/planted/hermes") monkeypatch.setattr(kbd, "_safe_which_no_cwd", lambda name: "/tmp/planted/hermes") - assert kbd._resolve_hermes_argv() == [sys.executable, "-m", "hermes_cli.main"] + assert kbd._resolve_hermes_argv() == kbd._current_installation_argv() monkeypatch.setenv("HERMES_BIN", "/opt/hermes/bin/hermes") assert kbd._resolve_hermes_argv() == ["/opt/hermes/bin/hermes"] - -def test_resolve_hermes_argv_module_actually_runs(): - """The fallback module name must be importable + runnable. - - A unit test that pins the literal string is necessary but not - sufficient — if `hermes_cli.main` ever loses `if __name__ == "__main__"` - handling or its argparse setup, `python -m hermes_cli.main --version` - would fail and so would every dispatcher spawn that hits the fallback. - Run it as a real subprocess to catch that regression. - """ +def test_resolve_hermes_argv_installation_command_actually_runs(): + """The install-bound launcher/bootstrap resolves the CLI outside its source CWD.""" import subprocess from hermes_cli import kanban_db_dispatch as kbd import shutil diff --git a/tests/hermes_cli/test_kanban_worker_spawn_toolsets.py b/tests/hermes_cli/test_kanban_worker_spawn_toolsets.py index a3c1dce230ac6..3e8848984dca1 100644 --- a/tests/hermes_cli/test_kanban_worker_spawn_toolsets.py +++ b/tests/hermes_cli/test_kanban_worker_spawn_toolsets.py @@ -122,6 +122,7 @@ def fake_popen(cmd, *args, **kwargs): workspace.mkdir() task = _make_task(kb, assignee="elias") task.model_override = "gpt-5.6-sol" + task.provider_override = "openai-codex" kbd._default_spawn(task, str(workspace)) parser, _subparsers, _chat_parser = build_top_level_parser() @@ -133,9 +134,23 @@ def fake_popen(cmd, *args, **kwargs): assert args.command == "chat" assert args.model == "gpt-5.6-sol" + assert args.provider == "openai-codex" assert args.query == "work kanban task t_spawn_tools" +def test_resolve_worker_argv_prefers_current_installation_over_path(monkeypatch, tmp_path): + from hermes_cli import kanban_db_dispatch as kbd + + rogue = tmp_path / "hermes" + rogue.write_text("#!/bin/sh\nexit 99\n", encoding="utf-8") + rogue.chmod(0o755) + monkeypatch.setenv("HERMES_BIN", "") + monkeypatch.setenv("PATH", str(tmp_path)) + monkeypatch.setattr(kbd, "_current_installation_argv", lambda: ["/install/.hermes/bin/hermes"]) + + assert kbd._resolve_hermes_argv() == ["/install/.hermes/bin/hermes"] + + def test_default_spawn_resolves_env_passthrough_under_multiplex(monkeypatch, tmp_path): """Under multiplex a worker spawn with ``terminal.env_passthrough`` configured must forward the ASSIGNEE profile's own value, never crash on an unscoped read or leak the diff --git a/tests/hermes_cli/test_relaunch.py b/tests/hermes_cli/test_relaunch.py index 37e0f358e7681..d6fad35e5dc54 100644 --- a/tests/hermes_cli/test_relaunch.py +++ b/tests/hermes_cli/test_relaunch.py @@ -1,6 +1,9 @@ """Tests for hermes_cli.relaunch — unified self-relaunch utility.""" +import os +import subprocess import sys +import venv import pytest @@ -15,22 +18,22 @@ def test_prefers_absolute_argv0_when_executable(self, monkeypatch): monkeypatch.setattr(relaunch_mod.os, "access", lambda p, mode: p == fake) assert relaunch_mod.resolve_hermes_bin() == fake - def test_resolves_relative_argv0(self, monkeypatch, tmp_path): + def test_does_not_resolve_bare_argv0_from_workspace_cwd(self, monkeypatch, tmp_path): fake = tmp_path / "hermes" fake.write_text("#!/bin/sh\n") fake.chmod(0o755) - monkeypatch.setattr(sys, "argv", [str(fake.name)]) + monkeypatch.setattr(sys, "argv", [fake.name]) monkeypatch.chdir(tmp_path) - # Ensure we don't accidentally match a real 'hermes' on PATH - monkeypatch.setattr(relaunch_mod.shutil, "which", lambda _name: None) - assert relaunch_mod.resolve_hermes_bin() == str(fake) - - def test_falls_back_to_path_which(self, monkeypatch): - monkeypatch.setattr(sys, "argv", ["-c"]) # not a real path - monkeypatch.setattr( - relaunch_mod.shutil, "which", lambda name: "/usr/bin/hermes" if name == "hermes" else None - ) - assert relaunch_mod.resolve_hermes_bin() == "/usr/bin/hermes" + monkeypatch.setenv("PATH", str(tmp_path)) + assert relaunch_mod.resolve_hermes_bin() is None + + def test_does_not_resolve_hermes_from_path(self, monkeypatch, tmp_path): + fake = tmp_path / "hermes" + fake.write_text("#!/bin/sh\n") + fake.chmod(0o755) + monkeypatch.setattr(sys, "argv", ["-c"]) + monkeypatch.setenv("PATH", str(tmp_path)) + assert relaunch_mod.resolve_hermes_bin() is None class TestExtractInheritedFlags: @@ -84,6 +87,45 @@ def test_can_disable_preserve(self, monkeypatch): assert "--tui" not in argv assert argv == ["/usr/bin/hermes", "--resume", "abc"] + def test_fallback_uses_current_installation_command(self, monkeypatch): + from hermes_cli import _launchers + + monkeypatch.setattr(relaunch_mod, "resolve_hermes_bin", lambda: None) + monkeypatch.setattr(_launchers, "current_installation_command", lambda: ["/install/bin/hermes"]) + argv = relaunch_mod.build_relaunch_argv(["--version"], preserve_inherited=False) + assert argv == ["/install/bin/hermes", "--version"] + + def test_isolated_import_failure_recovers_from_foreign_workspace_cwd(self, monkeypatch, tmp_path): + """The old ``python -I -m hermes_cli.main`` fallback cannot see source installed outside + site-packages. The replacement's install-bound bootstrap reaches the real CLI from a + workspace CWD with PATH, PYTHONPATH, and HERMES_BIN deliberately unavailable. + """ + workspace = tmp_path / "workspace" + workspace.mkdir() + clean_env = { + key: value for key, value in os.environ.items() + if key not in {"HERMES_BIN", "PYTHONPATH", "PYTHONHOME", "VIRTUAL_ENV"} + } + clean_env.update({"PATH": "/usr/bin:/bin", "HOME": str(tmp_path / "home"), + "HERMES_HOME": str(tmp_path / "hermes-home")}) + isolated = tmp_path / "isolated-python" + venv.EnvBuilder(with_pip=False, system_site_packages=False).create(isolated) + isolated_python = isolated / ("Scripts/python.exe" if sys.platform == "win32" else "bin/python") + legacy = subprocess.run( + [str(isolated_python), "-I", "-m", "hermes_cli.main", "--version"], + cwd=workspace, env=clean_env, capture_output=True, text=True, timeout=30, + ) + assert legacy.returncode != 0 + assert "No module named 'hermes_cli'" in legacy.stderr + + monkeypatch.setattr(relaunch_mod, "resolve_hermes_bin", lambda: None) + argv = relaunch_mod.build_relaunch_argv(["--version"], preserve_inherited=False) + launched = subprocess.run( + argv, cwd=workspace, env=clean_env, capture_output=True, text=True, timeout=90, + ) + assert launched.returncode == 0, launched.stderr + assert launched.stdout.strip(), launched.stderr + class TestRelaunch: def test_calls_execvp(self, monkeypatch): @@ -183,33 +225,21 @@ class TestResolveHermesBinWindowsPyGuard: """ @pytest.mark.platforms("windows") - def test_windows_rejects_py_argv0_falls_through_to_path(self, monkeypatch, tmp_path): - """On Windows, if sys.argv[0] is a .py file, we must skip the - argv[0] fast-path and fall through to PATH / python -m.""" - # Build a fake .py script that "passes" the isfile + X_OK checks. + def test_windows_rejects_py_argv0_uses_installation_fallback(self, monkeypatch, tmp_path): + """A .py argv0 is rejected; callers then use the install-bound command, never PATH.""" script = tmp_path / "main.py" script.write_text("# stub") - + wrapper = tmp_path / "bin" / "hermes.exe" + wrapper.parent.mkdir() + wrapper.write_text("executable stub") monkeypatch.setattr(relaunch_mod.sys, "argv", [str(script), "chat"]) - # Force PATH lookup to return a hermes.exe so the test doesn't - # exercise the None-fallback path (that's a separate test). - monkeypatch.setattr( - relaunch_mod.shutil, "which", - lambda name: r"C:\venv\Scripts\hermes.exe" if name == "hermes" else None, - ) - - bin_path = relaunch_mod.resolve_hermes_bin() - # Must NOT be the .py — must be the hermes.exe PATH entry. - assert bin_path == r"C:\venv\Scripts\hermes.exe" - - def test_posix_python_launcher_falls_through_to_path(self, monkeypatch, tmp_path): - """A Python source launcher must not be re-executed through its shebang. + monkeypatch.setenv("PATH", str(wrapper.parent)) + assert relaunch_mod.resolve_hermes_bin() is None - The git installer invokes ``hermes`` with the venv interpreter while the - source launcher's shebang uses ``/usr/bin/env python3``. Re-executing - that source file directly can therefore escape the venv and lose core - dependencies such as PyYAML. - """ + def test_posix_python_launcher_falls_through_to_installation_bootstrap( + self, monkeypatch, tmp_path + ): + """A source launcher with a foreign shebang is not exec'd or looked up on PATH.""" if sys.platform == "win32": pytest.skip("POSIX semantics") script = tmp_path / "hermes" @@ -220,31 +250,10 @@ def test_posix_python_launcher_falls_through_to_path(self, monkeypatch, tmp_path wrapper.write_text("#!/usr/bin/env bash\n") wrapper.chmod(0o755) monkeypatch.setattr(relaunch_mod.sys, "argv", [str(script), "chat"]) - monkeypatch.setattr( - relaunch_mod.shutil, "which", - lambda name: str(wrapper) if name == "hermes" else None, - ) - assert relaunch_mod.resolve_hermes_bin() == str(wrapper) - - def test_posix_python_launcher_on_path_falls_back_to_current_python( - self, monkeypatch, tmp_path - ): - """PATH must not re-select the Python launcher rejected from argv[0].""" - if sys.platform == "win32": - pytest.skip("POSIX semantics") - script = tmp_path / "hermes" - script.write_text("#!/usr/bin/env python3\n") - script.chmod(0o755) - monkeypatch.setattr(relaunch_mod.sys, "argv", [str(script), "chat"]) - monkeypatch.setattr( - relaunch_mod.shutil, - "which", - lambda name: str(script) if name == "hermes" else None, - ) - + monkeypatch.setenv("PATH", str(wrapper.parent)) assert relaunch_mod.resolve_hermes_bin() is None - # A console script pinned to the running interpreter keeps the venv: still exec-able. + # A console script pinned to the running interpreter remains a safe absolute executable. pinned = tmp_path / "pinned" / "hermes" pinned.parent.mkdir() pinned.write_text(f"#!{sys.executable}\n") @@ -254,13 +263,8 @@ def test_posix_python_launcher_on_path_falls_back_to_current_python( @pytest.mark.platforms("windows") def test_windows_py_argv0_with_no_hermes_on_path_returns_none(self, monkeypatch, tmp_path): - """Bulletproof fallback: if argv0 is .py on Windows AND hermes.exe - isn't on PATH, return None so the caller falls back to - python -m hermes_cli.main.""" + """If argv0 is .py, return None so relaunch uses the installation bootstrap.""" script = tmp_path / "main.py" script.write_text("# stub") - monkeypatch.setattr(relaunch_mod.sys, "argv", [str(script), "chat"]) - monkeypatch.setattr(relaunch_mod.shutil, "which", lambda name: None) - assert relaunch_mod.resolve_hermes_bin() is None