fix(dashboard): detect stale code after hermes update and refuse model picker with clear 503 (#86207)
This commit is contained in:
@@ -386,6 +386,17 @@ async def _lifespan(app: "FastAPI"):
|
||||
# Desktop's 10-second WebSocket ready-probe to time out (GH-73083).
|
||||
_warm_gateway_module()
|
||||
|
||||
# Snapshot the checkout revision at boot so risky lazy-import paths (the
|
||||
# model picker) can detect when `hermes update` replaced the code
|
||||
# underneath this long-lived process and refuse with a clear "restart
|
||||
# required" message instead of a stale-module ImportError (#86207). This
|
||||
# mirrors the gateway's record_boot_fingerprint in gateway/run.py; the
|
||||
# dashboard is a separate process/unit that the update flow does not
|
||||
# reliably restart, so it must detect the drift itself.
|
||||
from gateway.code_skew import record_boot_fingerprint
|
||||
|
||||
record_boot_fingerprint()
|
||||
|
||||
# Desktop-spawned backends (HERMES_DESKTOP=1) fire cron jobs themselves,
|
||||
# since the app has no gateway running the scheduler. Server `hermes
|
||||
# dashboard` is unaffected — it relies on its own gateway.
|
||||
@@ -7158,6 +7169,37 @@ _AUX_TASK_SLOTS: Tuple[str, ...] = (
|
||||
)
|
||||
|
||||
|
||||
def _dashboard_code_skew_guard() -> Optional[str]:
|
||||
"""Return a clear \"restart required\" message when the dashboard runs stale code.
|
||||
|
||||
The dashboard is a long-lived process; its ``sys.modules`` is frozen at
|
||||
boot. When ``hermes update`` (or a manual ``git pull``) replaces the
|
||||
checkout underneath it, a first-time lazy import on a new code path can
|
||||
resolve a freshly-pulled consumer module against a stale cached dependency
|
||||
-> ImportError — e.g. ``/api/model/options`` 500 after the update added
|
||||
``agent.model_metadata.is_grok_46_family`` while the running process kept
|
||||
serving the pre-update module (#86207). Mirror the gateway's
|
||||
``_model_switch_skew_guard``: refuse the risky call with an actionable
|
||||
message instead of crashing with a cryptic import error.
|
||||
|
||||
Returns None when no drift is detectable (fresh process, or a non-git
|
||||
install where the boot fingerprint could not be read — never a false
|
||||
positive).
|
||||
"""
|
||||
from gateway.code_skew import detect_code_skew
|
||||
|
||||
skew = detect_code_skew()
|
||||
if not skew:
|
||||
return None
|
||||
boot_rev, disk_rev = skew
|
||||
return (
|
||||
f"This dashboard is running code from {boot_rev} but the checkout on "
|
||||
f"disk is now {disk_rev}. The model picker would risk a stale-module "
|
||||
f"crash — restart the dashboard to load the new code "
|
||||
f"(systemctl --user restart hermes-dashboard, or hermes dashboard --port <port>)"
|
||||
)
|
||||
|
||||
|
||||
@app.get("/api/model/options")
|
||||
async def get_model_options(
|
||||
profile: Optional[str] = None,
|
||||
@@ -7181,6 +7223,13 @@ async def get_model_options(
|
||||
Models" control. Normal opens leave it false to stay on the 1h cache.
|
||||
"""
|
||||
try:
|
||||
skew_msg = _dashboard_code_skew_guard()
|
||||
if skew_msg:
|
||||
_log.warning("GET /api/model/options refused: %s", skew_msg)
|
||||
raise HTTPException(
|
||||
status_code=503, detail=f"Restart required: {skew_msg}"
|
||||
)
|
||||
|
||||
from hermes_cli.inventory import build_model_options_payload, load_picker_context
|
||||
|
||||
def _build_payload_scoped() -> dict:
|
||||
|
||||
+84
-1
@@ -2,9 +2,15 @@
|
||||
|
||||
Companion to ``tests/test_stale_utils_module_import.py``: that test proves the
|
||||
crash; these prove the guard that turns it into a clear "restart the gateway"
|
||||
message before a model switch can hit it.
|
||||
message before a model switch can hit it. The dashboard mirror (#86207) lives
|
||||
here too: ``web_server._dashboard_code_skew_guard`` and the
|
||||
``/api/model/options`` 503 guard, which protect the Models page from the same
|
||||
stale-module ImportError after ``hermes update``.
|
||||
"""
|
||||
|
||||
import asyncio
|
||||
import contextlib
|
||||
|
||||
import pytest
|
||||
|
||||
from gateway import code_skew
|
||||
@@ -61,3 +67,80 @@ class TestModelSwitchSkewGuard:
|
||||
assert "abc1234567" in msg
|
||||
assert "def4567890" in msg
|
||||
assert "hermes gateway restart" in msg
|
||||
|
||||
|
||||
class TestDashboardCodeSkewGuard:
|
||||
"""Dashboard mirror of the gateway's model-switch skew guard (#86207)."""
|
||||
|
||||
def test_dashboard_guard_returns_none_without_skew(self, monkeypatch):
|
||||
from hermes_cli import web_server
|
||||
|
||||
monkeypatch.setattr(code_skew, "detect_code_skew", lambda: None)
|
||||
assert web_server._dashboard_code_skew_guard() is None
|
||||
|
||||
def test_dashboard_guard_message_names_revs_and_restart(self, monkeypatch):
|
||||
from hermes_cli import web_server
|
||||
|
||||
monkeypatch.setattr(code_skew, "detect_code_skew", lambda: ("abc1234567", "def4567890"))
|
||||
msg = web_server._dashboard_code_skew_guard()
|
||||
assert msg is not None
|
||||
assert "abc1234567" in msg
|
||||
assert "def4567890" in msg
|
||||
assert "restart" in msg.lower()
|
||||
|
||||
|
||||
class TestModelOptionsSkewGuard:
|
||||
"""/api/model/options must refuse with a clear 503 when the dashboard is stale.
|
||||
|
||||
Regression for #86207: a dashboard kept alive across ``hermes update``
|
||||
serves stale modules, so the picker's lazy import of names the update added
|
||||
(``agent.model_metadata.is_grok_46_family``) raised ImportError and the
|
||||
handler collapsed it into a generic 500. With the guard, the stale process
|
||||
surfaces "Restart required" (503) and never reaches the payload build.
|
||||
"""
|
||||
|
||||
def test_stale_dashboard_returns_503_and_skips_payload_build(self, monkeypatch):
|
||||
from fastapi import HTTPException
|
||||
from hermes_cli import web_server
|
||||
|
||||
monkeypatch.setattr(code_skew, "detect_code_skew", lambda: ("abc1234567", "def4567890"))
|
||||
|
||||
payload_calls: list = []
|
||||
monkeypatch.setattr(
|
||||
"hermes_cli.inventory.build_model_options_payload",
|
||||
lambda *a, **k: payload_calls.append(1) or {"providers": []},
|
||||
)
|
||||
|
||||
with pytest.raises(HTTPException) as excinfo:
|
||||
asyncio.run(web_server.get_model_options())
|
||||
|
||||
assert excinfo.value.status_code == 503
|
||||
assert "restart" in str(excinfo.value.detail).lower()
|
||||
# The stale-import crash site (the payload build) is never reached.
|
||||
assert payload_calls == []
|
||||
|
||||
def test_fresh_dashboard_builds_payload_unchanged(self, monkeypatch):
|
||||
from hermes_cli import web_server
|
||||
|
||||
monkeypatch.setattr(code_skew, "detect_code_skew", lambda: None)
|
||||
|
||||
async def _fake_run_in_threadpool(func):
|
||||
return func()
|
||||
|
||||
monkeypatch.setattr(web_server, "run_in_threadpool", _fake_run_in_threadpool)
|
||||
monkeypatch.setattr(
|
||||
web_server, "_profile_scope", lambda profile: contextlib.nullcontext()
|
||||
)
|
||||
monkeypatch.setattr("hermes_cli.inventory.load_picker_context", lambda: {})
|
||||
|
||||
payload_calls: list = []
|
||||
expected = {"providers": [], "model": {}, "provider": None}
|
||||
monkeypatch.setattr(
|
||||
"hermes_cli.inventory.build_model_options_payload",
|
||||
lambda *a, **k: payload_calls.append(1) or expected,
|
||||
)
|
||||
|
||||
result = asyncio.run(web_server.get_model_options())
|
||||
|
||||
assert result == expected
|
||||
assert payload_calls == [1]
|
||||
|
||||
Reference in New Issue
Block a user