From 6423e6d1280b1a255d6f8773c3d564cda6e664e2 Mon Sep 17 00:00:00 2001 From: Bruce-anle <840596168@qq.com> Date: Sat, 20 Jun 2026 11:45:45 +0800 Subject: [PATCH 1/3] fix(install): keep managed node private MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 背景: Hermes POSIX installers linked managed node/npm/npx into user-global command directories, which shadowed user Node managers and persisted after update. 变更点: Add a shared Python node_runtime helper, remove shell symlink creation, add process-local PATH augmentation, and clean legacy Hermes-owned node links during update, TUI bootstrap, dependency ensure, and uninstall. 测试或冒烟结果: - /home/brucean/.hermes/hermes-agent/venv/bin/python -m pytest tests/hermes_cli/test_node_runtime.py tests/hermes_cli/test_dep_ensure.py tests/hermes_cli/test_tui_resume_flow.py tests/hermes_cli/test_cmd_update.py tests/hermes_cli/test_uninstall_node_symlinks.py tests/test_install_sh_node_global_prefix.py -q - bash -n scripts/install.sh - bash -n scripts/lib/node-bootstrap.sh - git diff --cached --check --- hermes_cli/dep_ensure.py | 48 ++++- hermes_cli/main.py | 63 +++++-- hermes_cli/node_runtime.py | 197 ++++++++++++++++++++ hermes_cli/uninstall.py | 42 +---- scripts/install.sh | 81 +++++--- scripts/lib/node-bootstrap.sh | 86 ++++++--- tests/hermes_cli/test_cmd_update.py | 59 +++++- tests/hermes_cli/test_dep_ensure.py | 82 +++++++- tests/hermes_cli/test_node_runtime.py | 108 +++++++++++ tests/hermes_cli/test_tui_resume_flow.py | 37 ++++ tests/test_install_sh_node_global_prefix.py | 60 ++++-- 11 files changed, 728 insertions(+), 135 deletions(-) create mode 100644 hermes_cli/node_runtime.py create mode 100644 tests/hermes_cli/test_node_runtime.py diff --git a/hermes_cli/dep_ensure.py b/hermes_cli/dep_ensure.py index 848e402396cc0..fa2522f81e155 100644 --- a/hermes_cli/dep_ensure.py +++ b/hermes_cli/dep_ensure.py @@ -22,10 +22,36 @@ import sys from pathlib import Path +from hermes_cli.node_runtime import ( + augment_path_with_hermes_node, + has_valid_private_hermes_node, + hermes_node_bin_dir, + node_satisfies_hermes_floor, + remove_legacy_node_symlinks, +) + _IS_WINDOWS = platform.system() == "Windows" + +def _has_node_runtime() -> bool: + node = shutil.which("node") + if node and node_satisfies_hermes_floor(node): + return True + return has_valid_private_hermes_node() + + +def _remove_legacy_node_links() -> None: + try: + from hermes_constants import get_hermes_home + + remove_legacy_node_symlinks(get_hermes_home()) + except Exception: + # Cleanup is best-effort. Dependency detection must remain fail-open. + pass + + _DEP_CHECKS = { - "node": lambda: shutil.which("node") is not None, + "node": _has_node_runtime, "browser": lambda: ( shutil.which("agent-browser") is not None or _has_system_browser() @@ -57,13 +83,13 @@ def _has_system_browser() -> bool: def _has_hermes_agent_browser() -> bool: from hermes_constants import get_hermes_home home = get_hermes_home() - if _IS_WINDOWS: - # npm -g --prefix puts .cmd shims directly in the prefix dir on Windows - return (home / "node" / "agent-browser.cmd").is_file() - # install.sh installs globally into $HERMES_HOME/node/bin/ via npm -g --prefix + node_bin = hermes_node_bin_dir(home) + if _IS_WINDOWS and (node_bin / "agent-browser.cmd").is_file(): + return True + # install.sh installs globally into the Hermes-managed Node bin dir. # Also check legacy node_modules/.bin/ path for git-clone installs. return ( - (home / "node" / "bin" / "agent-browser").is_file() + (node_bin / "agent-browser").is_file() or (home / "node_modules" / ".bin" / "agent-browser").is_file() ) @@ -105,11 +131,16 @@ def ensure_dependency( interactive: bool = True, ) -> bool: """Ensure a non-Python dependency is available. Returns True if available.""" + if dep in {"node", "browser"}: + _remove_legacy_node_links() + check = _DEP_CHECKS.get(dep) if check is None: # Unknown dep — don't silently forward to install script. return False if check(): + if dep == "node": + augment_path_with_hermes_node() return True script, shell = _find_install_script() @@ -155,5 +186,8 @@ def ensure_dependency( return False if check: - return check() + ok = check() + if ok and dep == "node": + augment_path_with_hermes_node() + return ok return True diff --git a/hermes_cli/main.py b/hermes_cli/main.py index 064b69277f6d8..a2e4ee9ecebe8 100644 --- a/hermes_cli/main.py +++ b/hermes_cli/main.py @@ -1596,7 +1596,23 @@ def _ensure_tui_node() -> None: Idempotent no-op when node+npm are already discoverable. Set ``HERMES_SKIP_NODE_BOOTSTRAP=1`` to disable auto-install. """ - if shutil.which("node") and shutil.which("npm"): + from hermes_cli.node_runtime import ( + augment_path_with_hermes_node, + node_satisfies_hermes_floor, + prepend_node_bin_dir, + remove_legacy_node_symlinks, + ) + + hermes_home = os.environ.get("HERMES_HOME") or str(Path.home() / ".hermes") + try: + remove_legacy_node_symlinks(Path(hermes_home)) + except Exception: + pass + + augment_path_with_hermes_node() + + node = shutil.which("node") + if node and shutil.which("npm") and node_satisfies_hermes_floor(node): return if os.environ.get("HERMES_SKIP_NODE_BOOTSTRAP"): return @@ -1605,7 +1621,6 @@ def _ensure_tui_node() -> None: if not helper.is_file(): return - hermes_home = os.environ.get("HERMES_HOME") or str(Path.home() / ".hermes") try: # Helper writes logs to stderr; we ask bash to print `command -v node` # on stdout once ensure_node succeeds. Subshell PATH edits don't leak @@ -1626,20 +1641,9 @@ def _ensure_tui_node() -> None: except (OSError, subprocess.SubprocessError): return - parts = os.environ.get("PATH", "").split(os.pathsep) - extras: list[Path] = [] - resolved = (result.stdout or "").strip() if resolved: - extras.append(Path(resolved).resolve().parent) - - extras.extend([Path(hermes_home) / "node" / "bin", Path.home() / ".local" / "bin"]) - - for extra in extras: - s = str(extra) - if extra.is_dir() and s not in parts: - parts.insert(0, s) - os.environ["PATH"] = os.pathsep.join(parts) + prepend_node_bin_dir(Path(resolved).resolve()) def _find_bundled_tui(hermes_cli_dir: Path | None = None) -> Path | None: @@ -7656,10 +7660,33 @@ def _ensure_uv_for_termux(pip_cmd: list[str]) -> str | None: return resolve_uv() or shutil.which("uv") +def _cleanup_legacy_node_symlinks() -> Path: + try: + from hermes_constants import get_hermes_home + from hermes_cli.node_runtime import remove_legacy_node_symlinks + + hermes_home = get_hermes_home() + remove_legacy_node_symlinks(hermes_home) + except Exception: + hermes_home = Path(os.environ.get("HERMES_HOME") or (Path.home() / ".hermes")) + return hermes_home + + +def _resolve_update_npm() -> str | None: + from hermes_cli.node_runtime import private_hermes_npm_path + + hermes_home = _cleanup_legacy_node_symlinks() + npm = shutil.which("npm") + if npm: + return npm + private_npm = private_hermes_npm_path(hermes_home) + return str(private_npm) if private_npm else None + + def _update_node_dependencies() -> None: - from hermes_constants import find_node_executable, with_hermes_node_path + from hermes_constants import with_hermes_node_path - npm = find_node_executable("npm") + npm = _resolve_update_npm() if not npm: return @@ -8591,6 +8618,10 @@ def _cmd_update_impl(args, gateway_mode: bool): print("⚕ Updating Hermes Agent...") print() + # Update is the common self-healing entrypoint for older installs. Clean up + # legacy Hermes-owned node/npm/npx PATH shims before any early return. + _cleanup_legacy_node_symlinks() + # On Windows, abort early if another hermes.exe is holding the venv shim # open. Continuing would result in a string of WinError 32 warnings and # then either a deferred-rename leftover or a failed git-pull fast path diff --git a/hermes_cli/node_runtime.py b/hermes_cli/node_runtime.py new file mode 100644 index 0000000000000..25eb6b730b873 --- /dev/null +++ b/hermes_cli/node_runtime.py @@ -0,0 +1,197 @@ +"""Helpers for Hermes-managed Node.js runtime discovery. + +Hermes may use its private Node inside Hermes subprocesses, but it must not +expose that Node by installing user-global ``node``/``npm``/``npx`` shims. +""" + +from __future__ import annotations + +import os +import re +import shutil +import subprocess +import sys +from pathlib import Path +from typing import Iterable + + +def hermes_node_bin_dir(hermes_home: Path | None = None) -> Path: + """Return the platform-preferred directory for Hermes-managed Node tools.""" + if hermes_home is None: + from hermes_constants import get_hermes_home + + hermes_home = get_hermes_home() + node_root = Path(hermes_home) / "node" + return node_root if sys.platform == "win32" else node_root / "bin" + + +def _node_command_name() -> str: + return "node.exe" if sys.platform == "win32" else "node" + + +def _npm_command_names() -> tuple[str, ...]: + if sys.platform == "win32": + return ("npm.cmd", "npm.exe", "npm") + return ("npm",) + + +def _parse_node_version(text: str) -> tuple[int, int] | None: + match = re.search(r"v?(\d+)\.(\d+)(?:\.\d+)?", text.strip()) + if not match: + return None + return int(match.group(1)), int(match.group(2)) + + +def _version_satisfies_floor(version: tuple[int, int]) -> bool: + major, minor = version + if major == 20: + return minor >= 19 + if major == 22: + return minor >= 12 + return major > 22 + + +def node_satisfies_hermes_floor(node_bin: str | Path) -> bool: + """Return whether ``node --version`` satisfies ``^20.19 || >=22.12``. + + Keep this floor in sync with ``scripts/install.sh::node_satisfies_build`` + and ``scripts/install.ps1::Test-NodeVersionOk``. + """ + try: + result = subprocess.run( + [str(node_bin), "--version"], + capture_output=True, + text=True, + encoding="utf-8", + errors="replace", + check=False, + timeout=5, + ) + except (OSError, subprocess.SubprocessError): + return False + if result.returncode != 0: + return False + version = _parse_node_version(result.stdout or result.stderr or "") + return bool(version and _version_satisfies_floor(version)) + + +def _path_parts() -> list[str]: + return [part for part in os.environ.get("PATH", "").split(os.pathsep) if part] + + +def prepend_node_bin_dir(node_bin: str | Path) -> bool: + """Prepend the directory containing *node_bin* to process-local ``PATH``.""" + path = Path(node_bin) + bin_dir = path.parent if path.name in {"node", "node.exe"} or path.is_file() else path + if not bin_dir.is_dir(): + return False + + bin_dir_s = str(bin_dir) + parts = _path_parts() + if parts and parts[0] == bin_dir_s: + return False + parts = [part for part in parts if part != bin_dir_s] + os.environ["PATH"] = os.pathsep.join([bin_dir_s, *parts]) + return True + + +def _private_node_path(hermes_home: Path | None = None) -> Path: + return hermes_node_bin_dir(hermes_home) / _node_command_name() + + +def _private_npm_path(hermes_home: Path | None = None) -> Path | None: + node_bin = hermes_node_bin_dir(hermes_home) + for name in _npm_command_names(): + candidate = node_bin / name + if candidate.is_file() and (sys.platform == "win32" or os.access(candidate, os.X_OK)): + return candidate + return None + + +def has_valid_private_hermes_node(hermes_home: Path | None = None) -> bool: + """Return whether Hermes has a complete private Node/npm pair.""" + node = _private_node_path(hermes_home) + if not node.is_file() or (sys.platform != "win32" and not os.access(node, os.X_OK)): + return False + if _private_npm_path(hermes_home) is None: + return False + return node_satisfies_hermes_floor(node) + + +def private_hermes_npm_path(hermes_home: Path | None = None) -> Path | None: + """Return private Hermes npm when the private Node runtime is complete.""" + if not has_valid_private_hermes_node(hermes_home): + return None + return _private_npm_path(hermes_home) + + +def augment_path_with_hermes_node() -> bool: + """Prepend Hermes-managed Node to this process when current Node is unusable. + + Missing ``npm`` alone is not a reason to shadow a modern user-managed Node. + """ + current_node = shutil.which("node") + if current_node and node_satisfies_hermes_floor(current_node): + return False + if not has_valid_private_hermes_node(): + return False + return prepend_node_bin_dir(_private_node_path()) + + +def legacy_node_symlink_candidate_dirs() -> list[Path]: + """Return directories where old POSIX installers created node/npm/npx links.""" + dirs: list[Path] = [Path.home() / ".local" / "bin"] + prefix = os.environ.get("PREFIX") + if prefix: + dirs.append(Path(prefix) / "bin") + if sys.platform != "win32": + dirs.append(Path("/usr/local/bin")) + + deduped: list[Path] = [] + seen: set[str] = set() + for directory in dirs: + key = str(directory) + if key not in seen: + deduped.append(directory) + seen.add(key) + return deduped + + +def _link_target_points_into(link: Path, node_dir: Path) -> bool: + if not link.is_symlink(): + return False + try: + target = Path(os.readlink(link)) + except OSError: + return False + if not target.is_absolute(): + target = link.parent / target + try: + target_resolved = target.resolve(strict=False) + node_resolved = node_dir.resolve(strict=False) + except OSError: + return False + return target_resolved == node_resolved or node_resolved in target_resolved.parents + + +def remove_legacy_node_symlinks( + hermes_home: Path, + *, + candidate_dirs: Iterable[Path] | None = None, +) -> list[Path]: + """Remove legacy Hermes-owned node/npm/npx symlinks from command dirs.""" + node_dir = Path(hermes_home) / "node" + removed: list[Path] = [] + for bin_dir in candidate_dirs or legacy_node_symlink_candidate_dirs(): + if not bin_dir.is_dir(): + continue + for name in ("node", "npm", "npx"): + link = bin_dir / name + if not _link_target_points_into(link, node_dir): + continue + try: + link.unlink() + except OSError: + continue + removed.append(link) + return removed diff --git a/hermes_cli/uninstall.py b/hermes_cli/uninstall.py index 9b53500734be1..ae21bfac2740b 100644 --- a/hermes_cli/uninstall.py +++ b/hermes_cli/uninstall.py @@ -120,15 +120,9 @@ def remove_wrapper_script(): def _node_symlink_candidate_dirs() -> "list[Path]": """Directories where the installer may have placed node/npm/npx symlinks.""" - dirs: list[Path] = [Path.home() / ".local" / "bin"] - # Root FHS installs put links in /usr/local/bin. - if sys.platform == "linux": - dirs.append(Path("/usr/local/bin")) - # Termux installs put links in $PREFIX/bin. - prefix = os.environ.get("PREFIX", "") - if prefix and "com.termux" in prefix: - dirs.append(Path(prefix) / "bin") - return dirs + from hermes_cli.node_runtime import legacy_node_symlink_candidate_dirs + + return legacy_node_symlink_candidate_dirs() def remove_node_symlinks(hermes_home: Path) -> list: @@ -148,32 +142,12 @@ def remove_node_symlinks(hermes_home: Path) -> list: directory are removed — links the user has repointed elsewhere (nvm, fnm, etc.) are left untouched. """ - node_dir = (hermes_home / "node").resolve() - removed = [] - - for name in ("node", "npm", "npx"): - for bin_dir in _node_symlink_candidate_dirs(): - link = bin_dir / name - try: - # Only act on symlinks — never delete a real binary the user put here. - if not link.is_symlink(): - continue + from hermes_cli.node_runtime import remove_legacy_node_symlinks - # Resolve the link target and confirm it points into our node dir. - # os.readlink + manual join handles broken (dangling) links too; - # Path.resolve() on a dangling link still returns the target path. - target = Path(os.readlink(link)) - if not target.is_absolute(): - target = (link.parent / target) - target = target.resolve() - - if target == node_dir or node_dir in target.parents: - link.unlink() - removed.append(link) - except Exception as e: - log_warn(f"Could not remove {link}: {e}") - - return removed + return remove_legacy_node_symlinks( + hermes_home, + candidate_dirs=_node_symlink_candidate_dirs(), + ) def uninstall_gateway_service(): diff --git a/scripts/install.sh b/scripts/install.sh index a969f31facdf1..efbf4cfff5bc8 100755 --- a/scripts/install.sh +++ b/scripts/install.sh @@ -413,23 +413,63 @@ get_command_link_display_dir() { fi } -# Point a Hermes-managed Node's `npm install -g` at a directory that is on -# PATH. npm's default global prefix for a bundled Node is the Node dir itself, -# so global package binaries land in $HERMES_HOME/node/bin — which is NOT on -# PATH (only the command link dir is) and is wiped on every Node upgrade. -# Redirecting the prefix to the link dir's parent makes global bins resolve to -# the command link dir (node/npm/npx live there too, already on PATH) and -# survive upgrades. Scoped to the managed Node via its prefix-local global +# Point a Hermes-managed Node's `npm install -g` at $HERMES_HOME/node. Global +# bins then land in $HERMES_HOME/node/bin, which Hermes can add to its own +# subprocess PATH without placing node/npm/npx in ~/.local/bin or another +# user-global command directory. Scoped to the managed Node via its prefix-local # npmrc, so the user's other Node installs and their ~/.npmrc are untouched. -# Hermes's own global installs pass an explicit --prefix and are unaffected. +# Hermes's own global installs pass the same explicit --prefix. # Idempotent and a no-op when there is no Hermes-managed npm, so calling it on # every install run repairs pre-existing installs, not just fresh ones. configure_managed_node_npm_prefix() { [ -x "$HERMES_HOME/node/bin/npm" ] || return 0 - local link_dir - link_dir="$(get_command_link_dir)" mkdir -p "$HERMES_HOME/node/etc" - printf 'prefix=%s\n' "$(dirname "$link_dir")" > "$HERMES_HOME/node/etc/npmrc" + printf 'prefix=%s\n' "$HERMES_HOME/node" > "$HERMES_HOME/node/etc/npmrc" +} + +node_link_points_into_hermes_node() { + local link="$1" + [ -L "$link" ] || return 1 + + local target target_abs node_dir + target="$(readlink "$link" 2>/dev/null)" || return 1 + case "$target" in + /*) target_abs="$target" ;; + *) target_abs="$(dirname "$link")/$target" ;; + esac + + node_dir="$HERMES_HOME/node" + case "$target_abs" in + "$node_dir"|"$node_dir"/*) return 0 ;; + esac + + if [ -d "$node_dir" ] && [ -e "$target_abs" ]; then + local resolved_target resolved_node + resolved_target="$(cd "$(dirname "$target_abs")" 2>/dev/null && pwd -P)/$(basename "$target_abs")" + resolved_node="$(cd "$node_dir" 2>/dev/null && pwd -P)" + case "$resolved_target" in + "$resolved_node"|"$resolved_node"/*) return 0 ;; + esac + fi + return 1 +} + +remove_managed_node_path_symlinks() { + local dirs=("$HOME/.local/bin" "/usr/local/bin") + if is_termux && [ -n "${PREFIX:-}" ]; then + dirs+=("$PREFIX/bin") + fi + + local dir name link + for dir in "${dirs[@]}"; do + [ -d "$dir" ] || continue + for name in node npm npx; do + link="$dir/$name" + if node_link_points_into_hermes_node "$link"; then + rm -f "$link" 2>/dev/null || true + fi + done + done } get_hermes_command_path() { @@ -741,9 +781,9 @@ node_satisfies_build() { check_node() { log_info "Checking Node.js (for browser tools)..." - # Repair pre-existing Hermes-managed installs where `npm install -g` lands - # off PATH. No-op when there's no managed Node, so this is safe to run on - # every install — including re-runs that skip the Node (re)install below. + # Repair pre-existing Hermes-managed installs. The cleanup only removes + # node/npm/npx symlinks that still point into this Hermes-managed Node tree. + remove_managed_node_path_symlinks configure_managed_node_npm_prefix if command -v node &> /dev/null && node_satisfies_build "$(node --version)"; then @@ -860,21 +900,14 @@ install_node() { return 0 fi - # Place into ~/.hermes/node/ and symlink binaries into the same bin dir - # the hermes command uses (get_command_link_dir): /usr/local/bin for root - # FHS installs, $PREFIX/bin on Termux, ~/.local/bin otherwise. + # Place into ~/.hermes/node/. Hermes exposes this Node only by prepending + # $HERMES_HOME/node/bin to subprocess PATH when it needs Node. rm -rf "$HERMES_HOME/node" mkdir -p "$HERMES_HOME" mv "$extracted_dir" "$HERMES_HOME/node" rm -rf "$tmp_dir" - local node_link_dir - node_link_dir="$(get_command_link_dir)" - mkdir -p "$node_link_dir" - ln -sf "$HERMES_HOME/node/bin/node" "$node_link_dir/node" - ln -sf "$HERMES_HOME/node/bin/npm" "$node_link_dir/npm" - ln -sf "$HERMES_HOME/node/bin/npx" "$node_link_dir/npx" - + remove_managed_node_path_symlinks configure_managed_node_npm_prefix export PATH="$HERMES_HOME/node/bin:$PATH" diff --git a/scripts/lib/node-bootstrap.sh b/scripts/lib/node-bootstrap.sh index 332ad81180ad7..c2cba260cc65b 100644 --- a/scripts/lib/node-bootstrap.sh +++ b/scripts/lib/node-bootstrap.sh @@ -44,30 +44,61 @@ _nb_is_termux() { [ -n "${TERMUX_VERSION:-}" ] || [[ "${PREFIX:-}" == *"com.termux/files/usr"* ]] } -# Where to symlink node/npm/npx so they land on PATH. -# Mirrors get_command_link_dir() from install.sh: root FHS → /usr/local/bin, -# Termux → $PREFIX/bin, otherwise ~/.local/bin. -_nb_get_link_dir() { - if _nb_is_termux && [ -n "${PREFIX:-}" ]; then - echo "$PREFIX/bin" - elif [ "$(id -u)" = 0 ] && [ "$(uname -s)" = "Linux" ]; then - echo "/usr/local/bin" - else - echo "$HOME/.local/bin" - fi -} - -# Redirect a Hermes-managed Node's `npm install -g` to the command link dir -# (already on PATH) instead of the default $HERMES_HOME/node/bin, which is off -# PATH and wiped on every Node upgrade. Scoped to the managed Node via its -# prefix-local global npmrc; the user's other Node installs / ~/.npmrc are -# untouched. Idempotent no-op when there's no managed npm. +# Point a Hermes-managed Node's `npm install -g` at $HERMES_HOME/node. Global +# bins then land in $HERMES_HOME/node/bin, which Hermes can add to its own +# subprocess PATH without placing node/npm/npx in ~/.local/bin or another +# user-global command directory. Scoped to the managed Node via its prefix-local +# global npmrc; the user's other Node installs / ~/.npmrc are untouched. +# Idempotent no-op when there's no managed npm. _nb_configure_npm_prefix() { [ -x "$HERMES_HOME/node/bin/npm" ] || return 0 - local _link_dir - _link_dir="$(_nb_get_link_dir)" mkdir -p "$HERMES_HOME/node/etc" - printf 'prefix=%s\n' "$(dirname "$_link_dir")" > "$HERMES_HOME/node/etc/npmrc" + printf 'prefix=%s\n' "$HERMES_HOME/node" > "$HERMES_HOME/node/etc/npmrc" +} + +_nb_link_points_into_hermes_node() { + local link="$1" + [ -L "$link" ] || return 1 + + local target target_abs node_dir + target="$(readlink "$link" 2>/dev/null)" || return 1 + case "$target" in + /*) target_abs="$target" ;; + *) target_abs="$(dirname "$link")/$target" ;; + esac + + node_dir="$HERMES_HOME/node" + case "$target_abs" in + "$node_dir"|"$node_dir"/*) return 0 ;; + esac + + if [ -d "$node_dir" ] && [ -e "$target_abs" ]; then + local resolved_target resolved_node + resolved_target="$(cd "$(dirname "$target_abs")" 2>/dev/null && pwd -P)/$(basename "$target_abs")" + resolved_node="$(cd "$node_dir" 2>/dev/null && pwd -P)" + case "$resolved_target" in + "$resolved_node"|"$resolved_node"/*) return 0 ;; + esac + fi + return 1 +} + +_nb_remove_legacy_node_links() { + local dirs=("$HOME/.local/bin" "/usr/local/bin") + if _nb_is_termux && [ -n "${PREFIX:-}" ]; then + dirs+=("$PREFIX/bin") + fi + + local dir name link + for dir in "${dirs[@]}"; do + [ -d "$dir" ] || continue + for name in node npm npx; do + link="$dir/$name" + if _nb_link_points_into_hermes_node "$link"; then + rm -f "$link" 2>/dev/null || true + fi + done + done } _nb_node_major() { @@ -213,13 +244,7 @@ _nb_install_bundled_node() { mv "$extracted" "$HERMES_HOME/node" rm -rf "$tmp" - local _link_dir - _link_dir="$(_nb_get_link_dir)" - mkdir -p "$_link_dir" - ln -sf "$HERMES_HOME/node/bin/node" "$_link_dir/node" - ln -sf "$HERMES_HOME/node/bin/npm" "$_link_dir/npm" - ln -sf "$HERMES_HOME/node/bin/npx" "$_link_dir/npx" - + _nb_remove_legacy_node_links _nb_configure_npm_prefix export PATH="$HERMES_HOME/node/bin:$PATH" @@ -236,8 +261,9 @@ _nb_install_bundled_node() { ensure_node() { HERMES_NODE_AVAILABLE=false - # Repair pre-existing managed installs where `npm install -g` lands off - # PATH. No-op when there's no managed Node, so it's safe to run first. + # Repair pre-existing managed installs. The cleanup only removes + # node/npm/npx symlinks that still point into this Hermes-managed Node tree. + _nb_remove_legacy_node_links _nb_configure_npm_prefix if _nb_have_modern_node; then diff --git a/tests/hermes_cli/test_cmd_update.py b/tests/hermes_cli/test_cmd_update.py index 224c65e9a484b..622e3f88ac6de 100644 --- a/tests/hermes_cli/test_cmd_update.py +++ b/tests/hermes_cli/test_cmd_update.py @@ -1,6 +1,7 @@ """Tests for cmd_update — branch fallback when remote branch doesn't exist.""" import subprocess +from pathlib import Path from types import SimpleNamespace from unittest.mock import patch @@ -127,9 +128,9 @@ def test_update_falls_back_to_main_when_branch_not_on_remote( # rev-list should use origin/main, not origin/fix/stoicneko rev_list_cmds = [c for c in commands if "rev-list" in c] - assert len(rev_list_cmds) == 1 - assert "origin/main" in rev_list_cmds[0] - assert "origin/fix/stoicneko" not in rev_list_cmds[0] + assert rev_list_cmds + assert all("origin/main" in c for c in rev_list_cmds) + assert not any("origin/fix/stoicneko" in c for c in rev_list_cmds) # pull should use main, not fix/stoicneko pull_cmds = [c for c in commands if "pull" in c] @@ -176,6 +177,22 @@ def test_update_already_up_to_date( pull_cmds = [c for c in commands if "pull" in c] assert len(pull_cmds) == 0 + @patch("shutil.which", return_value=None) + @patch("subprocess.run") + def test_update_already_up_to_date_cleans_legacy_node_symlinks( + self, mock_run, _mock_which, mock_args + ): + from hermes_cli import main as hm + + mock_run.side_effect = _make_run_side_effect( + branch="main", verify_ok=True, commit_count="0" + ) + + with patch.object(hm, "_cleanup_legacy_node_symlinks") as cleanup_mock: + cmd_update(mock_args) + + cleanup_mock.assert_called() + @patch("shutil.which", return_value=None) @patch("subprocess.run") def test_update_on_fork_checks_upstream_when_origin_up_to_date( @@ -825,3 +842,39 @@ def test_load_installable_optional_extras_supports_termux_group(tmp_path, monkey assert hm._load_installable_optional_extras(group="all") == ["mcp"] assert hm._load_installable_optional_extras(group="termux-all") == ["termux", "mcp"] + + +def test_update_node_dependencies_uses_private_hermes_npm_when_path_has_none( + tmp_path, monkeypatch +): + from hermes_cli import main as hm + + project = tmp_path / "repo" + project.mkdir() + (project / "package.json").write_text("{}", encoding="utf-8") + hermes_home = tmp_path / ".hermes" + node_bin = hermes_home / "node" / "bin" + node_bin.mkdir(parents=True) + node = node_bin / "node" + node.write_text("#!/bin/sh\nprintf 'v22.12.0\\n'\n", encoding="utf-8") + node.chmod(0o755) + npm = node_bin / "npm" + npm.write_text("#!/bin/sh\n", encoding="utf-8") + npm.chmod(0o755) + + calls = [] + + def fake_run_npm(npm_path, cwd, *, extra_args=(), capture_output=False, env=None): + calls.append((npm_path, Path(cwd), tuple(extra_args), capture_output, env)) + return subprocess.CompletedProcess([npm_path], 0, stdout="", stderr="") + + monkeypatch.setattr(hm, "PROJECT_ROOT", project) + monkeypatch.setattr(hm.shutil, "which", lambda _name: None) + monkeypatch.setattr("hermes_constants.get_hermes_home", lambda: hermes_home) + monkeypatch.setattr(hm, "_run_npm_install_deterministic", fake_run_npm) + monkeypatch.setattr(hm, "_nixos_build_env", lambda: None) + + hm._update_node_dependencies() + + assert [call[0] for call in calls] == [str(npm), str(npm)] + assert [call[1] for call in calls] == [project, project] diff --git a/tests/hermes_cli/test_dep_ensure.py b/tests/hermes_cli/test_dep_ensure.py index a19a6de63f2b4..923dd6237445f 100644 --- a/tests/hermes_cli/test_dep_ensure.py +++ b/tests/hermes_cli/test_dep_ensure.py @@ -1,13 +1,36 @@ +import os +from pathlib import Path from unittest.mock import patch -def test_ensure_dependency_skips_when_present(): +def _write_executable(path: Path, body: str) -> Path: + path.parent.mkdir(parents=True, exist_ok=True) + path.write_text(body, encoding="utf-8") + path.chmod(0o755) + return path + + +def _fake_node_at(path: Path, version: str) -> Path: + return _write_executable(path, f"#!/bin/sh\necho {version}\n") + + +def _fake_private_hermes_node(hermes_home: Path, version: str) -> Path: + node_bin = hermes_home / "node" / "bin" + _fake_node_at(node_bin / "node", version) + _write_executable(node_bin / "npm", "#!/bin/sh\necho npm\n") + return node_bin + + +def test_ensure_dependency_skips_when_present(tmp_path, monkeypatch): """ensure_dependency is a no-op when the dep is already available.""" from hermes_cli.dep_ensure import ensure_dependency - with patch("hermes_cli.dep_ensure.shutil") as mock_shutil: - mock_shutil.which.return_value = "/usr/bin/node" - result = ensure_dependency("node", interactive=False) - assert result is True + + user_bin = tmp_path / "user" / "bin" + _fake_node_at(user_bin / "node", "v24.0.0") + monkeypatch.setenv("PATH", str(user_bin)) + + result = ensure_dependency("node", interactive=False) + assert result is True def test_ensure_dependency_returns_false_when_missing_noninteractive(): @@ -20,6 +43,54 @@ def test_ensure_dependency_returns_false_when_missing_noninteractive(): assert result is False +def test_ensure_dependency_accepts_private_hermes_node_when_not_on_path( + tmp_path, monkeypatch +): + from hermes_cli.dep_ensure import ensure_dependency + + private_bin = _fake_private_hermes_node(tmp_path, "v22.12.0") + empty_path = tmp_path / "empty" + empty_path.mkdir() + monkeypatch.setenv("PATH", str(empty_path)) + monkeypatch.setattr("hermes_constants.get_hermes_home", lambda: tmp_path) + monkeypatch.setattr( + "hermes_cli.dep_ensure._find_install_script", lambda: (None, None) + ) + + assert ensure_dependency("node", interactive=False) is True + assert os.environ["PATH"].split(os.pathsep)[0] == str(private_bin) + + +def test_ensure_dependency_uses_private_hermes_node_when_path_node_is_too_old( + tmp_path, monkeypatch +): + from hermes_cli.dep_ensure import ensure_dependency + + user_bin = tmp_path / "user" / "bin" + _fake_node_at(user_bin / "node", "v22.11.0") + private_bin = _fake_private_hermes_node(tmp_path, "v22.12.0") + monkeypatch.setenv("PATH", str(user_bin)) + monkeypatch.setattr("hermes_constants.get_hermes_home", lambda: tmp_path) + + assert ensure_dependency("node", interactive=False) is True + assert os.environ["PATH"].split(os.pathsep)[0] == str(private_bin) + + +def test_ensure_dependency_does_not_shadow_modern_node_when_only_npm_is_missing( + tmp_path, monkeypatch +): + from hermes_cli.dep_ensure import ensure_dependency + + user_bin = tmp_path / "user" / "bin" + _fake_node_at(user_bin / "node", "v24.0.0") + _fake_private_hermes_node(tmp_path, "v22.12.0") + monkeypatch.setenv("PATH", str(user_bin)) + monkeypatch.setattr("hermes_constants.get_hermes_home", lambda: tmp_path) + + assert ensure_dependency("node", interactive=False) is True + assert os.environ["PATH"].split(os.pathsep)[0] == str(user_bin) + + def test_find_install_script_from_checkout(tmp_path): """_find_install_script finds scripts/install.sh in a git checkout.""" from hermes_cli.dep_ensure import _find_install_script @@ -113,6 +184,7 @@ def test_has_hermes_agent_browser_windows_path(tmp_path): (node_dir / "agent-browser.cmd").write_text("@echo off") from hermes_cli.dep_ensure import _has_hermes_agent_browser with patch("hermes_cli.dep_ensure._IS_WINDOWS", True), \ + patch("hermes_cli.node_runtime.sys.platform", "win32"), \ patch("hermes_constants.get_hermes_home", return_value=tmp_path): assert _has_hermes_agent_browser() is True diff --git a/tests/hermes_cli/test_node_runtime.py b/tests/hermes_cli/test_node_runtime.py new file mode 100644 index 0000000000000..1289cfaa95d38 --- /dev/null +++ b/tests/hermes_cli/test_node_runtime.py @@ -0,0 +1,108 @@ +import os +from pathlib import Path + + +def _write_executable(path: Path, body: str) -> Path: + path.parent.mkdir(parents=True, exist_ok=True) + path.write_text(body, encoding="utf-8") + path.chmod(0o755) + return path + + +def _fake_node_at(path: Path, version: str) -> Path: + return _write_executable(path, f"#!/bin/sh\necho {version}\n") + + +def _fake_private_hermes_node(hermes_home: Path, version: str) -> Path: + node_bin = hermes_home / "node" / "bin" + _fake_node_at(node_bin / "node", version) + _write_executable(node_bin / "npm", "#!/bin/sh\necho npm\n") + return node_bin + + +def test_node_satisfies_hermes_floor_matches_installer_floor(tmp_path): + from hermes_cli.node_runtime import node_satisfies_hermes_floor + + assert node_satisfies_hermes_floor(_fake_node_at(tmp_path / "n2018", "v20.18.9")) is False + assert node_satisfies_hermes_floor(_fake_node_at(tmp_path / "n2019", "v20.19.0")) is True + assert node_satisfies_hermes_floor(_fake_node_at(tmp_path / "n2211", "v22.11.0")) is False + assert node_satisfies_hermes_floor(_fake_node_at(tmp_path / "n2212", "v22.12.0")) is True + assert node_satisfies_hermes_floor(_fake_node_at(tmp_path / "n24", "v24.0.0")) is True + + +def test_node_satisfies_hermes_floor_rejects_bad_version_output(tmp_path): + from hermes_cli.node_runtime import node_satisfies_hermes_floor + + assert node_satisfies_hermes_floor(_fake_node_at(tmp_path / "node", "not-a-version")) is False + + +def test_augment_path_with_hermes_node_uses_private_node_when_path_node_missing( + tmp_path, monkeypatch +): + from hermes_cli.node_runtime import augment_path_with_hermes_node + + private_bin = _fake_private_hermes_node(tmp_path, "v22.12.0") + empty_path = tmp_path / "empty" + empty_path.mkdir() + monkeypatch.setenv("PATH", str(empty_path)) + monkeypatch.setattr("hermes_constants.get_hermes_home", lambda: tmp_path) + + assert augment_path_with_hermes_node() is True + assert os.environ["PATH"].split(os.pathsep)[0] == str(private_bin) + + +def test_augment_path_with_hermes_node_uses_private_node_when_path_node_too_old( + tmp_path, monkeypatch +): + from hermes_cli.node_runtime import augment_path_with_hermes_node + + user_bin = tmp_path / "user" / "bin" + _fake_node_at(user_bin / "node", "v20.18.9") + private_bin = _fake_private_hermes_node(tmp_path, "v22.12.0") + monkeypatch.setenv("PATH", str(user_bin)) + monkeypatch.setattr("hermes_constants.get_hermes_home", lambda: tmp_path) + + assert augment_path_with_hermes_node() is True + assert os.environ["PATH"].split(os.pathsep)[0] == str(private_bin) + + +def test_augment_path_with_hermes_node_does_not_shadow_modern_node_when_npm_missing( + tmp_path, monkeypatch +): + from hermes_cli.node_runtime import augment_path_with_hermes_node + + user_bin = tmp_path / "user" / "bin" + _fake_node_at(user_bin / "node", "v24.0.0") + _fake_private_hermes_node(tmp_path, "v22.12.0") + monkeypatch.setenv("PATH", str(user_bin)) + monkeypatch.setattr("hermes_constants.get_hermes_home", lambda: tmp_path) + + assert augment_path_with_hermes_node() is False + assert os.environ["PATH"].split(os.pathsep)[0] == str(user_bin) + + +def test_remove_legacy_node_symlinks_only_removes_current_hermes_home( + tmp_path, monkeypatch +): + from hermes_cli.node_runtime import remove_legacy_node_symlinks + + home = tmp_path / "home" + hermes_home = home / ".hermes" + local_bin = home / ".local" / "bin" + local_bin.mkdir(parents=True) + node_bin = _fake_private_hermes_node(hermes_home, "v22.12.0") + + for name in ("node", "npm", "npx"): + (local_bin / name).symlink_to(node_bin / name) + + nvm_bin = home / ".nvm" / "versions" / "node" / "v24.0.0" / "bin" + nvm_node = _fake_node_at(nvm_bin / "node", "v24.0.0") + (local_bin / "user-node").symlink_to(nvm_node) + monkeypatch.setattr(Path, "home", classmethod(lambda cls: home)) + + removed = remove_legacy_node_symlinks(hermes_home) + + assert sorted(path.name for path in removed) == ["node", "npm", "npx"] + assert (local_bin / "user-node").is_symlink() + for name in ("node", "npm", "npx"): + assert not (local_bin / name).is_symlink() diff --git a/tests/hermes_cli/test_tui_resume_flow.py b/tests/hermes_cli/test_tui_resume_flow.py index ad8ffbe79b81a..be85c35da5ca4 100644 --- a/tests/hermes_cli/test_tui_resume_flow.py +++ b/tests/hermes_cli/test_tui_resume_flow.py @@ -412,6 +412,43 @@ def test_termux_ultrafast_version_runs_before_heavy_startup( assert "OpenAI SDK:" in out +def test_ensure_tui_node_prepends_resolved_node_without_local_bin( + tmp_path, monkeypatch, main_mod +): + home = tmp_path / "home" + hermes_home = home / ".hermes" + node_bin = hermes_home / "node" / "bin" + local_bin = home / ".local" / "bin" + empty_path = tmp_path / "empty" + node_bin.mkdir(parents=True) + local_bin.mkdir(parents=True) + empty_path.mkdir() + + node = node_bin / "node" + node.write_text("#!/bin/sh\necho v22.12.0\n", encoding="utf-8") + node.chmod(0o755) + + calls = [] + + def fake_run(cmd, **kwargs): + calls.append((cmd, kwargs)) + return main_mod.subprocess.CompletedProcess(cmd, 0, stdout=f"{node}\n", stderr="") + + monkeypatch.setattr(Path, "home", classmethod(lambda cls: home)) + monkeypatch.setenv("HERMES_HOME", str(hermes_home)) + monkeypatch.setenv("PATH", str(empty_path)) + monkeypatch.setattr(main_mod.shutil, "which", lambda _name: None) + monkeypatch.setattr(main_mod.subprocess, "run", fake_run) + + main_mod._ensure_tui_node() + + path_entries = os.environ["PATH"].split(os.pathsep) + assert path_entries[0] == str(node_bin) + assert str(local_bin) not in path_entries + assert calls + assert "ensure_node" in " ".join(calls[0][0]) + + def test_read_openai_version_fast(monkeypatch, tmp_path, main_mod): package_dir = tmp_path / "openai" package_dir.mkdir() diff --git a/tests/test_install_sh_node_global_prefix.py b/tests/test_install_sh_node_global_prefix.py index e43b9201bd165..2c782f328668e 100644 --- a/tests/test_install_sh_node_global_prefix.py +++ b/tests/test_install_sh_node_global_prefix.py @@ -1,14 +1,11 @@ -"""Regression tests for the Hermes-managed Node's npm global prefix. +"""Regression tests for Hermes-managed Node PATH isolation. When the installer falls back to a bundled Node under ``$HERMES_HOME/node``, -npm's default global prefix is that Node dir, so ``npm install -g `` -drops the package binary in ``$HERMES_HOME/node/bin`` — which is NOT on PATH -(only the command link dir is) and is wiped on every Node upgrade. Users then -report "I can ``npm i -g`` but the package isn't usable on the command line". - -The fix redirects the bundled Node's global prefix to the command link dir's -parent (so global bins land in the already-on-PATH link dir alongside -node/npm/npx), scoped to the bundled Node via its prefix-local global npmrc. +Hermes must keep node/npm/npx private. Older installers linked them into the +command link dir (usually ``~/.local/bin``), which could shadow nvm/fnm/Volta. +The managed npm prefix stays inside ``$HERMES_HOME/node`` so npm global bins +land in ``$HERMES_HOME/node/bin``: private to Hermes, but still visible to +Hermes subprocess PATH construction and dependency detection. """ from pathlib import Path @@ -19,14 +16,14 @@ NODE_BOOTSTRAP = REPO_ROOT / "scripts" / "lib" / "node-bootstrap.sh" -def test_install_sh_redirects_bundled_npm_global_prefix_to_link_dir() -> None: +def test_install_sh_keeps_bundled_npm_global_prefix_inside_hermes_home() -> None: text = INSTALL_SH.read_text() - # The redirect must target the link dir's PARENT so global bins resolve to - # /bin == the command link dir (node/npm/npx live there and it is - # guaranteed on PATH by the installer's PATH setup). + # prefix=$HERMES_HOME/node keeps global bins in $HERMES_HOME/node/bin, + # which Hermes can add to subprocess PATH without polluting ~/.local/bin. assert "configure_managed_node_npm_prefix()" in text - assert 'printf \'prefix=%s\\n\' "$(dirname "$link_dir")" > "$HERMES_HOME/node/etc/npmrc"' in text + assert 'printf \'prefix=%s\\n\' "$HERMES_HOME/node" > "$HERMES_HOME/node/etc/npmrc"' in text + assert 'printf \'prefix=%s\\n\' "$(dirname "$link_dir")" > "$HERMES_HOME/node/etc/npmrc"' not in text def test_install_sh_repairs_existing_managed_node_on_rerun() -> None: @@ -37,19 +34,50 @@ def test_install_sh_repairs_existing_managed_node_on_rerun() -> None: check_node_body = text.split("check_node()", 1)[1].split("\ninstall_node()", 1)[0] assert "configure_managed_node_npm_prefix" in check_node_body + assert "remove_managed_node_path_symlinks" in check_node_body # No-op guard so it's safe to call when there is no managed Node. assert '[ -x "$HERMES_HOME/node/bin/npm" ] || return 0' in text + assert "node_link_points_into_hermes_node" in text -def test_node_bootstrap_redirects_bundled_npm_global_prefix_to_link_dir() -> None: +def test_node_bootstrap_keeps_bundled_npm_global_prefix_inside_hermes_home() -> None: text = NODE_BOOTSTRAP.read_text() assert "_nb_configure_npm_prefix()" in text - assert 'printf \'prefix=%s\\n\' "$(dirname "$_link_dir")" > "$HERMES_HOME/node/etc/npmrc"' in text + assert 'printf \'prefix=%s\\n\' "$HERMES_HOME/node" > "$HERMES_HOME/node/etc/npmrc"' in text + assert 'printf \'prefix=%s\\n\' "$(dirname "$_link_dir")" > "$HERMES_HOME/node/etc/npmrc"' not in text # Runs at the top of ensure_node so existing managed installs are repaired # even when a modern Node is already present (early return path). ensure_node_body = text.split("ensure_node()", 1)[1] assert "_nb_configure_npm_prefix" in ensure_node_body + assert "_nb_remove_legacy_node_links" in ensure_node_body assert '[ -x "$HERMES_HOME/node/bin/npm" ] || return 0' in text + assert "_nb_link_points_into_hermes_node" in text + + +def test_installers_never_link_managed_node_tools_into_command_dir() -> None: + install_text = INSTALL_SH.read_text() + bootstrap_text = NODE_BOOTSTRAP.read_text() + + forbidden = ( + 'ln -sf "$HERMES_HOME/node/bin/node"', + 'ln -sf "$HERMES_HOME/node/bin/npm"', + 'ln -sf "$HERMES_HOME/node/bin/npx"', + ) + for snippet in forbidden: + assert snippet not in install_text + assert snippet not in bootstrap_text + + +def test_node_bootstrap_keeps_version_manager_and_bundled_cascade() -> None: + text = NODE_BOOTSTRAP.read_text() + ensure_node_body = text.split("ensure_node()", 1)[1] + + assert "_nb_try_fnm" in ensure_node_body + assert "_nb_try_proto" in ensure_node_body + assert "_nb_try_nvm" in ensure_node_body + assert "_nb_try_termux_pkg" in ensure_node_body + assert "_nb_try_brew" in ensure_node_body + assert "_nb_install_bundled_node" in ensure_node_body From eea93ef177b4b9fa70a28b0eb82e46329cf8e39c Mon Sep 17 00:00:00 2001 From: Bruce-anle <840596168@qq.com> Date: Sat, 20 Jun 2026 13:04:14 +0800 Subject: [PATCH 2/3] fix(update): preserve user node path MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 背景: Independent review found two remaining Node runtime gaps: node-bootstrap accepted Node versions below the desktop build floor, and update could prepend Hermes-managed Node while running a user PATH npm. 变更点: Sync node-bootstrap.sh version checks with ^20.19 || >=22.12, and make update only prepend Hermes-managed Node when npm is private or the PATH Node is missing/too old. 测试或冒烟结果: - /home/brucean/.hermes/hermes-agent/venv/bin/python -m pytest tests/hermes_cli/test_node_runtime.py tests/hermes_cli/test_dep_ensure.py tests/hermes_cli/test_tui_resume_flow.py tests/hermes_cli/test_cmd_update.py tests/hermes_cli/test_uninstall_node_symlinks.py tests/test_install_sh_node_global_prefix.py -q - bash -n scripts/install.sh - bash -n scripts/lib/node-bootstrap.sh - git diff --check --- hermes_cli/main.py | 19 ++++--- scripts/lib/node-bootstrap.sh | 16 +++--- tests/hermes_cli/test_cmd_update.py | 55 +++++++++++++++++++++ tests/test_install_sh_node_global_prefix.py | 39 +++++++++++++++ 4 files changed, 116 insertions(+), 13 deletions(-) diff --git a/hermes_cli/main.py b/hermes_cli/main.py index a2e4ee9ecebe8..134143f8eac0e 100644 --- a/hermes_cli/main.py +++ b/hermes_cli/main.py @@ -7672,21 +7672,26 @@ def _cleanup_legacy_node_symlinks() -> Path: return hermes_home -def _resolve_update_npm() -> str | None: - from hermes_cli.node_runtime import private_hermes_npm_path +def _resolve_update_npm() -> tuple[str | None, bool]: + from hermes_cli.node_runtime import ( + node_satisfies_hermes_floor, + private_hermes_npm_path, + ) hermes_home = _cleanup_legacy_node_symlinks() npm = shutil.which("npm") if npm: - return npm + node = shutil.which("node") + use_hermes_node_path = not (node and node_satisfies_hermes_floor(node)) + return npm, use_hermes_node_path private_npm = private_hermes_npm_path(hermes_home) - return str(private_npm) if private_npm else None + return (str(private_npm), True) if private_npm else (None, False) def _update_node_dependencies() -> None: from hermes_constants import with_hermes_node_path - npm = _resolve_update_npm() + npm, use_hermes_node_path = _resolve_update_npm() if not npm: return @@ -7703,7 +7708,9 @@ def _update_node_dependencies() -> None: print("→ Updating Node.js dependencies...") extra_args = ["--no-fund", "--no-audit", "--progress=false"] - nixos_env = with_hermes_node_path(_nixos_build_env()) + nixos_env = _nixos_build_env() + if use_hermes_node_path: + nixos_env = with_hermes_node_path(nixos_env) # Step 1: root install (no workspace recursion). root_args = [*extra_args, "--workspaces=false"] diff --git a/scripts/lib/node-bootstrap.sh b/scripts/lib/node-bootstrap.sh index c2cba260cc65b..f4e82465c7b21 100644 --- a/scripts/lib/node-bootstrap.sh +++ b/scripts/lib/node-bootstrap.sh @@ -101,15 +101,17 @@ _nb_remove_legacy_node_links() { done } -_nb_node_major() { - local v - v=$(node --version 2>/dev/null | sed 's/^v//' | cut -d. -f1) - [[ "$v" =~ ^[0-9]+$ ]] && echo "$v" || echo 0 -} - _nb_have_modern_node() { command -v node >/dev/null 2>&1 || return 1 - [ "$(_nb_node_major)" -ge "$HERMES_NODE_MIN_VERSION" ] + local ver major minor + ver=$(node --version 2>/dev/null | sed 's/^v//') + major="${ver%%.*}" + minor="${ver#*.}"; minor="${minor%%.*}" + [[ "$major" =~ ^[0-9]+$ ]] || return 1 + [[ "$minor" =~ ^[0-9]+$ ]] || minor=0 + if [ "$major" -eq 20 ] && [ "$minor" -ge 19 ]; then return 0; fi + if [ "$major" -eq 22 ] && [ "$minor" -ge 12 ]; then return 0; fi + [ "$major" -gt 22 ] } # --------------------------------------------------------------------------- diff --git a/tests/hermes_cli/test_cmd_update.py b/tests/hermes_cli/test_cmd_update.py index 622e3f88ac6de..160eef64293ae 100644 --- a/tests/hermes_cli/test_cmd_update.py +++ b/tests/hermes_cli/test_cmd_update.py @@ -1,5 +1,6 @@ """Tests for cmd_update — branch fallback when remote branch doesn't exist.""" +import os import subprocess from pathlib import Path from types import SimpleNamespace @@ -878,3 +879,57 @@ def fake_run_npm(npm_path, cwd, *, extra_args=(), capture_output=False, env=None assert [call[0] for call in calls] == [str(npm), str(npm)] assert [call[1] for call in calls] == [project, project] + + +def test_update_node_dependencies_keeps_user_node_first_when_using_user_npm( + tmp_path, monkeypatch +): + from hermes_cli import main as hm + + project = tmp_path / "repo" + project.mkdir() + (project / "package.json").write_text("{}", encoding="utf-8") + + user_bin = tmp_path / "user" / "bin" + user_bin.mkdir(parents=True) + user_node = user_bin / "node" + user_node.write_text("#!/bin/sh\nprintf 'v24.0.0\\n'\n", encoding="utf-8") + user_node.chmod(0o755) + user_npm = user_bin / "npm" + user_npm.write_text("#!/bin/sh\n", encoding="utf-8") + user_npm.chmod(0o755) + + hermes_home = tmp_path / ".hermes" + private_bin = hermes_home / "node" / "bin" + private_bin.mkdir(parents=True) + private_node = private_bin / "node" + private_node.write_text("#!/bin/sh\nprintf 'v22.12.0\\n'\n", encoding="utf-8") + private_node.chmod(0o755) + private_npm = private_bin / "npm" + private_npm.write_text("#!/bin/sh\n", encoding="utf-8") + private_npm.chmod(0o755) + + calls = [] + + def fake_run_npm(npm_path, cwd, *, extra_args=(), capture_output=False, env=None): + calls.append((npm_path, Path(cwd), tuple(extra_args), capture_output, env)) + return subprocess.CompletedProcess([npm_path], 0, stdout="", stderr="") + + monkeypatch.setattr(hm, "PROJECT_ROOT", project) + monkeypatch.setenv("PATH", str(user_bin)) + monkeypatch.setattr( + hm.shutil, + "which", + lambda name: str(user_bin / name) if name in {"node", "npm"} else None, + ) + monkeypatch.setattr("hermes_constants.get_hermes_home", lambda: hermes_home) + monkeypatch.setattr(hm, "_run_npm_install_deterministic", fake_run_npm) + monkeypatch.setattr(hm, "_nixos_build_env", lambda: {"PATH": str(user_bin)}) + + hm._update_node_dependencies() + + assert [call[0] for call in calls] == [str(user_npm), str(user_npm)] + for call in calls: + env_path = call[4]["PATH"].split(os.pathsep) + assert env_path[0] == str(user_bin) + assert str(private_bin) not in env_path[:1] diff --git a/tests/test_install_sh_node_global_prefix.py b/tests/test_install_sh_node_global_prefix.py index 2c782f328668e..801cf110950e8 100644 --- a/tests/test_install_sh_node_global_prefix.py +++ b/tests/test_install_sh_node_global_prefix.py @@ -8,6 +8,8 @@ Hermes subprocess PATH construction and dependency detection. """ +import os +import subprocess from pathlib import Path @@ -16,6 +18,13 @@ NODE_BOOTSTRAP = REPO_ROOT / "scripts" / "lib" / "node-bootstrap.sh" +def _write_executable(path: Path, body: str) -> Path: + path.parent.mkdir(parents=True, exist_ok=True) + path.write_text(body, encoding="utf-8") + path.chmod(0o755) + return path + + def test_install_sh_keeps_bundled_npm_global_prefix_inside_hermes_home() -> None: text = INSTALL_SH.read_text() @@ -81,3 +90,33 @@ def test_node_bootstrap_keeps_version_manager_and_bundled_cascade() -> None: assert "_nb_try_termux_pkg" in ensure_node_body assert "_nb_try_brew" in ensure_node_body assert "_nb_install_bundled_node" in ensure_node_body + + +def test_node_bootstrap_version_floor_matches_desktop_build_floor(tmp_path) -> None: + cases = [ + ("v20.18.9", False), + ("v20.19.0", True), + ("v22.11.0", False), + ("v22.12.0", True), + ("v24.0.0", True), + ] + + for version, expected in cases: + bin_dir = tmp_path / version / "bin" + _write_executable(bin_dir / "node", f"#!/bin/sh\nprintf '{version}\\n'\n") + env = { + **os.environ, + "PATH": f"{bin_dir}{os.pathsep}{os.environ.get('PATH', '')}", + "HOME": str(tmp_path / "home"), + "HERMES_HOME": str(tmp_path / "home" / ".hermes"), + } + + result = subprocess.run( + ["bash", "-c", f'source "{NODE_BOOTSTRAP}"; _nb_have_modern_node'], + env=env, + capture_output=True, + text=True, + check=False, + ) + + assert (result.returncode == 0) is expected, version From df5cf60aa98b2ff5adc0152c39679e8d0bc90ef3 Mon Sep 17 00:00:00 2001 From: Bruce-anle <840596168@qq.com> Date: Sat, 20 Jun 2026 13:08:39 +0800 Subject: [PATCH 3/3] chore(install): remove stale node floor override MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 背景: Independent follow-up review found HERMES_NODE_MIN_VERSION was documented as configurable even though node-bootstrap now enforces the fixed desktop build floor. 变更点: Remove the stale HERMES_NODE_MIN_VERSION input and update node-bootstrap documentation to name the ^20.19 || >=22.12 floor. 测试或冒烟结果: - /home/brucean/.hermes/hermes-agent/venv/bin/python -m pytest tests/test_install_sh_node_global_prefix.py -q - bash -n scripts/lib/node-bootstrap.sh - git diff --check --- scripts/lib/node-bootstrap.sh | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/scripts/lib/node-bootstrap.sh b/scripts/lib/node-bootstrap.sh index f4e82465c7b21..9853c1b4e48cd 100644 --- a/scripts/lib/node-bootstrap.sh +++ b/scripts/lib/node-bootstrap.sh @@ -2,7 +2,7 @@ # ============================================================================ # scripts/lib/node-bootstrap.sh # ---------------------------------------------------------------------------- -# Sourceable helper: ensure Node.js >= MIN_VERSION is available for the TUI +# Sourceable helper: ensure Node.js satisfies ^20.19 || >=22.12 for the TUI # (React + Ink), browser tools, and the WhatsApp bridge. # # Strategy (first hit wins — respects the user's existing tooling): @@ -18,12 +18,10 @@ # if [ "$HERMES_NODE_AVAILABLE" = true ]; then ...; fi # # Env inputs (set before sourcing to override defaults): -# HERMES_NODE_MIN_VERSION (default: 20) — accepted on PATH # HERMES_NODE_TARGET_MAJOR (default: 22) — installed when we install # HERMES_HOME (default: $HOME/.hermes) # ============================================================================ -HERMES_NODE_MIN_VERSION="${HERMES_NODE_MIN_VERSION:-20}" HERMES_NODE_TARGET_MAJOR="${HERMES_NODE_TARGET_MAJOR:-22}" HERMES_HOME="${HERMES_HOME:-$HOME/.hermes}" HERMES_NODE_AVAILABLE=false