fix(desktop): put Hermes-managed Node on PATH for install/rebuild (#66002)

Desktop launch and the update-chain rebuild install npm deps whose child
scripts shell out to a bare `node` (e.g. electron-winstaller's
select-7z-arch.js). When launched from the desktop updater chain
(Desktop -> hermes-setup -> hermes update) the shell PATH customizations are
lost, so the install dies with `'node' is not recognized` / `node: not found`.

- cmd_gui: wrap the npm-install env with with_hermes_node_path(_nixos_build_env())
  so managed Node is prepended even on a stripped PATH — mirrors the idiom
  already used by the update deps refresh. (The original fix merged nixos_env
  on TOP of the managed env, whose full os.environ copy clobbered the managed
  PATH back to bare; wrapping fixes that merge order.)
- _cmd_update_impl: spawn the `desktop --build-only` subprocess with
  with_hermes_node_path() so the child starts with managed Node from the outset.

Regression test: the desktop install env now prepends the managed Node dir
ahead of a bare updater PATH instead of passing env=None.

Co-authored-by: F4TB0Yz <jfduarte09@gmail.com>
This commit is contained in:
brooklyn! 2026-07-16 20:45:05 -04:00 committed by GitHub
parent 432fca55a7
commit dfb76d36d5
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
2 changed files with 66 additions and 4 deletions

View file

@ -5733,7 +5733,14 @@ def cmd_gui(args: argparse.Namespace):
print(f"✓ Desktop {build_label} is up to date (content stamp matches)")
else:
print("→ Installing desktop workspace dependencies...")
nixos_env = _nixos_build_env()
# Put the Hermes-managed Node on PATH so npm's child scripts (which
# shell out to bare `node`, e.g. electron-winstaller's
# select-7z-arch.js) resolve it even when the parent PATH is
# stripped — the desktop updater chain (Desktop → hermes-setup →
# hermes update) loses shell PATH customizations. Wrapping the
# NixOS build env keeps its PYTHON hint while restoring managed Node
# ahead of a bare PATH (same idiom as the `hermes update` path).
nixos_env = with_hermes_node_path(_nixos_build_env())
install_result = _run_npm_install_deterministic(npm, PROJECT_ROOT, capture_output=False, env=nixos_env)
if install_result.returncode != 0:
if not _electron_pkg_staged_missing_dist(PROJECT_ROOT):
@ -10283,9 +10290,18 @@ def _cmd_update_impl(args, gateway_mode: bool):
# covers a still-settling rebuild window the first wait didn't fully
# catch — then surface the captured tail so the failure is
# debuggable.
build_result = _run_logged_subprocess(_desktop_build_cmd, cwd=PROJECT_ROOT)
#
# Start the build subprocess with the Hermes-managed Node on PATH:
# when `hermes update` runs inside the desktop updater chain
# (Desktop → hermes-setup → hermes update), the shell PATH
# customizations are lost, so a bare-PATH child would fail with
# `node: not found` before cmd_gui can self-heal.
from hermes_constants import with_hermes_node_path
_build_env = with_hermes_node_path()
build_result = _run_logged_subprocess(_desktop_build_cmd, cwd=PROJECT_ROOT, env=_build_env)
if build_result.returncode != 0:
build_result = _run_logged_subprocess(_desktop_build_cmd, cwd=PROJECT_ROOT)
build_result = _run_logged_subprocess(_desktop_build_cmd, cwd=PROJECT_ROOT, env=_build_env)
if build_result.returncode != 0:
print(" ⚠ Desktop build failed (non-fatal; run `hermes desktop` to retry)")
tail = "\n".join((build_result.stdout or "").strip().splitlines()[-15:])

View file

@ -70,13 +70,59 @@ def test_gui_installs_packages_and_launches_desktop_app(tmp_path, monkeypatch):
cli_main.cmd_gui(_ns())
assert exc.value.code == 0
mock_install.assert_called_once_with("/usr/bin/npm", root, capture_output=False, env=None)
# The install now runs with a resolved env (managed-Node PATH), never a bare
# ``env=None`` that would leave npm's child scripts unable to find ``node``.
mock_install.assert_called_once()
assert mock_install.call_args.args == ("/usr/bin/npm", root)
assert mock_install.call_args.kwargs["capture_output"] is False
install_env = mock_install.call_args.kwargs["env"]
assert install_env is not None and "PATH" in install_env
assert mock_run.call_args_list[0].args[0] == ["/usr/bin/npm", "run", "pack"]
assert mock_run.call_args_list[0].kwargs["cwd"] == desktop_dir
assert mock_run.call_args_list[1].args[0] == [str(packaged_exe)]
assert mock_run.call_args_list[1].kwargs["cwd"] == desktop_dir
def test_gui_install_env_prepends_managed_node_on_bare_path(tmp_path, monkeypatch):
"""Regression: npm's child scripts (electron-winstaller's select-7z-arch.js)
shell out to bare ``node``. When Desktop is launched from the updater chain
the parent PATH is stripped, so the install env MUST carry the Hermes-managed
Node ahead of that bare PATH or the install dies with ``node: not found``.
"""
import os
from hermes_constants import iter_hermes_node_dirs
root = _make_desktop_tree(tmp_path)
monkeypatch.setattr(cli_main, "PROJECT_ROOT", root)
_make_packaged_executable(root, monkeypatch, platform="win32")
# A managed Node tree on disk so with_hermes_node_path() actually prepends it.
home = tmp_path / "hermes-home"
(home / "node" / "bin").mkdir(parents=True)
monkeypatch.setenv("HERMES_HOME", str(home))
# Simulate the stripped PATH the desktop updater chain hands us.
monkeypatch.setenv("PATH", os.pathsep.join(["/usr/bin", "/bin"]))
install_ok = subprocess.CompletedProcess(["npm", "ci"], 0)
launch_ok = subprocess.CompletedProcess(["hermes"], 0)
with patch("hermes_cli.main._resolve_node_runtime_npm", return_value="/usr/bin/npm"), \
patch("hermes_cli.main._run_npm_install_deterministic", return_value=install_ok) as mock_install, \
patch("hermes_cli.main._desktop_build_needed", return_value=True), \
patch("hermes_cli.main._write_desktop_build_stamp"), \
patch("hermes_cli.main.subprocess.run", side_effect=[subprocess.CompletedProcess([], 0), launch_ok]), \
pytest.raises(SystemExit):
cli_main.cmd_gui(_ns(skip_build=False))
managed_dirs = [str(p) for p in iter_hermes_node_dirs() if p.is_dir()]
assert managed_dirs, "managed node tree not discovered"
install_env = mock_install.call_args.kwargs["env"]
path_parts = install_env["PATH"].split(os.pathsep)
assert path_parts[: len(managed_dirs)] == managed_dirs
assert "/usr/bin" in path_parts # the bare updater PATH is preserved, just after managed Node
def test_gui_forwards_desktop_environment_overrides(tmp_path, monkeypatch):
root = _make_desktop_tree(tmp_path)
hermes_root = tmp_path / "custom-hermes"