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.
This commit is contained in:
ethernet
2026-08-01 20:51:51 -04:00
parent f5130f0232
commit 25d0bcd424
13 changed files with 509 additions and 53 deletions
+6 -2
View File
@@ -35,6 +35,7 @@ from pathlib import Path
from typing import Any, Dict, Optional from typing import Any, Dict, Optional
from hermes_cli._subprocess_compat import windows_hide_flags from hermes_cli._subprocess_compat import windows_hide_flags
from hermes_constants import find_node_executable
logger = logging.getLogger("agent.lsp.install") logger = logging.getLogger("agent.lsp.install")
@@ -249,9 +250,12 @@ def _install_npm(
peer deps that npm doesn't auto-pull (typescript-language-server peer deps that npm doesn't auto-pull (typescript-language-server
needs ``typescript`` next to it; intelephense ships standalone). needs ``typescript`` next to it; intelephense ships standalone).
""" """
npm = shutil.which("npm") # Managed npm first: $HERMES_HOME/node is not on an arbitrary process's
# PATH, so a bare which() misses the Node that Hermes installed and
# reports "npm not on PATH" on a machine that has a perfectly good one.
npm = find_node_executable("npm")
if npm is None: if npm is None:
logger.info("[install] cannot install %s: npm not on PATH", pkg) logger.info("[install] cannot install %s: no usable npm found", pkg)
return None return None
staging = hermes_lsp_bin_dir().parent # <HERMES_HOME>/lsp/ staging = hermes_lsp_bin_dir().parent # <HERMES_HOME>/lsp/
install_targets = [pkg] + list(extra_pkgs or []) install_targets = [pkg] + list(extra_pkgs or [])
+5 -2
View File
@@ -21,13 +21,16 @@ import subprocess
import sys import sys
from pathlib import Path from pathlib import Path
from hermes_constants import agent_browser_runnable from hermes_constants import agent_browser_runnable, find_node_executable
from tools.environments.local import hermes_subprocess_env from tools.environments.local import hermes_subprocess_env
_IS_WINDOWS = platform.system() == "Windows" _IS_WINDOWS = platform.system() == "Windows"
_DEP_CHECKS = { _DEP_CHECKS = {
"node": lambda: shutil.which("node") is not None, # find_node_executable() rather than a bare which(): $HERMES_HOME/node is
# not on PATH, so which() would report Node missing on an install that has
# a managed one and trigger a redundant re-install.
"node": lambda: find_node_executable("node") is not None,
"browser": lambda: ( "browser": lambda: (
agent_browser_runnable(shutil.which("agent-browser")) agent_browser_runnable(shutil.which("agent-browser"))
or _has_system_browser() or _has_system_browser()
+39 -24
View File
@@ -2749,6 +2749,43 @@ def _systemd_watchdog_service_fields(
return "notify", f"NotifyAccess=main\nWatchdogSec={seconds}s\n" return "notify", f"NotifyAccess=main\nWatchdogSec={seconds}s\n"
def _append_node_dir_for_service(path_entries: list[str]) -> None:
"""Add the Node directory a generated service unit should use to *path_entries*.
The Hermes-managed Node under ``$HERMES_HOME/node`` goes first when it
exists. A bare ``shutil.which("node")`` cannot be trusted on its own here:
a service unit is written once and then survives reboots, so resolving a
system Node that happens to be ahead on the installing shell's PATH bakes
the wrong interpreter in permanently — the exact failure the desktop
backend spawn was fixed for. Managed dirs are profile-scoped
(``get_hermes_home()``), so each profile's unit still names its own Node.
PATH lookup remains the fallback rung for installs with no managed Node.
"""
from hermes_constants import iter_hermes_node_dirs
for directory in iter_hermes_node_dirs():
entry = str(directory)
if directory.is_dir() and entry not in path_entries:
path_entries.append(entry)
resolved_node = shutil.which("node")
if not resolved_node:
return
# Use the directory where ``node`` is *found on PATH*, NOT the symlink's
# resolved target. ``~/.local/bin/node`` is often a symlink into a
# specific profile's node install (e.g. profiles/jarvis/node/bin/node);
# calling .resolve() here would chase that symlink and bake one profile's
# node path into *every* profile's service unit. That cross-profile leak
# makes systemd_unit_is_current() perpetually false, so each gateway
# rewrites its unit + daemon-reload on every boot. Using the symlink's own
# parent keeps the generated unit profile-agnostic.
resolved_node_dir = str(Path(resolved_node).parent)
if resolved_node_dir not in path_entries:
path_entries.append(resolved_node_dir)
def generate_systemd_unit(system: bool = False, run_as_user: str | None = None) -> str: def generate_systemd_unit(system: bool = False, run_as_user: str | None = None) -> str:
python_path = get_python_path() python_path = get_python_path()
working_dir = _stable_service_working_dir() working_dir = _stable_service_working_dir()
@@ -2756,19 +2793,7 @@ def generate_systemd_unit(system: bool = False, run_as_user: str | None = None)
venv_dir = str(detected_venv) if detected_venv else str(PROJECT_ROOT / "venv") venv_dir = str(detected_venv) if detected_venv else str(PROJECT_ROOT / "venv")
path_entries = _build_service_path_dirs() path_entries = _build_service_path_dirs()
resolved_node = shutil.which("node") _append_node_dir_for_service(path_entries)
if resolved_node:
# Use the directory where ``node`` is *found on PATH*, NOT the
# symlink's resolved target. ``~/.local/bin/node`` is often a symlink
# into a specific profile's node install (e.g. profiles/jarvis/node/
# bin/node); calling .resolve() here would chase that symlink and bake
# one profile's node path into *every* profile's service unit. That
# cross-profile leak makes systemd_unit_is_current() perpetually false,
# so each gateway rewrites its unit + daemon-reload on every boot. Using
# the symlink's own parent keeps the generated unit profile-agnostic.
resolved_node_dir = str(Path(resolved_node).parent)
if resolved_node_dir not in path_entries:
path_entries.append(resolved_node_dir)
common_bin_paths = [ common_bin_paths = [
"/usr/local/sbin", "/usr/local/sbin",
@@ -3961,17 +3986,7 @@ def generate_launchd_plist() -> str:
# Resolve the directory containing the node binary (e.g. Homebrew, nvm) # Resolve the directory containing the node binary (e.g. Homebrew, nvm)
# so it's explicitly in PATH even if the user's shell PATH changes later. # so it's explicitly in PATH even if the user's shell PATH changes later.
priority_dirs = _build_service_path_dirs() priority_dirs = _build_service_path_dirs()
resolved_node = shutil.which("node") _append_node_dir_for_service(priority_dirs)
if resolved_node:
# Use the directory where ``node`` is *found on PATH*, NOT the symlink's
# resolved target. ``~/.local/bin/node`` is often a symlink into a
# specific profile's node install; calling .resolve() would chase it and
# bake one profile's path into every profile's service definition,
# breaking profile isolation and causing perpetual unit rewrites. See
# the matching fix in generate_systemd_unit().
resolved_node_dir = str(Path(resolved_node).parent)
if resolved_node_dir not in priority_dirs:
priority_dirs.append(resolved_node_dir)
sane_path = ":".join( sane_path = ":".join(
dict.fromkeys( dict.fromkeys(
priority_dirs + [p for p in os.environ.get("PATH", "").split(":") if p] priority_dirs + [p for p in os.environ.get("PATH", "").split(":") if p]
+8 -2
View File
@@ -1938,12 +1938,18 @@ def _make_tui_argv(tui_dir: Path, tui_dev: bool) -> tuple[list[str], Path]:
env_node = os.environ.get("HERMES_NODE") env_node = os.environ.get("HERMES_NODE")
if env_node and os.path.isfile(env_node) and os.access(env_node, os.X_OK): if env_node and os.path.isfile(env_node) and os.access(env_node, os.X_OK):
return env_node return env_node
path = shutil.which(bin) # find_node_executable() prefers the managed $HERMES_HOME/node tree,
# which is not on PATH — a bare which() would declare "node not found"
# and exit on an install whose only Node is the one Hermes installed,
# and would pick a system Node over the managed one when both exist.
from hermes_constants import find_node_executable
path = find_node_executable(bin)
if not path and bin == "node": if not path and bin == "node":
try: try:
from hermes_cli.dep_ensure import ensure_dependency from hermes_cli.dep_ensure import ensure_dependency
if ensure_dependency("node"): if ensure_dependency("node"):
path = shutil.which("node") path = find_node_executable("node")
except Exception: except Exception:
pass pass
if not path: if not path:
+8 -1
View File
@@ -1563,7 +1563,14 @@ def setup_terminal_backend(config: dict):
print_info("Installing vercel SDK...") print_info("Installing vercel SDK...")
import subprocess import subprocess
uv_bin = shutil.which("uv") # Managed uv first: $HERMES_HOME/bin is never on PATH, so a bare
# which() misses the uv Hermes installed. Bootstrapping one is
# welcome here — this is the interactive setup wizard, already
# mid-install, and the alternative tier is a pip that a `uv venv`
# venv may not even have.
from hermes_cli.managed_uv import ensure_uv
uv_bin = ensure_uv()
if uv_bin: if uv_bin:
result = subprocess.run( result = subprocess.run(
[uv_bin, "pip", "install", "--python", sys.executable, "vercel"], [uv_bin, "pip", "install", "--python", sys.executable, "vercel"],
+18 -5
View File
@@ -790,7 +790,15 @@ def _pip_install(
venv_root = Path(sys.executable).parent.parent venv_root = Path(sys.executable).parent.parent
uv_env = {**os.environ, "VIRTUAL_ENV": str(venv_root)} uv_env = {**os.environ, "VIRTUAL_ENV": str(venv_root)}
uv_bin = shutil.which("uv") # Managed uv first: $HERMES_HOME/bin is never on PATH, so a bare which()
# misses the uv Hermes installed and prefers a system one when both exist.
# ensure_uv() rather than a pure lookup because this runs during setup,
# where installing uv is in scope — and tier 2 is a pip that the Windows
# installer's `uv venv` does not seed, so failing to find uv here is the
# difference between a working post-setup hook and "No module named pip".
from hermes_cli.managed_uv import ensure_uv
uv_bin = ensure_uv()
if uv_bin: if uv_bin:
try: try:
result = subprocess.run( result = subprocess.run(
@@ -1608,10 +1616,15 @@ def _run_cua_driver_installer(
def _run_post_setup(post_setup_key: str): def _run_post_setup(post_setup_key: str):
"""Run post-setup hooks for tools that need extra installation steps.""" """Run post-setup hooks for tools that need extra installation steps."""
import shutil import shutil
from hermes_constants import find_node_executable
if post_setup_key in {"agent_browser", "browserbase"}: if post_setup_key in {"agent_browser", "browserbase"}:
node_modules = PROJECT_ROOT / "node_modules" / "agent-browser" node_modules = PROJECT_ROOT / "node_modules" / "agent-browser"
npm_bin = shutil.which("npm") # Managed Node first — $HERMES_HOME/node is not on PATH, so a bare
npx_bin = shutil.which("npx") # which() reports "no npm" on installs whose only Node is the one
# Hermes installed for exactly this toolchain.
npm_bin = find_node_executable("npm")
npx_bin = find_node_executable("npx")
# Step 1: install the agent-browser npm package into node_modules/ # Step 1: install the agent-browser npm package into node_modules/
if not node_modules.exists() and npm_bin: if not node_modules.exists() and npm_bin:
_print_info(" Installing Node.js dependencies for browser tools...") _print_info(" Installing Node.js dependencies for browser tools...")
@@ -1729,7 +1742,7 @@ def _run_post_setup(post_setup_key: str):
elif post_setup_key == "camofox": elif post_setup_key == "camofox":
camofox_dir = PROJECT_ROOT / "node_modules" / "@askjo" / "camofox-browser" camofox_dir = PROJECT_ROOT / "node_modules" / "@askjo" / "camofox-browser"
_npm_bin = shutil.which("npm") _npm_bin = find_node_executable("npm")
if camofox_dir.exists(): if camofox_dir.exists():
_print_success(" Camofox already installed, nothing to do") _print_success(" Camofox already installed, nothing to do")
elif _npm_bin: elif _npm_bin:
@@ -1751,7 +1764,7 @@ def _run_post_setup(post_setup_key: str):
_print_info(" npx @askjo/camofox-browser") _print_info(" npx @askjo/camofox-browser")
_print_info(" First run downloads the Camoufox engine (~300MB)") _print_info(" First run downloads the Camoufox engine (~300MB)")
_print_info(" Or use Docker: docker run -p 9377:9377 -e CAMOFOX_PORT=9377 jo-inc/camofox-browser") _print_info(" Or use Docker: docker run -p 9377:9377 -e CAMOFOX_PORT=9377 jo-inc/camofox-browser")
elif not shutil.which("npm"): elif not _npm_bin:
_print_warning(" Node.js not found. Install Camofox via Docker:") _print_warning(" Node.js not found. Install Camofox via Docker:")
_print_info(" docker run -p 9377:9377 -e CAMOFOX_PORT=9377 jo-inc/camofox-browser") _print_info(" docker run -p 9377:9377 -e CAMOFOX_PORT=9377 jo-inc/camofox-browser")
+3 -2
View File
@@ -294,8 +294,9 @@ def iter_hermes_node_dirs(home: Path | None = None) -> list[Path]:
dirs = [root / "node"] dirs = [root / "node"]
bin_dir = root / "node" / "bin" bin_dir = root / "node" / "bin"
# NOTE: keep this ordering in sync with hermesManagedNodePathEntries() in # NOTE: keep this ordering in sync with hermesManagedNodePathEntries() in
# apps/desktop/electron/main.cjs — the Electron main process is Node and # apps/desktop/electron/backend-env.ts — the Electron main process is Node
# cannot import this module, so the platform-ordering rule is mirrored there. # and cannot import this module, so the platform-ordering rule is mirrored
# there (once; main.ts imports it rather than keeping its own copy).
if sys.platform == "win32": if sys.platform == "win32":
return dirs + [bin_dir] return dirs + [bin_dir]
return [bin_dir] + dirs return [bin_dir] + dirs
@@ -0,0 +1,125 @@
# Behavioral test for install.ps1's persisted-User-PATH migration.
#
# Run: pwsh -NoProfile -File scripts/ci/test_install_ps1_path_migration.ps1
#
# Not wired into the default CI lane — the Linux runners have no PowerShell
# host. It runs on any machine with pwsh (including via nixpkgs#powershell),
# and on a Windows runner if one is ever added.
#
# This is NOT a source-regex test. It parses install.ps1, lifts the real
# Set-ManagedNodeFirstOnUserPath body out of the AST, and rewrites *only* the
# two registry calls into an in-memory store so the actual shipped logic —
# split, dedupe, prepend, change-detection — executes for real. Rewriting from
# the AST rather than hand-copying the body means the test cannot silently
# drift away from the function it claims to cover.
Set-StrictMode -Version Latest
$ErrorActionPreference = 'Stop'
$installPs1 = Join-Path $PSScriptRoot '..' 'install.ps1' | Resolve-Path
$ast = [System.Management.Automation.Language.Parser]::ParseFile(
$installPs1, [ref]$null, [ref]$null)
$fn = $ast.Find({
param($n)
$n -is [System.Management.Automation.Language.FunctionDefinitionAst] -and
$n.Name -eq 'Set-ManagedNodeFirstOnUserPath'
}, $true)
if (-not $fn) {
throw "Set-ManagedNodeFirstOnUserPath not found in $installPs1"
}
# Swap the two registry calls for the in-memory store. Both must match, or the
# function has changed shape and this harness is no longer exercising it.
# Rewrite the whole definition extent (which already carries `function <name>
# { param(...) ... }`) so the shipped param block and body run verbatim.
$definition = $fn.Extent.Text
$reads = ([regex]'\[Environment\]::GetEnvironmentVariable\("Path", "User"\)').Matches($definition).Count
$writes = ([regex]'\[Environment\]::SetEnvironmentVariable\("Path", ([^,]+), "User"\)').Matches($definition).Count
if ($reads -ne 1 -or $writes -ne 1) {
throw "expected exactly one User PATH read and one write in the function body; found $reads read(s), $writes write(s). Update this harness."
}
$definition = $definition -replace `
'\[Environment\]::GetEnvironmentVariable\("Path", "User"\)', '$script:FakeUserPath'
$definition = $definition -replace `
'\[Environment\]::SetEnvironmentVariable\("Path", ([^,]+), "User"\)', '$script:FakeUserPath = $1; $script:FakeWrites++'
Invoke-Expression $definition
$NODE = 'C:\Users\me\AppData\Local\hermes\node'
$script:Failures = 0
function Invoke-Migration {
param([string]$Start, [string]$NodeDir = $NODE)
$script:FakeUserPath = $Start
$script:FakeWrites = 0
Set-ManagedNodeFirstOnUserPath $NodeDir
}
function Assert-Equal {
param($Expected, $Actual, [string]$Name)
if ($Expected -ceq $Actual) {
Write-Host " PASS $Name"
} else {
Write-Host " FAIL $Name"
Write-Host " expected: [$Expected]"
Write-Host " actual: [$Actual]"
$script:Failures++
}
}
Write-Host "install.ps1 Set-ManagedNodeFirstOnUserPath"
# The regression this function exists for: an install made by an older
# install.ps1, which *appended*. A system Node leads and the managed dir is
# stranded at the tail, so every new shell resolves the wrong node.exe. An
# add-if-missing check would see the entry present and leave it there forever.
Invoke-Migration "C:\Program Files\nodejs;C:\Users\me\bin;$NODE"
Assert-Equal "$NODE;C:\Program Files\nodejs;C:\Users\me\bin" $script:FakeUserPath `
'upgrade from appending installer: managed dir becomes first entry'
Assert-Equal 1 (@($script:FakeUserPath -split ';' | Where-Object { $_ -eq $NODE }).Count) `
'upgrade: managed dir is not duplicated'
Assert-Equal "C:\Program Files\nodejs;C:\Users\me\bin" `
(($script:FakeUserPath -split ';' | Where-Object { $_ -ne $NODE }) -join ';') `
'upgrade: unrelated entries keep their relative order'
Assert-Equal 1 $script:FakeWrites 'upgrade: persists exactly once'
Invoke-Migration "$NODE;C:\Program Files\nodejs"
Assert-Equal "$NODE;C:\Program Files\nodejs" $script:FakeUserPath 'already correct: unchanged'
Assert-Equal 0 $script:FakeWrites 'already correct: no registry write'
Invoke-Migration "C:\Program Files\nodejs"
Assert-Equal "$NODE;C:\Program Files\nodejs" $script:FakeUserPath 'fresh install: prepended'
# Empty segments are legal in a real User PATH (a trailing ';' is common) and
# the installer's other PATH code preserves them. Migration must not quietly
# rewrite parts of PATH it was not asked to touch.
Invoke-Migration "C:\Program Files\nodejs;;C:\Users\me\bin;"
Assert-Equal "$NODE;C:\Program Files\nodejs;;C:\Users\me\bin;" $script:FakeUserPath `
'empty segments are preserved'
# Windows paths are case-insensitive, and -ne on strings is too.
Invoke-Migration "C:\Program Files\nodejs;c:\users\me\appdata\local\HERMES\Node"
Assert-Equal "$NODE;C:\Program Files\nodejs" $script:FakeUserPath `
'existing entry in different case is replaced, not duplicated'
Invoke-Migration "$NODE;C:\Program Files\nodejs;$NODE"
Assert-Equal "$NODE;C:\Program Files\nodejs" $script:FakeUserPath 'duplicates collapse'
Invoke-Migration ""
Assert-Equal $NODE $script:FakeUserPath 'empty User PATH'
Invoke-Migration "C:\Program Files\nodejs" ""
Assert-Equal "C:\Program Files\nodejs" $script:FakeUserPath 'empty NodeDir is a no-op'
Assert-Equal 0 $script:FakeWrites 'empty NodeDir does not write'
if ($script:Failures -gt 0) {
Write-Host ""
Write-Host "$script:Failures assertion(s) failed"
exit 1
}
Write-Host ""
Write-Host "all assertions passed"
+40 -14
View File
@@ -528,6 +528,41 @@ function Ensure-NodeExeOnPath {
return $true return $true
} }
# Put the Hermes-managed Node dir at the FRONT of the persisted User PATH.
#
# Appending is not enough: it leaves a pre-existing system Node ahead of the
# bundled one in every new shell, so anything launched without a curated
# environment (a standalone hermes-setup.exe run, a user typing `npm`) silently
# resolves the wrong Node. Bundled must win.
#
# Move-to-front rather than add-if-missing, because installs made by an older
# install.ps1 already have this dir in User PATH -- at the tail. An
# add-if-missing check sees it present and leaves the broken ordering in place
# forever, so the very users the ordering bug hurt would never be repaired.
#
# Unrelated entries keep their relative order, including empty segments (a
# trailing ';' is legal and common in a real User PATH; Install-Git's splitting
# preserves them too, so this must not quietly rewrite them). Duplicate
# occurrences of the managed dir collapse into the single leading entry.
# PowerShell's -ne is case-insensitive for strings, which is the right
# comparison on Windows. Persists only when the resulting string differs, so
# an already-correct PATH costs one registry read and no write.
function Set-ManagedNodeFirstOnUserPath {
param([string]$NodeDir)
if (-not $NodeDir) { return }
$userPath = [Environment]::GetEnvironmentVariable("Path", "User")
$items = if ($userPath) { @($userPath -split ";") } else { @() }
$rest = @($items | Where-Object { $_ -ne $NodeDir })
$updated = (@($NodeDir) + $rest) -join ";"
if ($updated -ne $userPath) {
[Environment]::SetEnvironmentVariable("Path", $updated, "User")
}
}
# Re-discover uv without re-installing it. Cross-process stage drivers # Re-discover uv without re-installing it. Cross-process stage drivers
# (the desktop GUI's onboarding wizard, CI step-runners) invoke each stage # (the desktop GUI's onboarding wizard, CI step-runners) invoke each stage
# in a fresh powershell process, so $script:UvCmd set by Install-Uv in a # in a fresh powershell process, so $script:UvCmd set by Install-Uv in a
@@ -1090,6 +1125,7 @@ function Test-Node {
if ((Test-Path $managedNode) -and (Test-NodeVersionOk (& $managedNode --version))) { if ((Test-Path $managedNode) -and (Test-NodeVersionOk (& $managedNode --version))) {
$version = & $managedNode --version $version = & $managedNode --version
$env:Path = "$HermesHome\node;$env:Path" $env:Path = "$HermesHome\node;$env:Path"
Set-ManagedNodeFirstOnUserPath "$HermesHome\node"
Write-Success "Node.js $version found (Hermes-managed)" Write-Success "Node.js $version found (Hermes-managed)"
$script:HasNode = $true $script:HasNode = $true
return $true return $true
@@ -1132,20 +1168,10 @@ function Test-Node {
# Persist to User PATH so fresh shells (and future stages # Persist to User PATH so fresh shells (and future stages
# in cross-process driver mode) see it. Matches the # in cross-process driver mode) see it. Matches the
# pattern Install-Git uses for PortableGit. # pattern Install-Git uses for PortableGit. See
# # Set-ManagedNodeFirstOnUserPath for why this is a
# PREPEND, don't append. Appending leaves a pre-existing # move-to-front and not an add-if-missing.
# system Node ahead of the bundled one in every new shell, Set-ManagedNodeFirstOnUserPath "$HermesHome\node"
# so anything launched without a curated environment (a
# standalone hermes-setup.exe run, a user typing `npm`)
# silently resolves the wrong Node. Bundled must win.
$nodeDir = "$HermesHome\node"
$userPath = [Environment]::GetEnvironmentVariable("Path", "User")
$userPathItems = if ($userPath) { $userPath -split ";" } else { @() }
if ($userPathItems -notcontains $nodeDir) {
$userPathItems = @($nodeDir) + $userPathItems
[Environment]::SetEnvironmentVariable("Path", ($userPathItems -join ";"), "User")
}
$version = & "$HermesHome\node\node.exe" --version $version = & "$HermesHome\node\node.exe" --version
Write-Success "Node.js $version installed to $HermesHome\node\ (portable, user-scoped)" Write-Success "Node.js $version installed to $HermesHome\node\ (portable, user-scoped)"
+203
View File
@@ -0,0 +1,203 @@
"""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"
+6
View File
@@ -203,6 +203,12 @@ def _build_probe_line() -> str:
py3_has_pip = _has_pip_module("python3") if py3_ver else False py3_has_pip = _has_pip_module("python3") if py3_ver else False
pip_bound_to = _pip_python_version() pip_bound_to = _pip_python_version()
py3_pep668 = _detect_pep668("python3") if py3_ver else False py3_pep668 = _detect_pep668("python3") if py3_ver else False
# Bare which() is correct here, unlike Hermes's own uv call sites: this
# reports the environment *the model will see* in the terminal tool, and
# what the model can type is exactly what is on that subshell's PATH.
# local.py puts the Hermes-managed $HERMES_HOME/bin there, so a managed-only
# install answers yes — without that, claiming uv the model cannot invoke
# would be worse than claiming none.
has_uv = shutil.which("uv") is not None has_uv = shutil.which("uv") is not None
# If python3 exists, has pip, has uv (or no PEP 668), and there's no # If python3 exists, has pip, has uv (or no PEP 668), and there's no
+36
View File
@@ -1127,6 +1127,34 @@ def _prepend_hermes_bin_dir(existing_path: str) -> str:
return sep.join([bin_dir, *entries]) return sep.join([bin_dir, *entries])
def _managed_runtime_path_entries() -> list[str]:
"""Return existing Hermes-managed runtime dirs for the terminal subshell PATH.
The terminal tool spawns a subshell whose PATH is the agent process's PATH
plus ``_SANE_PATH``. Neither carries the runtimes Hermes installs for
itself, so on a machine where Hermes provisioned its own toolchain a
command the agent runs resolves a system copy instead — or nothing at all:
- ``$HERMES_HOME/node`` (+ ``/bin``) — installed to satisfy the desktop and
browser toolchain. ``tools/browser_tool.py`` already does this for its own
subprocesses; the agent's shell deserves the same.
- ``$HERMES_HOME/bin`` — the managed ``uv``. ``install.sh`` writes it there
and nothing has ever put that directory on PATH, so an install whose only
uv is the managed one looks uv-less to both the agent and the model.
Resolved per call rather than cached in a module constant because
``get_hermes_home()`` is profile-scoped and a managed tree can appear
mid-process (``heal_hermes_managed_node``, a first browser install).
"""
try:
from hermes_constants import get_hermes_home, iter_hermes_node_dirs
candidates = [*iter_hermes_node_dirs(), get_hermes_home() / "bin"]
return [str(d) for d in candidates if d.is_dir()]
except Exception:
return []
def _append_missing_sane_path_entries(existing_path: str) -> str: def _append_missing_sane_path_entries(existing_path: str) -> str:
"""Return a normalised POSIX PATH with missing sane entries appended. """Return a normalised POSIX PATH with missing sane entries appended.
@@ -1144,6 +1172,11 @@ def _append_missing_sane_path_entries(existing_path: str) -> str:
- **Duplicates are collapsed** (first occurrence wins), so a caller PATH - **Duplicates are collapsed** (first occurrence wins), so a caller PATH
that already contains repeats is not propagated verbatim. that already contains repeats is not propagated verbatim.
Hermes-managed runtime dirs are appended alongside the sane entries, not
prepended: a tool the user deliberately put on their own PATH still wins,
and the managed one only fills the gap where there would otherwise be
nothing.
For a well-formed PATH (no empties, no duplicates) the leading segment is For a well-formed PATH (no empties, no duplicates) the leading segment is
byte-identical to the input and ordering is preserved; only the missing byte-identical to the input and ordering is preserved; only the missing
sane entries are appended. On Windows this is a no-op passthrough (the sane entries are appended. On Windows this is a no-op passthrough (the
@@ -1153,6 +1186,9 @@ def _append_missing_sane_path_entries(existing_path: str) -> str:
return existing_path return existing_path
sane_entries = [entry for entry in _SANE_PATH.split(":") if entry] sane_entries = [entry for entry in _SANE_PATH.split(":") if entry]
sane_entries.extend(
entry for entry in _managed_runtime_path_entries() if entry not in sane_entries
)
if not existing_path: if not existing_path:
return ":".join(sane_entries) return ":".join(sane_entries)
+12 -1
View File
@@ -732,7 +732,18 @@ def _venv_pip_install(specs: tuple[str, ...], *, timeout: int = 300) -> _Install
uv_env["VIRTUAL_ENV"] = str(venv_root) uv_env["VIRTUAL_ENV"] = str(venv_root)
# Tier 1: uv (preferred — fast, doesn't need pip in the venv) # Tier 1: uv (preferred — fast, doesn't need pip in the venv)
uv_bin = shutil.which("uv") # Managed uv first: $HERMES_HOME/bin is never on PATH, so a bare
# which() misses the uv Hermes installed and falls through to the
# slower pip tier. Deliberately a lookup and not ensure_uv(): this runs
# mid-turn to install an optional dependency, and downloading uv +
# migrating the Python runtime as a side effect of that is a far bigger
# action than the caller asked for. Tier 2 pip covers the no-uv case.
try:
from hermes_cli.managed_uv import resolve_uv
uv_bin = resolve_uv() or shutil.which("uv")
except Exception:
uv_bin = shutil.which("uv")
if uv_bin: if uv_bin:
try: try:
r = subprocess.run( r = subprocess.run(