From 8fb6d5e910b6fd89bdc698c477cc5f039a0deabd Mon Sep 17 00:00:00 2001 From: Rod Boev Date: Thu, 4 Jun 2026 19:28:55 -0400 Subject: [PATCH] perf(cli): skip npm install during update when lockfile is unchanged (#17268) --- hermes_cli/main.py | 41 +++++++++++++ tests/hermes_cli/test_cmd_update.py | 92 ++++++++++++++++++++++++++++- 2 files changed, 132 insertions(+), 1 deletion(-) diff --git a/hermes_cli/main.py b/hermes_cli/main.py index 6697435786bf..750795d63970 100644 --- a/hermes_cli/main.py +++ b/hermes_cli/main.py @@ -8076,6 +8076,39 @@ def _ensure_uv_for_termux(pip_cmd: list[str]) -> str | None: return resolve_uv() or shutil.which("uv") +def _npm_lockfile_changed(hermes_root: Path) -> bool: + lockfile = PROJECT_ROOT / "package-lock.json" + if not lockfile.exists(): + return True + # Also check that node_modules exists; a matching hash with missing + # node_modules means the cache was recorded by another checkout. + if not (PROJECT_ROOT / "node_modules").is_dir(): + return True + try: + current = hashlib.sha256(lockfile.read_bytes()).hexdigest() + # Key the cache by PROJECT_ROOT so parallel worktrees don't collide. + cache_key = hashlib.sha256(str(PROJECT_ROOT).encode()).hexdigest()[:12] + cache_file = hermes_root / f".npm_lock_hash_{cache_key}" + if not cache_file.exists(): + return True + return cache_file.read_text(encoding="utf-8").strip() != current + except OSError: + return True + + +def _record_npm_lockfile_hash(hermes_root: Path) -> None: + lockfile = PROJECT_ROOT / "package-lock.json" + if not lockfile.exists(): + return + try: + digest = hashlib.sha256(lockfile.read_bytes()).hexdigest() + cache_key = hashlib.sha256(str(PROJECT_ROOT).encode()).hexdigest()[:12] + cache_file = hermes_root / f".npm_lock_hash_{cache_key}" + cache_file.write_text(digest, encoding="utf-8") + except OSError: + logger.debug("Could not write npm lockfile hash cache") + + def _update_node_dependencies() -> None: from hermes_constants import find_node_executable, with_hermes_node_path @@ -8086,6 +8119,13 @@ def _update_node_dependencies() -> None: if not (PROJECT_ROOT / "package.json").exists(): return + from hermes_constants import get_default_hermes_root + + hermes_root = get_default_hermes_root() + if not _npm_lockfile_changed(hermes_root): + logger.info("npm lockfile unchanged, skipping npm install") + return + # With a single workspace lockfile the root install would cover ALL # workspaces — but apps/desktop pulls in Electron as a devDependency, # and its postinstall downloads a ~200MB binary. Most users don't @@ -8130,6 +8170,7 @@ def _update_node_dependencies() -> None: env=nixos_env, ) if ws_result.returncode == 0: + _record_npm_lockfile_hash(hermes_root) print(" ✓ repo root + ui-tui, web workspaces (desktop skipped)") else: print(" ⚠ npm workspace install failed") diff --git a/tests/hermes_cli/test_cmd_update.py b/tests/hermes_cli/test_cmd_update.py index 56671aa7ecc5..d8643fb43b62 100644 --- a/tests/hermes_cli/test_cmd_update.py +++ b/tests/hermes_cli/test_cmd_update.py @@ -1,5 +1,6 @@ """Tests for cmd_update — branch fallback when remote branch doesn't exist.""" +import hashlib import subprocess from types import SimpleNamespace from unittest.mock import patch @@ -70,6 +71,92 @@ def _patch_managed_uv(request): yield +class TestCmdUpdateNpmLockfileCache: + @staticmethod + def _cache_file(hermes_root, project_root): + cache_key = hashlib.sha256(str(project_root).encode()).hexdigest()[:12] + return hermes_root / f".npm_lock_hash_{cache_key}" + + def test_npm_lockfile_changed_no_cache(self, tmp_path, monkeypatch): + from hermes_cli import main as hm + + monkeypatch.setattr(hm, "PROJECT_ROOT", tmp_path) + (tmp_path / "package-lock.json").write_text('{"lockfileVersion": 3}') + (tmp_path / "node_modules").mkdir() + + assert hm._npm_lockfile_changed(tmp_path) is True + + def test_npm_lockfile_changed_matching(self, tmp_path, monkeypatch): + from hermes_cli import main as hm + + content = b'{"lockfileVersion": 3}' + monkeypatch.setattr(hm, "PROJECT_ROOT", tmp_path) + (tmp_path / "package-lock.json").write_bytes(content) + (tmp_path / "node_modules").mkdir() + digest = hashlib.sha256(content).hexdigest() + self._cache_file(tmp_path, tmp_path).write_text(digest) + + assert hm._npm_lockfile_changed(tmp_path) is False + + def test_npm_lockfile_changed_mismatch(self, tmp_path, monkeypatch): + from hermes_cli import main as hm + + monkeypatch.setattr(hm, "PROJECT_ROOT", tmp_path) + (tmp_path / "package-lock.json").write_text('{"lockfileVersion": 3}') + (tmp_path / "node_modules").mkdir() + self._cache_file(tmp_path, tmp_path).write_text("old-digest") + + assert hm._npm_lockfile_changed(tmp_path) is True + + def test_npm_lockfile_changed_missing_node_modules(self, tmp_path, monkeypatch): + from hermes_cli import main as hm + + content = b'{"lockfileVersion": 3}' + monkeypatch.setattr(hm, "PROJECT_ROOT", tmp_path) + (tmp_path / "package-lock.json").write_bytes(content) + digest = hashlib.sha256(content).hexdigest() + self._cache_file(tmp_path, tmp_path).write_text(digest) + # node_modules missing: should report changed even though hash matches + + assert hm._npm_lockfile_changed(tmp_path) is True + + def test_record_npm_lockfile_hash(self, tmp_path, monkeypatch): + from hermes_cli import main as hm + + content = b'{"lockfileVersion": 3}' + monkeypatch.setattr(hm, "PROJECT_ROOT", tmp_path) + (tmp_path / "package-lock.json").write_bytes(content) + + hm._record_npm_lockfile_hash(tmp_path) + + expected = hashlib.sha256(content).hexdigest() + assert self._cache_file(tmp_path, tmp_path).read_text() == expected + + def test_npm_lockfile_changed_cache_read_error(self, tmp_path, monkeypatch): + from hermes_cli import main as hm + + monkeypatch.setattr(hm, "PROJECT_ROOT", tmp_path) + (tmp_path / "package-lock.json").write_text('{"lockfileVersion": 3}') + (tmp_path / "node_modules").mkdir() + # Make cache file a directory to cause OSError on read + self._cache_file(tmp_path, tmp_path).mkdir(parents=True) + + assert hm._npm_lockfile_changed(tmp_path) is True + + def test_update_skips_npm_when_lockfile_unchanged(self, tmp_path, monkeypatch): + from hermes_cli import main as hm + + monkeypatch.setattr(hm, "PROJECT_ROOT", tmp_path) + (tmp_path / "package.json").write_text("{}") + + with patch("shutil.which", return_value="/usr/bin/npm"), \ + patch.object(hm, "_npm_lockfile_changed", return_value=False), \ + patch("subprocess.run") as mock_run: + hm._update_node_dependencies() + + mock_run.assert_not_called() + + class TestCmdUpdatePip: """Regression tests for pip-install update flows.""" @@ -246,7 +333,10 @@ class TestCmdUpdateBranchFallback: ), patch.object(hm, "_sync_with_upstream_if_needed") as sync_mock: cmd_update(mock_args) - sync_mock.assert_called_once_with(["git"], PROJECT_ROOT) + expected_git_cmd = ( + ["git", "-c", "windows.appendAtomically=false"] if hm._is_windows() else ["git"] + ) + sync_mock.assert_called_once_with(expected_git_cmd, PROJECT_ROOT) captured = capsys.readouterr() assert "Already up to date!" in captured.out