diff --git a/hermes_cli/main_web_build.py b/hermes_cli/main_web_build.py index f5b77ce99e..59dca07200 100644 --- a/hermes_cli/main_web_build.py +++ b/hermes_cli/main_web_build.py @@ -499,9 +499,17 @@ def _do_build_web_ui(web_dir: Path, *, fatal: bool = False) -> bool: # looks identical to a hang and users reboot mid-install). return _run_with_idle_timeout([npm, "run", "build"], cwd=web_dir, env=build_env) - r1 = _install_web_deps(silent=True) - if r1.returncode != 0: - return _report_web_build_failure("npm install", r1, fatal=fatal) + # `hermes update` already installed this exact closure and recorded the manifests digest after + # success; while it still matches (node_modules + toolchain checked inside), `npm ci` here would + # only wipe and re-reify the identical tree. Only the root lockfile is digested, so a web/ that + # owns its lockfile always installs. See #43837. + from hermes_cli.main import PROJECT_ROOT + from hermes_cli.update_cmd_deps import _npm_lockfile_changed + from hermes_constants import get_default_hermes_root + if npm_cwd != PROJECT_ROOT or _npm_lockfile_changed(get_default_hermes_root()): + r1 = _install_web_deps(silent=True) + if r1.returncode != 0: + return _report_web_build_failure("npm install", r1, fatal=fatal) r2 = _build() if r2.returncode != 0: # The install can exit 0 over a half-installed tree (lockfile-hash skip, diff --git a/tests/hermes_cli/test_web_ui_build.py b/tests/hermes_cli/test_web_ui_build.py index 91a96c4502..d19ec872da 100644 --- a/tests/hermes_cli/test_web_ui_build.py +++ b/tests/hermes_cli/test_web_ui_build.py @@ -419,3 +419,28 @@ class TestBuildRecoversFromMissingToolchain: assert mock_install.call_count == 1 assert mock_build.call_count == 1 + + +class TestBuildSkipsRedundantInstall: + """`hermes update` pass 1 installs the same closure; pass 2 must not `npm ci` it again. See #43837.""" + + @staticmethod + def _run(tmp_path, monkeypatch, *, lock_changed: bool) -> tuple[bool, int, int]: + import hermes_cli.main as main_mod + web_dir, _ = _make_web_dir(tmp_path) + (tmp_path / "package-lock.json").write_text("{}", encoding="utf-8") + monkeypatch.setattr(main_mod, "PROJECT_ROOT", tmp_path) + ok = __import__("subprocess").CompletedProcess([], 0, stdout="", stderr="") + with patch("hermes_cli.main_install_repair._resolve_node_runtime_npm", return_value="/usr/bin/npm"), \ + patch("hermes_cli.update_cmd_deps._npm_lockfile_changed", return_value=lock_changed), \ + patch("hermes_cli.main_web_build._run_npm_install_deterministic", return_value=ok) as mock_install, \ + patch("hermes_cli.main_web_build._run_with_idle_timeout", return_value=ok) as mock_build, \ + patch("hermes_cli.main_web_build._web_ui_build_needed", return_value=True), \ + patch("hermes_cli.main_web_build._write_web_ui_build_stamp"): + return _build_web_ui(web_dir), mock_install.call_count, mock_build.call_count + + def test_unchanged_manifests_build_without_reinstalling(self, tmp_path, monkeypatch): + assert self._run(tmp_path, monkeypatch, lock_changed=False) == (True, 0, 1) + + def test_changed_manifests_still_install_before_building(self, tmp_path, monkeypatch): + assert self._run(tmp_path, monkeypatch, lock_changed=True) == (True, 1, 1)