fix: derive skip-key manifests from npm workspaces config

Review round 2 from @ethernet8023 on #61580:

1. The manifest list was a hardcoded root/ui-tui/web trio — desktop and
   any future workspace escaped the skip key even though step 1's root
   install hoists deps for every workspace. The list is now expanded
   from the root package.json 'workspaces' globs (npm's own source of
   truth): on the real repo that yields all 8 manifests incl.
   apps/desktop, apps/bootstrap-installer, apps/shared, and the nested
   ui-tui/packages/hermes-ink. Unreadable package.json falls back to
   root manifests only (never skips more than main would install).

2. --prefer-offline dropped entirely (this branch no longer carries
   #39399): local 3-run benchmarks on the repo's real manifests show
   the flag is noise on npm ci with a warm cache (root: 0.90s vs 0.84s
   avg; ws: 4.02s vs 4.00s avg) — npm ci does no resolution and the
   content-addressed cache already serves tarballs locally. It also
   carried the stale-resolution risk on the npm install fallback the
   reviewer flagged. All the real win is the skip itself (0s vs ~5s+).

Tests: workspace-glob edit (desktop), literal-listed edit, and
new-workspace-under-glob all defeat the skip; verified against the
real repo's workspace config (8 manifests picked up).

hermes_cli/main.py

@@ -8126,16 +8126,68 @@ def _ensure_uv_for_termux(pip_cmd: list[str]) -> str | None:
     return resolve_uv() or shutil.which("uv")
 
 
+def _npm_manifest_paths() -> tuple[Path, ...]:
+    """Manifests whose changes must defeat the update-skip.
+
+    The lockfile alone is NOT a sufficient key: on a local checkout a dev
+    can edit package.json (root or a workspace) without running npm — the
+    lockfile is then unchanged but `hermes update` is exactly the step
+    expected to sync node_modules (via the `npm install` fallback in
+    _run_npm_install_deterministic).
+
+    The workspace list is pulled from the root package.json's `workspaces`
+    globs (npm's own source of truth) rather than hardcoded, so adding a
+    workspace can never silently escape the skip key. The root install
+    (step 1, --workspaces=false) still hoists shared deps for EVERY
+    workspace — desktop included — so all of them belong in the key, not
+    just the ones step 2 installs. Falls back to hashing just root
+    manifests if package.json is unreadable (never skips more than main
+    would have installed).
+    """
+    root_pkg = PROJECT_ROOT / "package.json"
+    paths = [PROJECT_ROOT / "package-lock.json", root_pkg]
+    try:
+        workspaces = json.loads(root_pkg.read_text(encoding="utf-8")).get(
+            "workspaces", []
+        )
+        if isinstance(workspaces, dict):  # legacy {"packages": [...]} form
+            workspaces = workspaces.get("packages", [])
+        for pattern in workspaces:
+            for match in sorted(PROJECT_ROOT.glob(str(pattern))):
+                manifest = match / "package.json"
+                if manifest.is_file():
+                    paths.append(manifest)
+    except (OSError, json.JSONDecodeError, TypeError):
+        pass
+    return tuple(paths)
+
+
+def _npm_manifests_digest() -> str | None:
+    """Combined sha256 over the lockfile + all workspace package.json files.
+
+    Returns None when the lockfile is missing (never skip then).
+    """
+    if not (PROJECT_ROOT / "package-lock.json").exists():
+        return None
+    h = hashlib.sha256()
+    for p in _npm_manifest_paths():
+        h.update(str(p.relative_to(PROJECT_ROOT)).encode())
+        try:
+            h.update(p.read_bytes())
+        except OSError:
+            h.update(b"<missing>")
+    return h.hexdigest()
+
+
 def _npm_lockfile_changed(hermes_root: Path) -> bool:
-    lockfile = PROJECT_ROOT / "package-lock.json"
-    if not lockfile.exists():
+    current = _npm_manifests_digest()
+    if current is None:
         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}"
@@ -8147,11 +8199,10 @@ def _npm_lockfile_changed(hermes_root: Path) -> bool:
 
 
 def _record_npm_lockfile_hash(hermes_root: Path) -> None:
-    lockfile = PROJECT_ROOT / "package-lock.json"
-    if not lockfile.exists():
+    digest = _npm_manifests_digest()
+    if digest is None:
         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")

