fix(update): stream + idle-kill npm run build so a stalled webui-build can't soft-brick the install (#33803)
`hermes update` ran the webui build with `capture_output=True` and no timeout. On low-memory hosts (WSL2's 4 GB default, small VPSes, antivirus stalls) Vite goes silent for minutes; users see a frozen terminal, decide the update is hung, and reboot. The reboot lands *after* `pip install -e .` has already touched the install but *before* the build completes, leaving the `hermes` launcher in place while `hermes_cli` is no longer importable — i.e. `ModuleNotFoundError: No module named 'hermes_cli'` (#33788, same class as #32384). Changes: - New `_run_with_idle_timeout()` helper: streams subprocess output line-by-line (so the user sees Vite progress in real time) and kills the process if no bytes appear on stdout/stderr for 180s. The existing stale-dist fallback (#23817) then serves the previous build instead of failing the update. - `_build_web_ui()` uses the helper for `npm run build` (the actual stall site). `npm install` keeps `subprocess.run` + capture_output to preserve the existing EPERM-retry-on-Windows contract. - Both `cmd_update` call sites print `→ Core update complete. Building dashboard (optional)...` before the webui build. The CLI is fully functional at this point; a webui-build failure only affects `hermes dashboard`. Telegraphing the boundary explicitly stops users from rebooting through the build step. Tests: - `tests/hermes_cli/test_run_with_idle_timeout.py` — 4 tests covering streaming success, nonzero exit, idle-kill, and missing-binary cases. Uses real `subprocess.Popen` on tiny Python scripts; isolated in its own file so per-file canonical-runner parallelism doesn't pair it with the mock-heavy tests. - `tests/hermes_cli/test_web_ui_build.py` — updated existing tests to patch `_run_with_idle_timeout` for the build step in addition to `subprocess.run` for the install step. - `tests/hermes_cli/test_cmd_update.py::test_update_refreshes_repo_and_tui_node_dependencies` — same update. Full suite: `scripts/run_tests.sh tests/hermes_cli/` → 5646 passed, 0 failed. Fixes #33788.
This commit is contained in:
@@ -144,7 +144,13 @@ class TestCmdUpdateBranchFallback:
|
||||
mock_run.side_effect = _make_run_side_effect(
|
||||
branch="main", verify_ok=True, commit_count="1"
|
||||
)
|
||||
with patch.object(hm, "_is_termux_env", return_value=False):
|
||||
# The web UI build runs through _run_with_idle_timeout now (issue
|
||||
# #33788) so it no longer appears in subprocess.run's call list.
|
||||
# Mock it so the test doesn't actually shell out to ``tsc``.
|
||||
import subprocess as _subprocess
|
||||
build_ok = _subprocess.CompletedProcess([], 0, stdout="", stderr="")
|
||||
with patch.object(hm, "_is_termux_env", return_value=False), \
|
||||
patch.object(hm, "_run_with_idle_timeout", return_value=build_ok) as mock_idle:
|
||||
cmd_update(mock_args)
|
||||
|
||||
npm_calls = [
|
||||
@@ -153,10 +159,11 @@ class TestCmdUpdateBranchFallback:
|
||||
if call.args and call.args[0][0] == "/usr/bin/npm"
|
||||
]
|
||||
|
||||
# cmd_update runs npm commands in three locations:
|
||||
# 1. repo root — slash-command / TUI bridge deps
|
||||
# 2. ui-tui/ — Ink TUI deps
|
||||
# 3. web/ — install + "npm run build" for the web frontend
|
||||
# cmd_update runs npm commands in four locations:
|
||||
# 1. repo root — slash-command / TUI bridge deps (subprocess.run)
|
||||
# 2. ui-tui/ — Ink TUI deps (subprocess.run)
|
||||
# 3. web/ — npm install (subprocess.run)
|
||||
# 4. web/ — npm run build (_run_with_idle_timeout)
|
||||
#
|
||||
# Repo-root and ui-tui installs intentionally omit `--silent` and run
|
||||
# without `capture_output` so optional postinstall scripts (e.g.
|
||||
@@ -175,11 +182,18 @@ class TestCmdUpdateBranchFallback:
|
||||
(update_flags, PROJECT_ROOT / "ui-tui"),
|
||||
]
|
||||
if len(npm_calls) > 2:
|
||||
# Only the web/ install is left in subprocess.run; the build moved
|
||||
# to _run_with_idle_timeout to make Vite progress visible (#33788).
|
||||
assert npm_calls[2:] == [
|
||||
(["/usr/bin/npm", "ci", "--silent"], PROJECT_ROOT / "web"),
|
||||
(["/usr/bin/npm", "run", "build"], PROJECT_ROOT / "web"),
|
||||
]
|
||||
|
||||
# The web UI build itself went through the streaming helper.
|
||||
mock_idle.assert_called_once()
|
||||
idle_args, idle_kwargs = mock_idle.call_args
|
||||
assert idle_args[0] == ["/usr/bin/npm", "run", "build"]
|
||||
assert idle_kwargs["cwd"] == PROJECT_ROOT / "web"
|
||||
|
||||
# Regression for #18840: repo root + ui-tui installs must stream
|
||||
# output (capture_output=False) so postinstall progress is visible
|
||||
# to the user.
|
||||
|
||||
Reference in New Issue
Block a user