diff --git a/hermes_cli/main.py b/hermes_cli/main.py index 81c8de4aa1e6..4ff7ab0900c0 100644 --- a/hermes_cli/main.py +++ b/hermes_cli/main.py @@ -1822,6 +1822,12 @@ def _make_tui_argv(tui_dir: Path, tui_dev: bool) -> tuple[list[str], Path]: npm, "install", *npm_workspace_args, + # --include=dev: ui-tui's build toolchain (esbuild, typescript) + # lives in devDependencies. An inherited NODE_ENV=production + # (e.g. from a container shell or a parent TUI launch) or an + # npm `omit=dev` config would silently skip them and the TUI + # build would fail. See _run_npm_install_deterministic. + "--include=dev", "--silent", "--no-fund", "--no-audit", @@ -4832,6 +4838,25 @@ def _run_npm_install_deterministic( rewrites committed lockfiles (stripping ``"peer": true`` etc.), which leaves the working tree dirty and causes the next ``hermes update`` to stash the lockfile — repeatedly. + + ``--include=dev`` is forced on every invocation: the callers are frontend + builds (web UI / TUI / desktop workspaces), and those builds need the dev + toolchain (``tsc``, ``vite``, ``electron-builder`` — all + ``devDependencies``). If the caller's environment has + ``NODE_ENV=production`` (or npm config ``omit=dev``) — which leaks in from + a shell profile, a container image, or the bundled TUI launcher that sets + ``NODE_ENV=production`` on its subprocess env — npm silently omits + devDependencies (exit 0, no error), so the build toolchain never installs + and the subsequent build dies with ``tsc: command not found`` (exit 127). + The flag overrides both the env var and npm config, unlike scrubbing + ``NODE_ENV`` from the environment which only fixes the env-leak case. + + ``--no-save`` on the ``npm install`` fallback keeps it true to this + function's contract: never mutate ``package-lock.json``. Without it, an + out-of-sync lockfile gets rewritten by the fallback, which drifts the + committed lockfile and makes every future ``npm ci`` fail — a + self-reinforcing cycle where web devDeps never install and a stale dist + is served on every update (PR #65595). """ # unicode-animations' postinstall animates to /dev/tty (bypasses # --silent/capture_output). It no-ops when CI is set — same as the TUI @@ -4840,7 +4865,7 @@ def _run_npm_install_deterministic( lockfile = cwd / "package-lock.json" if lockfile.exists(): - ci_cmd = [npm, "ci", *extra_args] + ci_cmd = [npm, "ci", "--include=dev", *extra_args] ci_result = subprocess.run( ci_cmd, cwd=cwd, @@ -4855,7 +4880,7 @@ def _run_npm_install_deterministic( return ci_result # Fall through to `npm install` — lockfile may be out of sync on a # WIP fork/branch, or `npm ci` may not be available on very old npm. - install_cmd = [npm, "install", *extra_args] + install_cmd = [npm, "install", "--no-save", "--include=dev", *extra_args] return subprocess.run( install_cmd, cwd=cwd, diff --git a/tests/hermes_cli/test_cmd_update.py b/tests/hermes_cli/test_cmd_update.py index dd3d2130a360..b8b0244531a0 100644 --- a/tests/hermes_cli/test_cmd_update.py +++ b/tests/hermes_cli/test_cmd_update.py @@ -483,6 +483,7 @@ class TestCmdUpdateBranchFallback: root_flags = [ "/usr/bin/npm", "ci", + "--include=dev", "--no-fund", "--no-audit", "--progress=false", @@ -491,6 +492,7 @@ class TestCmdUpdateBranchFallback: ws_flags = [ "/usr/bin/npm", "ci", + "--include=dev", "--no-fund", "--no-audit", "--progress=false", @@ -507,7 +509,7 @@ class TestCmdUpdateBranchFallback: # The web/ install runs from the workspace root when the root # lockfile exists (npm workspaces hoist node_modules upward). assert npm_calls[2:] == [ - (["/usr/bin/npm", "ci", "--workspace", "web", "--silent"], PROJECT_ROOT), + (["/usr/bin/npm", "ci", "--include=dev", "--workspace", "web", "--silent"], PROJECT_ROOT), ] # The web UI build itself went through the streaming helper. diff --git a/tests/hermes_cli/test_tui_npm_install.py b/tests/hermes_cli/test_tui_npm_install.py index 109fe6411209..e4fd04c551d9 100644 --- a/tests/hermes_cli/test_tui_npm_install.py +++ b/tests/hermes_cli/test_tui_npm_install.py @@ -265,6 +265,7 @@ def test_make_tui_argv_keeps_desktop_workspace_install_behaviour( "install", "--workspace", "ui-tui", + "--include=dev", "--silent", "--no-fund", "--no-audit", @@ -275,6 +276,39 @@ def test_make_tui_argv_keeps_desktop_workspace_install_behaviour( _assert_utf8_replace_capture(calls[1][1]) +def test_make_tui_argv_npm_install_forces_include_dev( + tmp_path: Path, main_mod, monkeypatch +) -> None: + """The TUI-launch npm install must force --include=dev: ui-tui's build + toolchain (esbuild, typescript) lives in devDependencies, and an inherited + NODE_ENV=production (container shells; a parent TUI sets it on its own + subprocess env) or an npm `omit=dev` config would silently skip them, + breaking the TUI build with `tsc`/`esbuild: command not found.""" + tui_dir = tmp_path / "ui-tui" + tui_dir.mkdir() + (tui_dir / "package.json").write_text("{}") + (tmp_path / "package-lock.json").write_text("{}") + + monkeypatch.delenv("TERMUX_VERSION", raising=False) + monkeypatch.setenv("PREFIX", "/usr") + monkeypatch.setenv("NODE_ENV", "production") + monkeypatch.setattr(main_mod, "_tui_need_npm_install", lambda _root: True) + monkeypatch.setattr(main_mod.shutil, "which", lambda name: f"/bin/{name}") + calls = [] + + def fake_run(*args, **kwargs): + calls.append((args, kwargs)) + return types.SimpleNamespace(returncode=0, stdout="", stderr="") + + monkeypatch.setattr(main_mod.subprocess, "run", fake_run) + + main_mod._make_tui_argv(tui_dir, tui_dev=False) + + install_cmd = calls[0][0][0] + assert install_cmd[:2] == ["/bin/npm", "install"] + assert "--include=dev" in install_cmd + + def test_make_tui_argv_keeps_desktop_always_build_behaviour( tmp_path: Path, main_mod, monkeypatch ) -> None: diff --git a/tests/hermes_cli/test_web_ui_build.py b/tests/hermes_cli/test_web_ui_build.py index 8f94458a6ace..0dc513113934 100644 --- a/tests/hermes_cli/test_web_ui_build.py +++ b/tests/hermes_cli/test_web_ui_build.py @@ -156,6 +156,47 @@ class TestBuildWebUISkipsWhenFresh: assert kwargs["env"]["CI"] == "1" assert kwargs["env"]["PYTHON"] == "/nix/store/python" + def test_npm_ci_forces_include_dev(self, tmp_path): + """`npm ci` must pass --include=dev so an inherited NODE_ENV=production + (e.g. from a container shell, or the bundled TUI launcher which sets + NODE_ENV=production on its subprocess env) or an npm `omit=dev` config + can't silently strip the build toolchain (tsc/vite/electron-builder), + which otherwise fails the web/desktop build with `tsc: command not + found` (exit 127) despite the install exiting 0.""" + web_dir, _ = _make_web_dir(tmp_path) + (web_dir / "package-lock.json").write_text("{}", encoding="utf-8") + + mock_cp = __import__("subprocess").CompletedProcess([], 0, stdout="", stderr="") + with patch("hermes_cli.main.subprocess.run", return_value=mock_cp) as mock_run: + _run_npm_install_deterministic("/usr/bin/npm", web_dir) + + args, _ = mock_run.call_args + cmd = args[0] + assert cmd[:2] == ["/usr/bin/npm", "ci"] + assert "--include=dev" in cmd + + def test_npm_install_fallback_forces_include_dev_and_no_save(self, tmp_path): + """When `npm ci` fails (lockfile out of sync) the `npm install` + fallback must still force --include=dev (same NODE_ENV rationale as + above) and must pass --no-save so the fallback never rewrites the + committed lockfile — a drifted lockfile makes every future `npm ci` + fail, a self-reinforcing cycle that keeps devDeps from installing.""" + web_dir, _ = _make_web_dir(tmp_path) + (web_dir / "package-lock.json").write_text("{}", encoding="utf-8") + + ci_fail = __import__("subprocess").CompletedProcess([], 1, stdout="", stderr="lockfile out of sync") + install_ok = __import__("subprocess").CompletedProcess([], 0, stdout="", stderr="") + with patch("hermes_cli.main.subprocess.run", side_effect=[ci_fail, install_ok]) as mock_run: + result = _run_npm_install_deterministic("/usr/bin/npm", web_dir) + + assert result.returncode == 0 + assert mock_run.call_count == 2 + install_args, _ = mock_run.call_args_list[1] + install_cmd = install_args[0] + assert install_cmd[:2] == ["/usr/bin/npm", "install"] + assert "--include=dev" in install_cmd + assert "--no-save" in install_cmd + def test_npm_install_uses_workspace_web_scope(self, tmp_path): web_dir, _ = _make_web_dir(tmp_path) # Real workspace checkout: the single lockfile lives at the root, so @@ -201,7 +242,7 @@ class TestBuildWebUISkipsWhenFresh: assert result is True args, kwargs = mock_run.call_args assert "--workspace" not in args[0] - assert args[0] == ["/usr/bin/npm", "ci", "--silent"] + assert args[0] == ["/usr/bin/npm", "ci", "--include=dev", "--silent"] assert kwargs["cwd"] == web_dir def test_web_build_uses_idle_timeout_helper(self, tmp_path): @@ -245,6 +286,7 @@ class TestBuildWebUISkipsWhenFresh: assert args[0] == [ "/usr/bin/npm", "ci", + "--include=dev", "--workspace", "web", "--include-workspace-root=false", @@ -269,7 +311,7 @@ class TestBuildWebUISkipsWhenFresh: assert result is True args, kwargs = mock_run.call_args - assert args[0] == ["/usr/bin/npm", "ci", "--workspace", "web", "--silent"] + assert args[0] == ["/usr/bin/npm", "ci", "--include=dev", "--workspace", "web", "--silent"] assert kwargs["cwd"] == tmp_path