tests/hermes_cli/test_cmd_update.py

@@ -89,12 +89,10 @@ class TestCmdUpdateNpmLockfileCache:
     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 / "package-lock.json").write_text('{"lockfileVersion": 3}')
         (tmp_path / "node_modules").mkdir()
-        digest = hashlib.sha256(content).hexdigest()
-        self._cache_file(tmp_path, tmp_path).write_text(digest)
+        self._cache_file(tmp_path, tmp_path).write_text(hm._npm_manifests_digest())
 
         assert hm._npm_lockfile_changed(tmp_path) is False
 
@@ -123,14 +121,80 @@ class TestCmdUpdateNpmLockfileCache:
     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)
+        (tmp_path / "package-lock.json").write_text('{"lockfileVersion": 3}')
 
         hm._record_npm_lockfile_hash(tmp_path)
 
-        expected = hashlib.sha256(content).hexdigest()
-        assert self._cache_file(tmp_path, tmp_path).read_text() == expected
+        assert (
+            self._cache_file(tmp_path, tmp_path).read_text()
+            == hm._npm_manifests_digest()
+        )
+
+    def test_package_json_only_edit_defeats_skip(self, tmp_path, monkeypatch):
+        """Reviewer scenario (#61580): dev edits package.json WITHOUT running
+        npm — lockfile unchanged. `hermes update` must still install (the
+        npm-install fallback is what syncs node_modules in that state)."""
+        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 / "package.json").write_text('{"dependencies": {}}')
+        (tmp_path / "node_modules").mkdir()
+        hm._record_npm_lockfile_hash(tmp_path)
+        assert hm._npm_lockfile_changed(tmp_path) is False
+
+        (tmp_path / "package.json").write_text(
+            '{"dependencies": {"left-pad": "^1.0.0"}}'
+        )
+        assert hm._npm_lockfile_changed(tmp_path) is True
+
+    def test_workspace_package_json_edit_defeats_skip(self, tmp_path, monkeypatch):
+        """The manifest list comes from the root package.json `workspaces`
+        globs (npm's source of truth), so ANY workspace (desktop included)
+        defeats the skip, not a hardcoded set."""
+        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 / "package.json").write_text(
+            '{"workspaces": ["apps/*", "ui-tui"]}'
+        )
+        (tmp_path / "ui-tui").mkdir()
+        (tmp_path / "ui-tui" / "package.json").write_text("{}")
+        (tmp_path / "apps" / "desktop").mkdir(parents=True)
+        (tmp_path / "apps" / "desktop" / "package.json").write_text("{}")
+        (tmp_path / "node_modules").mkdir()
+        hm._record_npm_lockfile_hash(tmp_path)
+        assert hm._npm_lockfile_changed(tmp_path) is False
+
+        # A glob-matched workspace (desktop) defeats the skip…
+        (tmp_path / "apps" / "desktop" / "package.json").write_text(
+            '{"name": "desktop"}'
+        )
+        assert hm._npm_lockfile_changed(tmp_path) is True
+
+        # …and so does a literal-listed one.
+        hm._record_npm_lockfile_hash(tmp_path)
+        assert hm._npm_lockfile_changed(tmp_path) is False
+        (tmp_path / "ui-tui" / "package.json").write_text('{"name": "x"}')
+        assert hm._npm_lockfile_changed(tmp_path) is True
+
+    def test_new_workspace_added_defeats_skip(self, tmp_path, monkeypatch):
+        """Adding a whole new workspace dir under an existing glob changes
+        the manifest set itself — must also defeat the skip."""
+        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 / "package.json").write_text('{"workspaces": ["apps/*"]}')
+        (tmp_path / "node_modules").mkdir()
+        hm._record_npm_lockfile_hash(tmp_path)
+        assert hm._npm_lockfile_changed(tmp_path) is False
+
+        (tmp_path / "apps" / "newtool").mkdir(parents=True)
+        (tmp_path / "apps" / "newtool" / "package.json").write_text("{}")
+        assert hm._npm_lockfile_changed(tmp_path) is True
 
     def test_npm_lockfile_changed_cache_read_error(self, tmp_path, monkeypatch):
         from hermes_cli import main as hm