From 7a7b56d49830682d9c7ec1dbbbe2ec9d99b8eff3 Mon Sep 17 00:00:00 2001 From: helix4u <4317663+helix4u@users.noreply.github.com> Date: Fri, 19 Jun 2026 13:55:15 -0600 Subject: [PATCH] fix(windows): prefer managed node for whatsapp and desktop --- apps/desktop/electron/main.cjs | 26 ++++++++++---- gateway/platforms/whatsapp.py | 26 +++++++------- hermes_cli/main.py | 43 ++++++++++++++++------ hermes_constants.py | 66 ++++++++++++++++++++++++++++++++++ tests/test_hermes_constants.py | 43 +++++++++++++++++++++- 5 files changed, 173 insertions(+), 31 deletions(-) diff --git a/apps/desktop/electron/main.cjs b/apps/desktop/electron/main.cjs index be89c6c91cf..3961760bcaa 100644 --- a/apps/desktop/electron/main.cjs +++ b/apps/desktop/electron/main.cjs @@ -268,6 +268,20 @@ function resolveHermesHome() { } const HERMES_HOME = resolveHermesHome() + +function hermesManagedNodePathEntries() { + const root = path.join(HERMES_HOME, 'node') + const bin = path.join(root, 'bin') + const entries = IS_WINDOWS ? [root, bin] : [bin, root] + return entries.filter(directoryExists) +} + +function pathWithHermesManagedNode(...entries) { + return [...hermesManagedNodePathEntries(), ...entries, process.env.PATH] + .filter(Boolean) + .join(path.delimiter) +} + // ACTIVE_HERMES_ROOT — the canonical mutable Hermes install. Same path // install.ps1 / install.sh use, so a desktop-only user and a CLI-only user end // up with identical layouts and can share one install. @@ -1827,7 +1841,7 @@ async function applyUpdates(opts = {}) { env: { ...process.env, HERMES_HOME, - PATH: [path.join(HERMES_HOME, 'node', 'bin'), venvBin, process.env.PATH].filter(Boolean).join(path.delimiter) + PATH: pathWithHermesManagedNode(venvBin) }, detached: true, stdio: 'ignore', @@ -1871,7 +1885,7 @@ async function handOffWindowsBootstrapRecovery(reason) { env: { ...process.env, HERMES_HOME, - PATH: [path.join(HERMES_HOME, 'node', 'bin'), venvBin, process.env.PATH].filter(Boolean).join(path.delimiter) + PATH: pathWithHermesManagedNode(venvBin) }, detached: true, stdio: 'ignore', @@ -1952,13 +1966,11 @@ async function applyUpdatesPosixInApp() { } // Put the Hermes-managed Node and the venv on PATH so `hermes desktop`'s - // npm build can find them on a machine with no system Node. - const extraPath = [path.join(HERMES_HOME, 'node', 'bin'), path.join(updateRoot, 'venv', 'bin')] - .filter(Boolean) - .join(path.delimiter) + // npm build can find them on a machine with no system Node. Windows portable + // Node lives directly under %LOCALAPPDATA%\hermes\node, not node\bin. const env = { HERMES_HOME, - PATH: [extraPath, process.env.PATH].filter(Boolean).join(path.delimiter) + PATH: pathWithHermesManagedNode(path.join(updateRoot, 'venv', 'bin')) } // `hermes update` reaps stale `hermes dashboard` backends (a code update diff --git a/gateway/platforms/whatsapp.py b/gateway/platforms/whatsapp.py index 00ff2c967e7..9e18500c49b 100644 --- a/gateway/platforms/whatsapp.py +++ b/gateway/platforms/whatsapp.py @@ -19,7 +19,6 @@ import asyncio import logging import os import platform -import shutil import signal import subprocess @@ -27,7 +26,11 @@ _IS_WINDOWS = platform.system() == "Windows" from pathlib import Path from typing import Dict, Optional, Any -from hermes_constants import get_hermes_dir +from hermes_constants import ( + find_node_executable, + get_hermes_dir, + with_hermes_node_path, +) logger = logging.getLogger(__name__) @@ -212,10 +215,9 @@ def check_whatsapp_requirements() -> bool: WhatsApp requires a Node.js bridge for most implementations. """ - # Check for Node.js. Resolve via shutil.which so we respect PATHEXT - # (node.exe vs node) and get a meaningful "not installed" signal - # instead of spawning a cmd flash on Windows. - _node = shutil.which("node") + # Prefer Hermes-managed Node/npm so Windows installs are not broken by a + # bad or elevation-triggering system Node on PATH. + _node = find_node_executable("node") if not _node: return False try: @@ -404,10 +406,9 @@ class WhatsAppAdapter(WhatsAppBehaviorMixin, BasePlatformAdapter): _deps_fresh = False if not _deps_fresh: print(f"[{self.name}] Installing WhatsApp bridge dependencies...") - # Resolve npm path so Windows can execute the .cmd shim. - # shutil.which honours PATHEXT; on POSIX it returns the - # plain executable path. - _npm_bin = shutil.which("npm") or "npm" + # Resolve npm path so Windows uses npm.cmd from the + # Hermes-managed portable Node before falling back to PATH. + _npm_bin = find_node_executable("npm") or "npm" try: # Read timeout from environment variable, default to 300 seconds (5 minutes) # to accommodate slower systems like Unraid NAS @@ -418,6 +419,7 @@ class WhatsAppAdapter(WhatsAppBehaviorMixin, BasePlatformAdapter): capture_output=True, text=True, timeout=npm_install_timeout, + env=with_hermes_node_path(), ) if install_result.returncode != 0: print(f"[{self.name}] npm install failed: {install_result.stderr}") @@ -490,7 +492,7 @@ class WhatsAppAdapter(WhatsAppBehaviorMixin, BasePlatformAdapter): # Build bridge subprocess environment. # Pass WHATSAPP_REPLY_PREFIX from config.yaml so the Node bridge # can use it without the user needing to set a separate env var. - bridge_env = os.environ.copy() + bridge_env = with_hermes_node_path(os.environ.copy()) if self._reply_prefix is not None: bridge_env["WHATSAPP_REPLY_PREFIX"] = self._reply_prefix # Pass the profile-aware cache directories so the bridge writes @@ -508,7 +510,7 @@ class WhatsAppAdapter(WhatsAppBehaviorMixin, BasePlatformAdapter): self._bridge_process = subprocess.Popen( [ - "node", + find_node_executable("node") or "node", str(bridge_path), "--port", str(self._bridge_port), "--session", str(self._session_path), diff --git a/hermes_cli/main.py b/hermes_cli/main.py index 039eb5d449c..c2b5985c232 100644 --- a/hermes_cli/main.py +++ b/hermes_cli/main.py @@ -2363,6 +2363,7 @@ def cmd_whatsapp(args): """Set up WhatsApp: choose mode, configure, install bridge, pair via QR.""" _require_tty("whatsapp") from hermes_cli.config import get_env_value, save_env_value + from hermes_constants import find_node_executable, with_hermes_node_path print() print("⚕ WhatsApp Setup") @@ -2477,7 +2478,7 @@ def cmd_whatsapp(args): print( "\n→ Installing WhatsApp bridge dependencies (this can take a few minutes)..." ) - npm = shutil.which("npm") + npm = find_node_executable("npm") if not npm: print(" ✗ npm not found on PATH — install Node.js first") return @@ -2490,6 +2491,7 @@ def cmd_whatsapp(args): text=True, encoding="utf-8", errors="replace", + env=with_hermes_node_path(), ) except KeyboardInterrupt: print("\n ✗ Install cancelled") @@ -2546,8 +2548,15 @@ def cmd_whatsapp(args): try: subprocess.run( - ["node", str(bridge_script), "--pair-only", "--session", str(session_dir)], + [ + find_node_executable("node") or "node", + str(bridge_script), + "--pair-only", + "--session", + str(session_dir), + ], cwd=str(bridge_dir), + env=with_hermes_node_path(), ) except KeyboardInterrupt: pass @@ -4535,6 +4544,7 @@ def _run_with_idle_timeout( *, idle_timeout_seconds: int = 180, indent: str = " ", + env: dict[str, str] | None = None, ) -> subprocess.CompletedProcess: """Run a subprocess that streams output, with an idle-output timeout. @@ -4569,6 +4579,7 @@ def _run_with_idle_timeout( encoding="utf-8", errors="replace", bufsize=1, + env=env, ) except OSError as exc: # E.g. npm not on PATH between the which() check and now. @@ -4760,12 +4771,15 @@ def _build_web_ui(web_dir: Path, *, fatal: bool = False) -> bool: encoding = getattr(sys.stdout, "encoding", None) or "ascii" print(text.encode(encoding, errors="replace").decode(encoding, errors="replace")) - npm = shutil.which("npm") + from hermes_constants import find_node_executable, with_hermes_node_path + + npm = find_node_executable("npm") if not npm: if fatal: _say("Web UI frontend not built and npm is not available.") _say("Install Node.js, then run: cd web && npm install && npm run build") return not fatal + build_env = with_hermes_node_path() _say("→ Building web UI...") def _relay(result: "subprocess.CompletedProcess") -> None: @@ -4797,6 +4811,7 @@ def _build_web_ui(web_dir: Path, *, fatal: bool = False) -> bool: npm, npm_cwd, extra_args=(*npm_workspace_args, "--silent"), + env=build_env, ) if r1.returncode != 0: _say( @@ -4812,13 +4827,13 @@ def _build_web_ui(web_dir: Path, *, fatal: bool = False) -> bool: # users react by rebooting, which leaves the editable install in a # half-state. Streaming + idle-kill makes failures observable AND # recoverable (the stale-dist fallback below handles the kill path). - r2 = _run_with_idle_timeout([npm, "run", "build"], cwd=web_dir) + r2 = _run_with_idle_timeout([npm, "run", "build"], cwd=web_dir, env=build_env) if r2.returncode != 0: # Retry once after a short delay — covers boot-time races on Windows # (antivirus scanning Node.js binaries, npm cache not ready, transient # I/O when launched via Scheduled Task at logon). See issue #23817. _time.sleep(3) - r2 = _run_with_idle_timeout([npm, "run", "build"], cwd=web_dir) + r2 = _run_with_idle_timeout([npm, "run", "build"], cwd=web_dir, env=build_env) if r2.returncode != 0: # _run_with_idle_timeout merges stderr into stdout; older callers @@ -5197,7 +5212,9 @@ def _redownload_electron_dist( installer = electron_dir / "install.js" if not installer.is_file(): return False - node = shutil.which("node") + from hermes_constants import find_node_executable, with_hermes_node_path + + node = find_node_executable("node") if not node: return False @@ -5208,7 +5225,7 @@ def _redownload_electron_dist( except OSError: pass - dl_env = dict(env) + dl_env = with_hermes_node_path(env) if mirror: dl_env["ELECTRON_MIRROR"] = mirror try: @@ -5388,7 +5405,9 @@ def cmd_gui(args: argparse.Namespace): except Exception: pass - env = os.environ.copy() + from hermes_constants import find_node_executable, with_hermes_node_path + + env = with_hermes_node_path(os.environ.copy()) if getattr(args, "fake_boot", False): env["HERMES_DESKTOP_BOOT_FAKE"] = "1" if getattr(args, "ignore_existing", False): @@ -5405,7 +5424,7 @@ def cmd_gui(args: argparse.Namespace): packaged_executable = _desktop_packaged_executable(desktop_dir) if source_mode or not skip_build: - npm = shutil.which("npm") + npm = find_node_executable("npm") if not npm: print("Desktop GUI requires Node.js/npm, but npm was not found on PATH.") print("Install Node.js, then run: hermes gui") @@ -7637,7 +7656,9 @@ def _ensure_uv_for_termux(pip_cmd: list[str]) -> str | None: def _update_node_dependencies() -> None: - npm = shutil.which("npm") + from hermes_constants import find_node_executable, with_hermes_node_path + + npm = find_node_executable("npm") if not npm: return @@ -7654,7 +7675,7 @@ def _update_node_dependencies() -> None: print("→ Updating Node.js dependencies...") extra_args = ["--no-fund", "--no-audit", "--progress=false"] - nixos_env = _nixos_build_env() + nixos_env = with_hermes_node_path(_nixos_build_env()) # Step 1: root install (no workspace recursion). root_args = [*extra_args, "--workspaces=false"] diff --git a/hermes_constants.py b/hermes_constants.py index a80e9763148..48be65d2781 100644 --- a/hermes_constants.py +++ b/hermes_constants.py @@ -5,6 +5,7 @@ without risk of circular imports. """ import os +import shutil import sys import sysconfig from contextvars import ContextVar, Token @@ -242,6 +243,71 @@ def get_hermes_dir(new_subpath: str, old_name: str) -> Path: return home / new_subpath +def iter_hermes_node_dirs(home: Path | None = None) -> list[Path]: + """Return Hermes-managed Node.js directories in preferred lookup order. + + Windows installs from ``scripts/install.ps1`` unpack portable Node directly + into ``%LOCALAPPDATA%\\hermes\\node``. POSIX installs use + ``$HERMES_HOME/node/bin``. Include both shapes on every platform so mixed + or migrated installs still work. + """ + root = home or get_hermes_home() + dirs = [root / "node"] + bin_dir = root / "node" / "bin" + if sys.platform == "win32": + return dirs + [bin_dir] + return [bin_dir] + dirs + + +def _candidate_node_command_names(command: str) -> list[str]: + base = Path(command).name + if sys.platform != "win32" or "." in base: + return [base] + if base.lower() == "npm": + # Prefer npm.cmd. PowerShell may block npm.ps1 by execution policy, and + # CreateProcess cannot launch a bare .ps1 the way it can launch .cmd. + return ["npm.cmd", "npm.exe", "npm"] + if base.lower() == "npx": + return ["npx.cmd", "npx.exe", "npx"] + if base.lower() == "node": + return ["node.exe", "node"] + return [f"{base}.cmd", f"{base}.exe", base] + + +def find_hermes_node_executable(command: str) -> str | None: + """Return a Hermes-managed Node/npm executable path, if installed.""" + for directory in iter_hermes_node_dirs(): + for name in _candidate_node_command_names(command): + candidate = directory / name + if candidate.is_file() and ( + sys.platform == "win32" or os.access(candidate, os.X_OK) + ): + return str(candidate) + return None + + +def find_node_executable(command: str) -> str | None: + """Resolve a Node.js command, preferring Hermes-managed installs. + + This is for Hermes-owned subprocesses that should not be broken by a bad, + missing, or elevation-triggering system Node/npm on PATH. + """ + return find_hermes_node_executable(command) or shutil.which(command) + + +def with_hermes_node_path(env: dict[str, str] | None = None) -> dict[str, str]: + """Return *env* with Hermes-managed Node directories prepended to PATH.""" + merged = dict(os.environ if env is None else env) + existing = merged.get("PATH", "") + parts = [p for p in existing.split(os.pathsep) if p] + managed = [str(path) for path in iter_hermes_node_dirs() if path.is_dir()] + for entry in reversed(managed): + if entry not in parts: + parts.insert(0, entry) + merged["PATH"] = os.pathsep.join(parts) + return merged + + def display_hermes_home() -> str: """Return a user-friendly display string for the current HERMES_HOME. diff --git a/tests/test_hermes_constants.py b/tests/test_hermes_constants.py index 0a9dcce3651..a3c2a03a304 100644 --- a/tests/test_hermes_constants.py +++ b/tests/test_hermes_constants.py @@ -8,11 +8,14 @@ import pytest import hermes_constants from hermes_constants import ( VALID_REASONING_EFFORTS, + find_hermes_node_executable, get_default_hermes_root, get_hermes_home, + iter_hermes_node_dirs, is_container, parse_reasoning_effort, secure_parent_dir, + with_hermes_node_path, ) @@ -105,6 +108,45 @@ class TestGetHermesHome: assert get_hermes_home() == local_appdata / "hermes" +class TestHermesManagedNode: + def test_windows_node_dir_prefers_portable_root(self, tmp_path, monkeypatch): + home = tmp_path / "hermes" + node_dir = home / "node" + bin_dir = node_dir / "bin" + node_dir.mkdir(parents=True) + bin_dir.mkdir() + monkeypatch.setattr(hermes_constants.sys, "platform", "win32") + monkeypatch.setenv("HERMES_HOME", str(home)) + + assert iter_hermes_node_dirs() == [node_dir, bin_dir] + + def test_windows_finds_npm_cmd_before_path(self, tmp_path, monkeypatch): + home = tmp_path / "hermes" + node_dir = home / "node" + node_dir.mkdir(parents=True) + npm_cmd = node_dir / "npm.cmd" + npm_cmd.write_text("@echo off\n") + monkeypatch.setattr(hermes_constants.sys, "platform", "win32") + monkeypatch.setenv("HERMES_HOME", str(home)) + + assert find_hermes_node_executable("npm") == str(npm_cmd) + + def test_with_hermes_node_path_prepends_existing_managed_dirs(self, tmp_path, monkeypatch): + home = tmp_path / "hermes" + node_dir = home / "node" + bin_dir = node_dir / "bin" + node_dir.mkdir(parents=True) + bin_dir.mkdir() + monkeypatch.setattr(hermes_constants.sys, "platform", "win32") + monkeypatch.setenv("HERMES_HOME", str(home)) + + env = with_hermes_node_path({"PATH": "system-node"}) + parts = env["PATH"].split(os.pathsep) + + assert parts[:2] == [str(node_dir), str(bin_dir)] + assert parts[-1] == "system-node" + + class TestIsContainer: """Tests for is_container() — Docker/Podman detection.""" @@ -351,4 +393,3 @@ class TestSecureParentDir: secure_parent_dir(link_target) assert len(called_with) == 1 assert called_with[0] == (str(real_dir), 0o700) -