From def7bdc638e8024e458b6fbbd0a5196663c16596 Mon Sep 17 00:00:00 2001 From: andyst-dev <150129844+andyst-dev@users.noreply.github.com> Date: Wed, 12 Aug 2026 18:36:38 +0200 Subject: [PATCH] fix(cli): don't re-run npm install on every TUI launch with npm>=10 reduced hidden lockfile _tui_need_npm_install compared every field of the root package-lock.json against node_modules/.package-lock.json. npm>=10/11 writes a reduced hidden lockfile that omits declarative fields (version/dependencies/dev) and adds extraneous, so nearly every package looked 'changed'; workspace link entries ("link": true, paths outside node_modules/) are never materialized by the partial --workspace install. Both made the check return True forever, so hermes --tui re-ran npm install (and dirtied package-lock.json) on every launch (#84617). Compare only the keys both sides record with non-null values (resolved, integrity, ...), ignore workspace link entries and non-node_modules paths in the missing-entry check, and treat extraneous as an npm runtime annotation. Real skew (lockfile bumped while node_modules is behind) is still detected. --- hermes_cli/main.py | 27 ++++- tests/hermes_cli/test_tui_npm_install.py | 130 +++++++++++++++++++++++ 2 files changed, 153 insertions(+), 4 deletions(-) diff --git a/hermes_cli/main.py b/hermes_cli/main.py index 149fd59ead..8b4aa82809 100644 --- a/hermes_cli/main.py +++ b/hermes_cli/main.py @@ -2019,8 +2019,9 @@ installed, so we exclude them from the comparison in :func:`_tui_need_npm_instal to avoid false-positive reinstalls on every launch. ``dev``, ``optional``, ``extraneous``, and ``hasInstallScript`` are boolean -annotations that npm populates differently in the hidden lock (e.g. ``dev: true`` -from the root lock may be absent or ``false`` in the hidden actualized tree). +annotations that npm populates differently in the hidden lock (npm >= 10/11 +writes ``extraneous`` into the hidden lock only, and ``dev: true`` from the +root lock may be absent or ``false`` in the hidden actualized tree). They never indicate a changed dependency — the authoritative check is the ``resolved``/``integrity`` pair, which the intersection comparison always catches. @@ -2246,7 +2247,18 @@ def _tui_need_npm_install(root: Path) -> bool: return lock.stat().st_mtime > marker.stat().st_mtime def comparable(pkg: dict) -> dict: - return {k: v for k, v in pkg.items() if k not in _NPM_LOCK_RUNTIME_KEYS} + # npm >= 10/11 writes a reduced hidden lockfile that omits declarative + # fields (`version`, `dependencies`, `dev`, `engines`, `bin`, ...) or + # stores them as null. Comparing every key field-by-field then flags + # nearly every installed package as "changed". Compare only the keys + # both sides actually record with a non-null value (`resolved`, + # `integrity`, ...): a genuinely stale install (root lockfile bumped + # while node_modules is behind) still differs on those keys, while the + # reduced-lockfile field omissions no longer false-positive. + a = {k: v for k, v in pkg.items() if k not in _NPM_LOCK_RUNTIME_KEYS} + b = {k: v for k, v in installed_pkg.items() if k not in _NPM_LOCK_RUNTIME_KEYS} + common = a.keys() & b.keys() + return {k: a[k] for k in common if a[k] is not None and b[k] is not None} # In a shared workspace checkout the launch install is scoped to the ui-tui # workspace (plus its child packages/* workspaces on Termux), so only that @@ -2272,7 +2284,14 @@ def _tui_need_npm_install(root: Path) -> bool: continue if name not in installed: - if pkg.get("optional") or pkg.get("peer"): + # Workspace link entries (`"link": true`, paths outside + # node_modules/ like `apps/desktop`, `node_modules/web`) are never + # materialized by a partial `npm install --workspace ui-tui` — + # they're deliberately skipped (see #38772) and would otherwise + # force a reinstall on every launch. + if pkg.get("optional") or pkg.get("peer") or pkg.get("link"): + continue + if not name.startswith("node_modules/"): continue return True diff --git a/tests/hermes_cli/test_tui_npm_install.py b/tests/hermes_cli/test_tui_npm_install.py index 1edfbe03f3..49a3346722 100644 --- a/tests/hermes_cli/test_tui_npm_install.py +++ b/tests/hermes_cli/test_tui_npm_install.py @@ -1,5 +1,6 @@ """_tui_need_npm_install: auto npm when node_modules is behind the lockfile.""" +import json import os import types from pathlib import Path @@ -565,6 +566,135 @@ def test_make_tui_argv_exits_with_recovery_hint_when_workspace_unrecoverable( # for the install cwd — both use _workspace_root(sub). +def test_need_npm_install_false_with_reduced_npm11_hidden_lockfile( + tmp_path: Path, main_mod +) -> None: + """npm >= 10/11 writes a reduced hidden `.package-lock.json` that omits + declarative fields (version/dependencies/dev) and adds `extraneous`, + and it never materializes workspace `"link": true` entries. A fresh + install therefore used to look perpetually stale and re-ran `npm install` + on every TUI launch (#84617). After the fix it must be stable.""" + ws = tmp_path / "ui-tui" + ws.mkdir() + (ws / "package.json").write_text("{}") + _touch_ink(tmp_path) + (tmp_path / "package-lock.json").write_text( + json.dumps( + { + "packages": { + "node_modules/ink": { + "version": "5.0.0", + "resolved": "https://reg/ink.tgz", + "integrity": "sha512-aaaa", + "dependencies": {"yocto": "^1.0.0"}, + }, + "apps/desktop": {"link": True, "resolved": "apps/desktop"}, + } + } + ) + ) + # Hidden lockfile as npm 11 writes it: reduced, plus extraneous. + (tmp_path / "node_modules").mkdir(parents=True, exist_ok=True) + (tmp_path / "node_modules" / ".package-lock.json").write_text( + json.dumps( + { + "packages": { + "node_modules/ink": { + "resolved": "https://reg/ink.tgz", + "integrity": "sha512-aaaa", + "extraneous": True, + }, + "apps/desktop": {"link": True, "resolved": "apps/desktop"}, + } + } + ) + ) + + # Must be False: real skew keys (resolved/integrity) match, declarative + # omissions and extraneous are ignored, and the workspace link is skipped. + assert main_mod._tui_need_npm_install(ws) is False + + +def test_need_npm_install_true_when_resolved_drifts(tmp_path: Path, main_mod) -> None: + """A genuinely stale install (lockfile bumped the resolved URL/integrity + while node_modules is behind) must still be detected — the reduced-lockfile + fix must not paper over real skew (#84617).""" + ws = tmp_path / "ui-tui" + ws.mkdir() + (ws / "package.json").write_text("{}") + _touch_ink(tmp_path) + (tmp_path / "package-lock.json").write_text( + json.dumps( + { + "packages": { + "node_modules/ink": { + "version": "5.0.0", + "resolved": "https://reg/ink-NEW.tgz", + "integrity": "sha512-bbbb", + }, + } + } + ) + ) + (tmp_path / "node_modules").mkdir(parents=True, exist_ok=True) + (tmp_path / "node_modules" / ".package-lock.json").write_text( + json.dumps( + { + "packages": { + "node_modules/ink": { + "resolved": "https://reg/ink-OLD.tgz", + "integrity": "sha512-aaaa", + }, + } + } + ) + ) + + # resolved/integrity differ on both sides → must reinstall. + assert main_mod._tui_need_npm_install(ws) is True + + +def test_need_npm_install_true_when_regular_pkg_missing(tmp_path: Path, main_mod) -> None: + """A real non-link node_modules/ package missing from the install must + still trigger a reinstall — only workspace links and optional/peer skips + are exempt (#84617).""" + ws = tmp_path / "ui-tui" + ws.mkdir() + (ws / "package.json").write_text("{}") + _touch_ink(tmp_path) + (tmp_path / "package-lock.json").write_text( + json.dumps( + { + "packages": { + "node_modules/ink": { + "resolved": "https://reg/ink.tgz", + "integrity": "sha512-aaaa", + }, + "node_modules/missing-pkg": { + "resolved": "https://reg/missing.tgz", + "integrity": "sha512-cccc", + }, + } + } + ) + ) + (tmp_path / "node_modules").mkdir(parents=True, exist_ok=True) + (tmp_path / "node_modules" / ".package-lock.json").write_text( + json.dumps( + { + "packages": { + "node_modules/ink": { + "resolved": "https://reg/ink.tgz", + "integrity": "sha512-aaaa", + }, + } + } + ) + ) + + assert main_mod._tui_need_npm_install(ws) is True + + def test_no_stray_lockfiles_in_workspace_subdirs(main_mod) -> None: """Workspace sub-directories must not contain their own package-lock.json.