Files
hermes-agent/tests/test_managed_runtime_resolution.py
ethernet 25d0bcd424 fix(runtime): resolve Hermes-managed Node and uv before bare PATH
Hermes installs runtimes for itself — `uv` at `$HERMES_HOME/bin/uv`, Node
at `$HERMES_HOME/node` — and neither directory is on an arbitrary
process's PATH. Every `shutil.which("node"/"npm"/"npx"/"uv")` in Hermes's
own code therefore has two failure modes: the managed runtime is invisible,
so the caller reports "not installed" or degrades to a slower tier on a
machine that has exactly what it needed; and when a system copy also
exists, the one Hermes does not own wins.

Routed the Hermes-owned call sites through managed-aware resolvers:

- `agent/lsp/install.py`, `hermes_cli/dep_ensure.py`, `hermes_cli/main.py`
  (`_make_tui_argv`), `hermes_cli/tools_config.py` (`_run_post_setup`) now
  use `find_node_executable()`.
- `hermes_cli/tools_config.py::_pip_install` and `hermes_cli/setup.py`'s
  vercel install use `ensure_uv()` (installing uv is in scope during setup,
  and the Windows installer's `uv venv` does not seed pip, so the fallback
  tier is "No module named pip"). `tools/lazy_deps.py` uses `resolve_uv()`
  — a lookup, not a bootstrap, because it runs mid-turn for an optional
  dependency and downloading a runtime as a side effect exceeds what the
  caller asked for.
- `hermes_cli/gateway.py`: extracted `_append_node_dir_for_service()`,
  shared by the systemd unit and launchd plist generators, which appends
  the managed dirs before the PATH-resolved one. A service definition is
  written once and survives reboots, so resolving a system Node that
  happens to lead the installing shell's PATH bakes the wrong interpreter
  in permanently. Managed dirs are profile-scoped, so each profile's unit
  still names its own Node; the existing symlink-parent rule (don't
  `.resolve()`) is preserved verbatim.
- `tools/environments/local.py`: the terminal tool's subshell PATH gains
  the managed dirs, appended alongside the sane entries rather than
  prepended — a tool the user deliberately put on their own PATH still
  wins, and the managed one only fills a gap. This is also what makes the
  bare `which("uv")` in `tools/env_probe.py` correct: that probe reports
  the environment the *model* sees, and the model can only run what is on
  that subshell's PATH.

