perf(cli): skip npm install during update when lockfile is unchanged (#17268)

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")

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 _fake_update_managed_uv():
         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 @@ def test_update_on_fork_checks_upstream_when_origin_up_to_date(
         ), 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