Merge pull request #19520 from NousResearch/fix_docker_tui

fix(docker/tui): tolerate npm's peer-flag drop in lockfile comparison
This commit is contained in:
Ben Barclay
2026-05-04 14:29:43 +10:00
committed by GitHub
2 changed files with 51 additions and 4 deletions

View File

@ -837,7 +837,17 @@ def _print_tui_exit_summary(session_id: Optional[str], active_session_file: Opti
)
_NPM_LOCK_RUNTIME_KEYS = frozenset({"ideallyInert"})
_NPM_LOCK_RUNTIME_KEYS = frozenset({"ideallyInert", "peer"})
"""Lockfile fields npm writes non-deterministically at install time.
``ideallyInert`` is npm's runtime annotation for packages it skipped installing
(per-platform opt-outs). ``peer`` is dropped from the hidden ``.package-lock.json``
on dev-dependencies that are *also* declared as peers — the canonical
``package-lock.json`` records the dual role, but npm 9's actualized tree strips
it. Neither key represents a real skew between what was declared and what was
installed, so we exclude them from the comparison in :func:`_tui_need_npm_install`
to avoid false-positive reinstalls on every launch.
"""
def _tui_need_npm_install(root: Path) -> bool:
@ -1042,17 +1052,21 @@ def _make_tui_argv(tui_dir: Path, tui_dev: bool) -> tuple[list[str], Path]:
if _tui_need_npm_install(tui_dir):
if not os.environ.get("HERMES_QUIET"):
print("Installing TUI dependencies…")
# Capture stdout as well as stderr — some npm errors (notably EACCES on a
# root-owned node_modules in containers) are emitted on stdout, and a
# bare "npm install failed." with no preview defeats debugging. We keep
# the failure-only print path so a successful install stays silent.
result = subprocess.run(
[npm, "install", "--silent", "--no-fund", "--no-audit", "--progress=false"],
cwd=str(tui_dir),
stdout=subprocess.DEVNULL,
stdout=subprocess.PIPE,
stderr=subprocess.PIPE,
text=True,
env={**os.environ, "CI": "1"},
)
if result.returncode != 0:
err = (result.stderr or "").strip()
preview = "\n".join(err.splitlines()[-30:])
combined = f"{result.stdout or ''}\n{result.stderr or ''}".strip()
preview = "\n".join(combined.splitlines()[-30:])
print("npm install failed.")
if preview:
print(preview)

View File

@ -69,6 +69,39 @@ def test_no_install_when_only_optional_peer_package_missing_from_hidden_lock(tmp
assert main_mod._tui_need_npm_install(tmp_path) is False
def test_no_install_when_only_peer_annotation_differs(tmp_path: Path, main_mod) -> None:
"""npm 9 drops the ``peer`` flag from the hidden lock on dev-deps that are
*also* declared as peers. That's a cosmetic difference — the package is
installed at the requested version — so it must not trigger a reinstall.
Regression for the TUI-in-Docker failure where 16 such mismatches caused
`Installing TUI dependencies…` → EACCES on every launch.
"""
_touch_ink(tmp_path)
(tmp_path / "package-lock.json").write_text(
'{"packages":{'
'"node_modules/foo":{"version":"1.0.0","dev":true,"peer":true,"resolved":"https://x/foo.tgz"}'
'}}'
)
(tmp_path / "node_modules" / ".package-lock.json").write_text(
'{"packages":{'
'"node_modules/foo":{"version":"1.0.0","dev":true,"resolved":"https://x/foo.tgz"}'
'}}'
)
assert main_mod._tui_need_npm_install(tmp_path) is False
def test_install_when_version_differs_even_with_peer_drop(tmp_path: Path, main_mod) -> None:
"""The peer-drop tolerance must not mask a real version skew."""
_touch_ink(tmp_path)
(tmp_path / "package-lock.json").write_text(
'{"packages":{"node_modules/foo":{"version":"2.0.0","dev":true,"peer":true}}}'
)
(tmp_path / "node_modules" / ".package-lock.json").write_text(
'{"packages":{"node_modules/foo":{"version":"1.0.0","dev":true}}}'
)
assert main_mod._tui_need_npm_install(tmp_path) is True
def test_no_install_when_lock_older_than_marker(tmp_path: Path, main_mod) -> None:
_touch_ink(tmp_path)
(tmp_path / "package-lock.json").write_text("{}")