`scripts/install.ps1`: the persisted User PATH update becomes
`Set-ManagedNodeFirstOnUserPath`, a move-to-front rather than an
add-if-missing. Installs made by an older install.ps1 already have the
managed dir in User PATH — at the tail, behind a system Node — and an
add-if-missing check sees it present and leaves that ordering in place
forever, so the users the bug hurt would never be repaired. Unrelated
entries keep their relative order (empty segments included; a trailing
`;` is legal and the installer's other PATH code preserves them),
duplicates collapse, and it writes only when the string actually changes.

Tests:

- `tests/test_managed_runtime_resolution.py` — AST guard that fails any
  new bare `which()` for a managed runtime, with a short justified
  allow-list and a companion test that fails when an allow-list entry goes
  stale. Reading source is banned by AGENTS.md and this is the documented
  exception: the property is "no call site anywhere spells it this way",
  which no runtime seam can observe.
- `scripts/ci/test_install_ps1_path_migration.ps1` — behavioral, not a
  source regex: it lifts the real `Set-ManagedNodeFirstOnUserPath` out of
  install.ps1's AST and rewrites only the two registry calls into an
  in-memory store, so the shipped split/dedupe/prepend/change-detection
  logic executes for real. Not in the default lane (Linux runners have no
  PowerShell host); runs under `pwsh`. 13/13 assertions pass.
2026-08-01 21:17:51 -04:00

204 lines
7.5 KiB
Python

"""Guard: Hermes-owned subprocesses must not resolve managed runtimes by bare PATH.
Hermes installs runtimes for itself — ``uv`` at ``$HERMES_HOME/bin/uv``, Node at
``$HERMES_HOME/node``. Neither directory is on the ambient PATH of an arbitrary
process, so ``shutil.which("uv")`` / ``shutil.which("node")`` in Hermes's own
code has two failure modes:
* the managed runtime is invisible, so the caller reports "not installed" or
degrades to a slower tier on a machine that has exactly what it needed; and
* when a system copy also exists, the one Hermes does not own wins — which is
how a generated systemd unit or launchd plist can bake a system Node in and
keep resolving it across reboots.
The fix per call site is one of ``find_node_executable()``,
``iter_hermes_node_dirs()``, ``resolve_uv()``, or ``ensure_uv()``. This test is
the ratchet that stops a new bare lookup from being added back.
Reading source is normally banned (see AGENTS.md). It is the right tool here and
only here: the property under test is "no call site anywhere in the tree spells
it this way", which is a statement about the whole codebase rather than about
one function's behavior, and there is no runtime seam that can observe a lookup
that was never written. Every entry in the allow-list below names a call site
whose behavior is separately covered by a real test.
"""
from __future__ import annotations
import ast
from pathlib import Path
import pytest
REPO_ROOT = Path(__file__).resolve().parents[1]
# Runtimes Hermes provisions into HERMES_HOME and must therefore resolve
# through a managed-aware helper rather than PATH.
_MANAGED_COMMANDS = frozenset({"uv", "node", "npm", "npx"})
# Directories that are not Hermes-owned subprocess code: plugins ship their own
# resolution policy, tests assert against PATH deliberately, and skills/scripts
# run as standalone user-invoked programs.
_EXEMPT_DIRS = (
"tests",
"plugins",
"skills",
"optional-skills",
"scripts",
"website",
"node_modules",
".git",
".venv",
"venv",
".worktrees",
)
# Call sites where a bare PATH lookup is the correct answer. Each entry is
# (path, command) -> why. Keep this list short and justified — the default
# answer for a new call site is a managed-aware helper, not a new exemption.
_ALLOWED: dict[tuple[str, str], str] = {
("tools/env_probe.py", "uv"): (
"Reports the environment the MODEL sees in the terminal tool. The model "
"can only run what is on that subshell's PATH, which local.py populates "
"with the managed dirs — so PATH is the correct question to ask here."
),
("hermes_cli/update_cmd.py", "uv"): (
"Termux fallback: a pkg-installed uv lands on PATH but not in the "
"managed bin dir, and it is checked only after resolve_uv() misses."
),
("hermes_cli/update_cmd.py", "npm"): (
"WSL diagnostic: deliberately inspects what PATH resolves so it can "
"warn that the only reachable npm is the Windows one."
),
("tools/lazy_deps.py", "uv"): (
"Fallback after resolve_uv(), plus the except-branch for the "
"hermes_cli import guard."
),
("hermes_cli/gateway.py", "node"): (
"Fallback rung of _append_node_dir_for_service(), after the managed "
"dirs from iter_hermes_node_dirs() are already appended."
),
("hermes_cli/main.py", "node"): (
"_ensure_tui_node()'s idempotence gate: the question really is 'is "
"node already discoverable on PATH', before bootstrapping one."
),
("hermes_cli/main.py", "npm"): (
"Same _ensure_tui_node() gate as node."
),
("tools/browser_tool.py", "npx"): (
"agent-browser runs via `npx`, resolved against the extended browser "
"PATH that _merge_browser_path() already seeds with the managed dirs."
),
}
def _iter_which_calls(tree: ast.AST):
"""Yield (command, lineno) for every ``which("<cmd>")`` call in *tree*.
AST rather than a regex so prose in docstrings and comments that mentions
``shutil.which("npm")`` is not mistaken for a call site.
"""
for node in ast.walk(tree):
if not isinstance(node, ast.Call) or not node.args:
continue
func = node.func
name = (
func.attr
if isinstance(func, ast.Attribute)
else func.id if isinstance(func, ast.Name) else None
)
if name != "which":
continue
first = node.args[0]
if isinstance(first, ast.Constant) and first.value in _MANAGED_COMMANDS:
yield first.value, node.lineno
def _source_files() -> list[Path]:
files: list[Path] = []
for path in REPO_ROOT.rglob("*.py"):
rel = path.relative_to(REPO_ROOT)
if rel.parts and rel.parts[0] in _EXEMPT_DIRS:
continue
files.append(path)
return files
def _findings() -> list[tuple[str, str, int]]:
"""Return (relpath, command, lineno) for every bare managed lookup."""
found: list[tuple[str, str, int]] = []
for path in _source_files():
try:
source = path.read_text(encoding="utf-8")
except (OSError, UnicodeDecodeError):
continue
if "which(" not in source:
continue
try:
tree = ast.parse(source)
except SyntaxError:
continue
rel = path.relative_to(REPO_ROOT).as_posix()
for command, lineno in _iter_which_calls(tree):
found.append((rel, command, lineno))
return found
def test_no_unreviewed_bare_managed_runtime_lookups():
"""Every bare which() for a managed runtime is a reviewed exemption."""
unexpected = [
(rel, cmd, lineno)
for rel, cmd, lineno in _findings()
if (rel, cmd) not in _ALLOWED
]
assert not unexpected, (
"Bare PATH lookup for a Hermes-managed runtime.\n\n"
+ "\n".join(f" {rel}:{lineno} which({cmd!r})" for rel, cmd, lineno in unexpected)
+ "\n\n$HERMES_HOME/bin (uv) and $HERMES_HOME/node are not on an "
"arbitrary process's PATH, so this resolves a system copy — or nothing "
"— on an install that has a managed one.\n"
"Use instead:\n"
" uv -> managed_uv.resolve_uv() (lookup) or ensure_uv() (may install)\n"
" node/npm -> hermes_constants.find_node_executable()\n"
" PATH env -> hermes_constants.iter_hermes_node_dirs()\n"
"If PATH really is the right question, add the site to _ALLOWED with a "
"reason."
)
def test_allowlist_has_no_stale_entries():
"""A fixed call site must be dropped from the allow-list, not left to rot."""
live = {(rel, cmd) for rel, cmd, _lineno in _findings()}
stale = sorted(set(_ALLOWED) - live)
assert not stale, (
"Allow-list entries no longer match any source line — the call site was "
"fixed or moved. Remove them:\n"
+ "\n".join(f" {rel} ({cmd})" for rel, cmd in stale)
)
@pytest.mark.parametrize(
"helper",
[
"find_node_executable",
"find_hermes_node_executable",
"iter_hermes_node_dirs",
"with_hermes_node_path",
],
)
def test_managed_node_helpers_exist(helper):
"""The alternatives this guard points contributors at must be importable."""
import hermes_constants
assert callable(getattr(hermes_constants, helper))
def test_managed_uv_helpers_exist():
from hermes_cli.managed_uv import ensure_uv, managed_uv_path, resolve_uv
assert callable(resolve_uv)
assert callable(ensure_uv)
assert managed_uv_path().parent.name == "bin"