From 283381b1ce9dd8d69aaba88639d50d1f825c2121 Mon Sep 17 00:00:00 2001 From: Teknium1 <127238744+teknium1@users.noreply.github.com> Date: Mon, 11 May 2026 08:20:23 -0700 Subject: [PATCH] fix(dashboard): validate dist exists when --skip-build is set MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Follow-up to PR #23824. Adds two correctness fixes on top of the contributor's salvaged commit: 1. Stale-dist fallback no longer gated on `fatal=False`. `cmd_dashboard` passes `fatal=True` and is the primary scenario this fallback is for (issue #23817 — Windows Scheduled Task at logon). The previous gate meant the fallback never fired in the case it was designed for. 2. `--skip-build` now verifies the dist actually exists before starting the server. Without this, a misconfigured pre-build would launch the dashboard pointing at a missing dist and silently serve 404s. We now exit 1 with a clear "pre-build first: cd web && npm run build" message, and on success print which dist directory is being used. Verified end-to-end on Linux: - build fails + stale dist (fatal=True) -> fallback fires - build fails + no dist (fatal=True) -> exit 1 with stderr surfaced - build fails + stale dist (fatal=False) -> fallback fires - --skip-build + missing dist -> exit 1 with clear guidance - --skip-build + valid dist -> 'Skipping web UI build...' --- hermes_cli/main.py | 15 +++++++ tests/hermes_cli/test_web_ui_build.py | 61 +++++++++++++++++++++++++++ 2 files changed, 76 insertions(+) diff --git a/hermes_cli/main.py b/hermes_cli/main.py index 1afd4089bda..2bf679b14ae 100644 --- a/hermes_cli/main.py +++ b/hermes_cli/main.py @@ -9120,6 +9120,21 @@ def cmd_dashboard(args): if "HERMES_WEB_DIST" not in os.environ and not getattr(args, "skip_build", False): if not _build_web_ui(PROJECT_ROOT / "web", fatal=True): sys.exit(1) + elif getattr(args, "skip_build", False): + # --skip-build trusts the caller to have pre-built the web UI. + # Verify the dist actually exists; otherwise the server will start + # and serve 404s with no obvious cause (issue #23817). + _dist_root = ( + Path(os.environ["HERMES_WEB_DIST"]) + if "HERMES_WEB_DIST" in os.environ + else PROJECT_ROOT / "hermes_cli" / "web_dist" + ) + if not (_dist_root / "index.html").exists(): + print(f"✗ --skip-build was passed but no web dist found at: {_dist_root}") + print(" Pre-build first: cd web && npm install && npm run build") + print(" Or drop --skip-build to build automatically.") + sys.exit(1) + print(f"→ Skipping web UI build (--skip-build); using dist at {_dist_root}") from hermes_cli.web_server import start_server diff --git a/tests/hermes_cli/test_web_ui_build.py b/tests/hermes_cli/test_web_ui_build.py index 688cf7bdc29..6400075b861 100644 --- a/tests/hermes_cli/test_web_ui_build.py +++ b/tests/hermes_cli/test_web_ui_build.py @@ -147,3 +147,64 @@ class TestBuildWebUISkipsWhenFresh: assert build_kwargs["text"] is True assert build_kwargs["encoding"] == "utf-8" assert build_kwargs["errors"] == "replace" + + +class TestBuildWebUIRetryAndStaleFallback: + """Coverage for the retry + stale-dist fallback added in #23824 / issue #23817.""" + + def test_retries_build_once_on_failure(self, tmp_path): + web_dir, _ = _make_web_dir(tmp_path) + Subprocess = __import__("subprocess") + # install: success; build attempt 1: fail; build attempt 2: success + install_ok = Subprocess.CompletedProcess([], 0, stdout="", stderr="") + build_fail = Subprocess.CompletedProcess([], 1, stdout="", stderr="EPERM") + build_ok = Subprocess.CompletedProcess([], 0, stdout="", stderr="") + with patch("hermes_cli.main.shutil.which", return_value="/usr/bin/npm"), \ + patch("hermes_cli.main._time.sleep") as mock_sleep, \ + patch("hermes_cli.main.subprocess.run", + side_effect=[install_ok, build_fail, build_ok]) as mock_run: + result = _build_web_ui(web_dir) + + assert result is True + assert mock_run.call_count == 3 # install + build + retry + mock_sleep.assert_called_once_with(3) + + def test_falls_back_to_stale_dist_when_retry_also_fails(self, tmp_path, capsys): + web_dir, dist_dir = _make_web_dir(tmp_path) + # Stale dist exists but is older than source + _touch(dist_dir / "index.html", offset=-100) + _touch(web_dir / "src" / "App.tsx") # newer source -> build_needed=True + + Subprocess = __import__("subprocess") + install_ok = Subprocess.CompletedProcess([], 0, stdout="", stderr="") + build_fail = Subprocess.CompletedProcess([], 1, stdout="", stderr="vite ENOMEM") + with patch("hermes_cli.main.shutil.which", return_value="/usr/bin/npm"), \ + patch("hermes_cli.main._time.sleep"), \ + patch("hermes_cli.main.subprocess.run", + side_effect=[install_ok, build_fail, build_fail]): + result = _build_web_ui(web_dir, fatal=True) + + # MUST return True (serve stale) — issue #23817 — even with fatal=True, + # because cmd_dashboard passes fatal=True and is the primary caller. + assert result is True + out = capsys.readouterr().out + assert "serving stale dist as fallback" in out + assert "vite ENOMEM" in out # stderr surfaced to user + + def test_hard_fails_when_no_dist_to_fall_back_to(self, tmp_path, capsys): + web_dir, _ = _make_web_dir(tmp_path) + + Subprocess = __import__("subprocess") + install_ok = Subprocess.CompletedProcess([], 0, stdout="", stderr="") + build_fail = Subprocess.CompletedProcess([], 1, stdout="", stderr="vite ENOMEM") + with patch("hermes_cli.main.shutil.which", return_value="/usr/bin/npm"), \ + patch("hermes_cli.main._time.sleep"), \ + patch("hermes_cli.main.subprocess.run", + side_effect=[install_ok, build_fail, build_fail]): + result = _build_web_ui(web_dir, fatal=True) + + assert result is False + out = capsys.readouterr().out + assert "Web UI build failed" in out + assert "vite ENOMEM" in out + assert "Run manually" in out