diff --git a/hermes_cli/main_desktop.py b/hermes_cli/main_desktop.py index fe8d9aee1b..b6406cd659 100644 --- a/hermes_cli/main_desktop.py +++ b/hermes_cli/main_desktop.py @@ -1423,14 +1423,15 @@ def _install_desktop_workspace_deps(npm: str, env: dict) -> None: """npm-install the desktop workspace; exits on a failure that isn't a repairable missing Electron dist.""" from hermes_cli.main import PROJECT_ROOT from hermes_cli.main_web_build import _run_npm_install_deterministic - from hermes_cli.update_cmd_deps import _clear_npm_lockfile_hash, _desktop_deps_changed, _record_npm_lockfile_hash + from hermes_cli.update_cmd_deps import ( + DESKTOP_NPM_SCOPE, _clear_npm_lockfile_hash, _desktop_deps_changed, _record_npm_lockfile_hash) from hermes_constants import get_default_hermes_root, with_hermes_node_path hermes_root = get_default_hermes_root() - if not _desktop_deps_changed(hermes_root): + if not _desktop_deps_changed(hermes_root) and (_electron_dir(PROJECT_ROOT) / "package.json").is_file(): print("→ Desktop workspace dependencies unchanged, skipping install") return print("→ Installing desktop workspace dependencies...") - _clear_npm_lockfile_hash(hermes_root, "_desktop") + _clear_npm_lockfile_hash(hermes_root, DESKTOP_NPM_SCOPE) _remove_half_installed_get_windows(PROJECT_ROOT) # Managed Node on PATH so npm's child scripts that shell out to bare `node` # (e.g. electron-winstaller's select-7z-arch.js) resolve it even when the @@ -1439,7 +1440,7 @@ def _install_desktop_workspace_deps(npm: str, env: dict) -> None: 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: - _record_npm_lockfile_hash(hermes_root, "_desktop") + _record_npm_lockfile_hash(hermes_root, DESKTOP_NPM_SCOPE) return if not _electron_pkg_staged_missing_dist(PROJECT_ROOT): print(f"✗ Desktop dependency install failed\n Run manually: cd {PROJECT_ROOT} && npm ci") diff --git a/hermes_cli/update_cmd_deps.py b/hermes_cli/update_cmd_deps.py index 6b024b47d9..8778260776 100644 --- a/hermes_cli/update_cmd_deps.py +++ b/hermes_cli/update_cmd_deps.py @@ -564,15 +564,15 @@ def _record_npm_lockfile_hash(hermes_root: Path, scope: str = "") -> None: logger.debug("Could not write npm lockfile hash cache") +# Stamp scope of the full-graph desktop install (pass 1's workspace-scoped stamp has none). +DESKTOP_NPM_SCOPE = "_desktop" + + def _desktop_deps_changed(hermes_root: Path) -> bool: - """True when the full-graph root ``npm ci`` the desktop build needs must run again: manifests - changed since its last success, or Electron is gone (the workspace-scoped pass-1 install prunes - it whenever it runs). See #43837.""" - from hermes_cli.update_cmd import _m + """True when the manifests changed since the full-graph desktop ``npm ci`` last succeeded (#43837). + The caller also re-installs when Electron is missing: pass 1 prunes it whenever it runs.""" current = _npm_manifests_digest() - if current is None or not (_m().PROJECT_ROOT / "node_modules" / "electron" / "package.json").is_file(): - return True - return not _npm_stamp_matches(hermes_root, current, "_desktop") + return current is None or not _npm_stamp_matches(hermes_root, current, DESKTOP_NPM_SCOPE) def _repair_node_deps_on_current_checkout( diff --git a/tests/hermes_cli/test_desktop_half_installed_get_windows.py b/tests/hermes_cli/test_desktop_half_installed_get_windows.py index 524d5ad9af..d30d4d0a45 100644 --- a/tests/hermes_cli/test_desktop_half_installed_get_windows.py +++ b/tests/hermes_cli/test_desktop_half_installed_get_windows.py @@ -5,9 +5,29 @@ mid-package, leaving the dir without ``package.json``; npm never revisits an exi the optional dep stayed broken on every later update until a manual repair. """ +import shutil +from pathlib import Path + from hermes_cli import main_desktop +def _desktop_checkout(tmp_path: Path, monkeypatch, *, electron_dir: Path | None = None) -> Path: + """A minimal checkout with a staged Electron package; returns the electron dir.""" + import hermes_cli.main as main_mod + monkeypatch.setenv("HERMES_HOME", str(tmp_path / "home")) + (tmp_path / "home").mkdir() + monkeypatch.setattr(main_mod, "PROJECT_ROOT", tmp_path) + monkeypatch.setattr(main_desktop, "_nixos_build_env", lambda: {}) + (tmp_path / "package.json").write_text('{"workspaces": ["apps/*"]}', encoding="utf-8") + (tmp_path / "package-lock.json").write_text("{}", encoding="utf-8") + (tmp_path / "apps" / "desktop").mkdir(parents=True) + (tmp_path / "apps" / "desktop" / "package.json").write_text("{}", encoding="utf-8") + electron = electron_dir or tmp_path / "node_modules" / "electron" + electron.mkdir(parents=True) + (electron / "package.json").write_text("{}", encoding="utf-8") + return electron + + def test_half_installed_dir_is_removed_and_a_complete_one_is_kept(tmp_path): half = tmp_path / "node_modules" / "get-windows" / "lib" / "binding" half.mkdir(parents=True) @@ -44,19 +64,10 @@ def test_install_removes_the_half_installed_dir_before_npm_runs(tmp_path, monkey def test_install_is_skipped_while_manifests_and_electron_are_unchanged(tmp_path, monkeypatch): """Root `npm ci` re-reifies the whole graph; only manifest changes or a pruned Electron warrant it. See #43837.""" - import hermes_cli.main as main_mod import hermes_cli.main_web_build as web_build from hermes_constants import get_default_hermes_root - monkeypatch.setenv("HERMES_HOME", str(tmp_path / "home")) - (tmp_path / "home").mkdir() - monkeypatch.setattr(main_mod, "PROJECT_ROOT", tmp_path) - monkeypatch.setattr(main_desktop, "_nixos_build_env", lambda: {}) - (tmp_path / "package.json").write_text('{"workspaces": ["apps/*"]}', encoding="utf-8") - (tmp_path / "package-lock.json").write_text("{}", encoding="utf-8") - (tmp_path / "apps" / "desktop").mkdir(parents=True) - (tmp_path / "apps" / "desktop" / "package.json").write_text("{}", encoding="utf-8") - (tmp_path / "node_modules" / "electron").mkdir(parents=True) - (tmp_path / "node_modules" / "electron" / "package.json").write_text("{}", encoding="utf-8") + # Electron under the workspace-local hoist, where electronDist points (_electron_dir). + electron = _desktop_checkout(tmp_path, monkeypatch, electron_dir=tmp_path / "apps" / "desktop" / "node_modules" / "electron") calls: list[Path] = [] monkeypatch.setattr(web_build, "_run_npm_install_deterministic", lambda npm, cwd, **kw: calls.append(cwd) or type("R", (), {"returncode": 0})()) @@ -64,11 +75,16 @@ def test_install_is_skipped_while_manifests_and_electron_are_unchanged(tmp_path, main_desktop._install_desktop_workspace_deps("npm", {}) main_desktop._install_desktop_workspace_deps("npm", {}) assert calls == [tmp_path], "second build with unchanged manifests must not npm ci again" - assert list(get_default_hermes_root().glob(".npm_lock_hash_*_desktop")), "stamp lives beside pass 1's" + + shutil.rmtree(electron) # pass 1 pruned Electron → the full install must run even with a matching stamp + main_desktop._install_desktop_workspace_deps("npm", {}) + assert calls == [tmp_path, tmp_path] + electron.mkdir(parents=True) + (electron / "package.json").write_text("{}", encoding="utf-8") (tmp_path / "apps" / "desktop" / "package.json").write_text('{"name": "bumped"}', encoding="utf-8") main_desktop._install_desktop_workspace_deps("npm", {}) - assert calls == [tmp_path, tmp_path], "a changed manifest must reinstall" + assert calls == [tmp_path] * 3, "a changed manifest must reinstall" def test_failed_reinstall_drops_the_stamp_so_the_next_update_repairs(tmp_path, monkeypatch): @@ -76,20 +92,9 @@ def test_failed_reinstall_drops_the_stamp_so_the_next_update_repairs(tmp_path, m must not leave an older matching stamp behind, or the half-installed tree is never revisited.""" import pytest - import hermes_cli.main as main_mod import hermes_cli.main_web_build as web_build from hermes_constants import get_default_hermes_root - monkeypatch.setenv("HERMES_HOME", str(tmp_path / "home")) - (tmp_path / "home").mkdir() - monkeypatch.setattr(main_mod, "PROJECT_ROOT", tmp_path) - monkeypatch.setattr(main_desktop, "_nixos_build_env", lambda: {}) - (tmp_path / "package.json").write_text('{"workspaces": ["apps/*"]}', encoding="utf-8") - (tmp_path / "package-lock.json").write_text("{}", encoding="utf-8") - (tmp_path / "apps" / "desktop").mkdir(parents=True) - (tmp_path / "apps" / "desktop" / "package.json").write_text("{}", encoding="utf-8") - electron = tmp_path / "node_modules" / "electron" - electron.mkdir(parents=True) - (electron / "package.json").write_text("{}", encoding="utf-8") + electron = _desktop_checkout(tmp_path, monkeypatch) codes = [0, 1, 0] monkeypatch.setattr(web_build, "_run_npm_install_deterministic", lambda npm, cwd, **kw: type("R", (), {"returncode": codes.pop(0)})()) @@ -100,6 +105,5 @@ def test_failed_reinstall_drops_the_stamp_so_the_next_update_repairs(tmp_path, m main_desktop._install_desktop_workspace_deps("npm", {}) # interrupted mid-extract (electron / "package.json").write_text("{}", encoding="utf-8") # partially reified tree - assert not list(get_default_hermes_root().glob(".npm_lock_hash_*_desktop")) main_desktop._install_desktop_workspace_deps("npm", {}) assert codes == [], "the next update must run the install again, not skip on the stale stamp"