From 150b197363451161308c0621385c7ca490d84108 Mon Sep 17 00:00:00 2001 From: fangliquanflq Date: Sun, 9 Aug 2026 15:32:02 +0800 Subject: [PATCH] fix(desktop): prioritize managed Node in install PATH --- hermes_constants.py | 18 ++++++++++---- tests/hermes_cli/test_gui_command.py | 19 +++++++++++---- tests/test_hermes_constants.py | 35 ++++++++++++++++++++++++++++ 3 files changed, 63 insertions(+), 9 deletions(-) diff --git a/hermes_constants.py b/hermes_constants.py index e7af1883970e..aa9b04e72b36 100644 --- a/hermes_constants.py +++ b/hermes_constants.py @@ -4,6 +4,7 @@ without risk of circular imports. """ +import ntpath import os import shutil import stat @@ -879,16 +880,25 @@ def find_node_executable(command: str) -> str | None: return find_node_executable_on_path(command) +def _node_path_comparison_key(path: str) -> str: + """Normalize a PATH entry using the active platform's path semantics.""" + path_module = ntpath if sys.platform == "win32" else os.path + return path_module.normcase(path_module.normpath(path)) + + 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) + managed_keys = {_node_path_comparison_key(entry) for entry in managed} + remaining = [ + entry + for entry in parts + if _node_path_comparison_key(entry) not in managed_keys + ] + merged["PATH"] = os.pathsep.join([*managed, *remaining]) return merged diff --git a/tests/hermes_cli/test_gui_command.py b/tests/hermes_cli/test_gui_command.py index 6d310bfd6aab..ee610c5ea36a 100644 --- a/tests/hermes_cli/test_gui_command.py +++ b/tests/hermes_cli/test_gui_command.py @@ -132,11 +132,18 @@ def test_gui_installs_packages_and_launches_desktop_app(tmp_path, monkeypatch): 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): +@pytest.mark.parametrize( + "managed_already_present", + [False, True], + ids=["missing", "after-system-node"], +) +def test_gui_install_env_prepends_managed_node( + tmp_path, monkeypatch, managed_already_present +): """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``. + the parent PATH can be stripped or put system Node first, so the install env + MUST carry Hermes-managed Node first or lifecycle scripts use the wrong Node. """ import os @@ -150,8 +157,10 @@ def test_gui_install_env_prepends_managed_node_on_bare_path(tmp_path, monkeypatc 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"])) + inherited_path = ["/usr/bin", "/bin"] + if managed_already_present: + inherited_path.extend([str(home / "node"), str(home / "node" / "bin")]) + monkeypatch.setenv("PATH", os.pathsep.join(inherited_path)) install_ok = subprocess.CompletedProcess(["npm", "ci"], 0) launch_ok = subprocess.CompletedProcess(["hermes"], 0) diff --git a/tests/test_hermes_constants.py b/tests/test_hermes_constants.py index 9df43e08ff9d..2d61aad4daf8 100644 --- a/tests/test_hermes_constants.py +++ b/tests/test_hermes_constants.py @@ -179,6 +179,41 @@ def test_windows_finds_npm_cmd_before_path(self, tmp_path, monkeypatch): assert find_hermes_node_executable("npm") == str(npm_cmd) + def test_managed_node_dirs_move_ahead_of_system_path(self, tmp_path, monkeypatch): + home = tmp_path / "hermes" + node_dir = home / "node" + bin_dir = node_dir / "bin" + bin_dir.mkdir(parents=True) + monkeypatch.setattr(hermes_constants.sys, "platform", "win32") + monkeypatch.setenv("HERMES_HOME", str(home)) + + env = with_hermes_node_path( + {"PATH": os.pathsep.join(["system-node", str(node_dir), "tools"])} + ) + + assert env["PATH"].split(os.pathsep) == [ + str(node_dir), + str(bin_dir), + "system-node", + "tools", + ] + + def test_windows_managed_node_path_collapses_normalized_duplicates( + self, tmp_path, monkeypatch + ): + home = tmp_path / "Hermes" + node_dir = home / "node" + node_dir.mkdir(parents=True) + monkeypatch.setattr(hermes_constants.sys, "platform", "win32") + monkeypatch.setenv("HERMES_HOME", str(home)) + duplicate = f"{str(node_dir).upper()}{os.sep}" + + env = with_hermes_node_path( + {"PATH": os.pathsep.join(["system-node", duplicate, str(node_dir)])} + ) + + assert env["PATH"].split(os.pathsep) == [str(node_dir), "system-node"] + @pytest.mark.windows_only