diff --git a/hermes_cli/web_routers/actions.py b/hermes_cli/web_routers/actions.py index 8ce2ecaa7d..01b28805a5 100644 --- a/hermes_cli/web_routers/actions.py +++ b/hermes_cli/web_routers/actions.py @@ -1,7 +1,7 @@ """Gateway restart/drain, Hermes update and background-action status dashboard routes. Extracted from ``hermes_cli.web_server``; helpers/state that tests monkeypatch on -``web_server`` stay there and are imported lazily at call time (cycle-safe). +``web_server`` stay there and are late-bound (cycle-safe). """ import logging @@ -10,7 +10,7 @@ import secrets import subprocess from fastapi import APIRouter from hermes_cli.web_routers._common import http_failure -from hermes_cli.web_deps import late +from hermes_cli.web_deps import LateState, late from fastapi import HTTPException, Request from hermes_cli import __version__ from hermes_cli.config import format_docker_update_message, recommended_update_command_for_method @@ -23,41 +23,59 @@ _log = logging.getLogger("hermes_cli.web_server") router = APIRouter() status_router = APIRouter() -# web_server helpers, late-bound so monkeypatch.setattr(web_server, ...) stays authoritative. +# web_server helpers/state, late-bound so monkeypatch.setattr(web_server, ...) stays authoritative. _dashboard_local_update_managed_externally = late("_dashboard_local_update_managed_externally") _spawn_gateway_restart = late("_spawn_gateway_restart") _spawn_hermes_action = late("_spawn_hermes_action") detect_install_method = late("detect_install_method") get_hermes_home = late("get_hermes_home") +_ACTION_COMMANDS = LateState("_ACTION_COMMANDS") +_ACTION_IDS = LateState("_ACTION_IDS") +_ACTION_LOG_FILES = LateState("_ACTION_LOG_FILES") +_ACTION_PROCS = LateState("_ACTION_PROCS") +_ACTION_RESULTS = LateState("_ACTION_RESULTS") + + +def _action_log_dir() -> Path: + """Live ``web_server._ACTION_LOG_DIR`` (a Path value, so not proxied by LateState).""" + from hermes_cli.web_server import _ACTION_LOG_DIR + + return _ACTION_LOG_DIR + + +def _project_root() -> Path: + from hermes_cli.web_server import PROJECT_ROOT + + return PROJECT_ROOT _ACTION_LOG_TAIL_MAX_BYTES = 256 * 1024 - - _ACTION_LOG_TAIL_INITIAL_CHUNK_BYTES = 8 * 1024 - - _ACTION_LOG_TAIL_MAX_CHUNK_BYTES = 64 * 1024 - _UPDATE_ACTION_COMPLETED_RE = re.compile( r"^=== hermes-update completed ([0-9a-f]{32}) ===$" ) +_MANAGED_EXTERNALLY_MESSAGE = ( + "Hermes updates are managed outside this dashboard in " + "containerized environments." +) + + +def _finish_action(name: str, exit_code: Optional[int], pid: Optional[int]) -> None: + """Record a terminal result and drop the live-process registries for ``name``.""" + _ACTION_RESULTS[name] = {"exit_code": exit_code, "pid": pid} + _ACTION_PROCS.pop(name, None) + _ACTION_COMMANDS.pop(name, None) + _ACTION_IDS.pop(name, None) + def _record_completed_action(name: str, message: str, exit_code: int = 1) -> None: """Record a non-spawned action result and write it to the action log.""" - from hermes_cli.web_server import ( - _ACTION_COMMANDS, - _ACTION_IDS, - _ACTION_LOG_DIR, - _ACTION_LOG_FILES, - _ACTION_PROCS, - _ACTION_RESULTS, - ) - log_file_name = _ACTION_LOG_FILES[name] - _ACTION_LOG_DIR.mkdir(parents=True, exist_ok=True) - log_path = _ACTION_LOG_DIR / log_file_name + log_dir = _action_log_dir() + log_dir.mkdir(parents=True, exist_ok=True) + log_path = log_dir / _ACTION_LOG_FILES[name] with open(log_path, "ab", buffering=0) as log_file: log_file.write( f"\n=== {name} completed {time.strftime('%Y-%m-%d %H:%M:%S')} ===\n".encode() @@ -65,10 +83,7 @@ def _record_completed_action(name: str, message: str, exit_code: int = 1) -> Non log_file.write(message.encode("utf-8", errors="replace")) if not message.endswith("\n"): log_file.write(b"\n") - _ACTION_PROCS.pop(name, None) - _ACTION_COMMANDS.pop(name, None) - _ACTION_IDS.pop(name, None) - _ACTION_RESULTS[name] = {"exit_code": exit_code, "pid": None} + _finish_action(name, exit_code, None) def _tail_lines(path: Path, n: int) -> List[str]: @@ -98,21 +113,14 @@ def _tail_lines(path: Path, n: int) -> List[str]: chunk = handle.read(read_size) chunks.append(chunk) newline_count += chunk.count(b"\n") - chunk_size = min( - chunk_size * 2, - _ACTION_LOG_TAIL_MAX_CHUNK_BYTES, - ) + chunk_size = min(chunk_size * 2, _ACTION_LOG_TAIL_MAX_CHUNK_BYTES) if offset > 0: handle.seek(offset - 1) drop_partial_first_line = handle.read(1) != b"\n" except OSError: return [] - lines = ( - b"".join(reversed(chunks)) - .decode("utf-8", errors="replace") - .splitlines() - ) + lines = b"".join(reversed(chunks)).decode("utf-8", errors="replace").splitlines() if drop_partial_first_line and lines: lines = lines[1:] return lines[-n:] @@ -121,11 +129,10 @@ def _tail_lines(path: Path, n: int) -> List[str]: def _durable_completed_update_action_id(lines: List[str]) -> Optional[str]: """Recover the latest successful update identity from ``update.log``. - The dashboard action process can restart the dashboard that spawned it. - That loses the in-memory ``Popen``/result registries while the durable - update log survives. Only accept a completion marker that occurs after - the latest update-start marker, so a stale success cannot mask a newer - failed attempt. + The dashboard action process can restart the dashboard that spawned it, + losing the in-memory ``Popen``/result registries while the durable log + survives. Only a completion marker after the latest update-start marker + counts, so a stale success cannot mask a newer failed attempt. """ last_start = -1 last_completed = -1 @@ -163,25 +170,18 @@ async def gateway_drain(request: Request): """Begin or cancel an external (NAS-driven) gateway drain. Authenticated by the non-interactive token-auth seam: the - ``dashboard_auth/drain`` plugin registers this exact path as a token route - and verifies the ``Authorization`` bearer secret. If that plugin isn't - active (no ``HERMES_DASHBOARD_DRAIN_SECRET``), the route is NOT a token - route, so on a gated bind the cookie gate handles it (a browser session can - still drive it from the dashboard) and on a loopback bind the legacy - session-token gate applies — either way it is never unauthenticated on a + ``dashboard_auth/drain`` plugin registers this path as a token route and + verifies the bearer secret. Without that plugin (no + ``HERMES_DASHBOARD_DRAIN_SECRET``) the cookie gate covers a gated bind and + the legacy session-token gate a loopback bind — never unauthenticated on a network-exposed bind. - Body: ``{"action": "drain"}`` (begin) or ``{"action": "cancel"}`` (cancel). - Begin writes the ``.drain_request.json`` marker the gateway's - ``_drain_control_watcher`` observes (flip to ``draining`` + refuse new - turns); cancel removes it (revert to ``running`` + re-accept). Idempotent - on both sides. This endpoint only writes/removes the marker — the gateway - process owns the actual state transition (there is no HTTP control channel - into the running gateway; the marker IS the channel, decisions.md Q-B). - - The force-override (D6: "unless a user commands it") is NOT here — an - immediate, drain-skipping action maps onto the existing - ``POST /api/gateway/restart`` force path, which supersedes a drain. + Body: ``{"action": "drain"}`` or ``{"action": "cancel"}``. Begin writes the + ``.drain_request.json`` marker the gateway's ``_drain_control_watcher`` + observes (flip to ``draining`` + refuse new turns); cancel removes it. + Idempotent on both sides. Only the marker is written here — the gateway + process owns the state transition (the marker IS the control channel). The + force-override is ``POST /api/gateway/restart``, which supersedes a drain. """ from gateway.drain_control import ( clear_drain_request, @@ -231,54 +231,48 @@ async def gateway_drain(request: Request): } +def _update_refused(error: str, message: str, update_command: str) -> Dict[str, Any]: + return { + "ok": False, + "pid": None, + "name": "hermes-update", + "error": error, + "message": message, + "update_command": update_command, + } + + +# Per-kind dashboard error codes the UI keys on, by admission-refusal code. +_UPDATE_REFUSAL_ERROR_CODES = { + "docker": "docker_update_unsupported", + "image-marker": "docker_update_unsupported", + "image-marker-invalid": "docker_update_unsupported", + "apt": "apt_update_required", + "nix": "nix_update_unsupported", +} + + @router.post("/api/hermes/update") async def update_hermes(): """Kick off ``hermes update`` in the background.""" - from hermes_cli.web_server import PROJECT_ROOT, _ACTION_IDS, _ACTION_PROCS if _dashboard_local_update_managed_externally(): - message = ( - "Hermes updates are managed outside this dashboard in " - "containerized environments. The built-in local updater is " - "disabled here." - ) + message = _MANAGED_EXTERNALLY_MESSAGE + " The built-in local updater is disabled here." _record_completed_action("hermes-update", message, exit_code=1) - return { - "ok": False, - "pid": None, - "name": "hermes-update", - "error": "dashboard_update_managed_externally", - "message": message, - "update_command": "managed outside dashboard", - } + return _update_refused("dashboard_update_managed_externally", message, "managed outside dashboard") - # Shared admission gate (#91277 Phase 3): marker-first, then the - # docker/nix/apt heuristics — one decision with the CLI paths. The - # response keeps the pre-existing per-kind error codes the dashboard UI - # already keys on. + # Shared admission gate: marker-first, then the docker/nix/apt heuristics — + # one decision with the CLI paths. from hermes_cli.update_contract import ( evaluate_update_admission, record_refusal_receipt, ) - refusal = evaluate_update_admission(PROJECT_ROOT) + refusal = evaluate_update_admission(_project_root()) if refusal is not None: _record_completed_action("hermes-update", refusal.message, exit_code=1) record_refusal_receipt(refusal) - error_code = { - "docker": "docker_update_unsupported", - "image-marker": "docker_update_unsupported", - "image-marker-invalid": "docker_update_unsupported", - "apt": "apt_update_required", - "nix": "nix_update_unsupported", - }.get(refusal.code, "update_not_in_place") - return { - "ok": False, - "pid": None, - "name": "hermes-update", - "error": error_code, - "message": refusal.message, - "update_command": refusal.update_command, - } + error_code = _UPDATE_REFUSAL_ERROR_CODES.get(refusal.code, "update_not_in_place") + return _update_refused(error_code, refusal.message, refusal.update_command) existing = _ACTION_PROCS.get("hermes-update") if existing is not None and existing.poll() is None: @@ -315,21 +309,17 @@ def _recent_upstream_commits(n: int = 20) -> List[Dict[str, Any]]: """Commits the local checkout is behind ``origin/main`` by, newest first. Logs the SAME range the behind-count uses (``HEAD..origin/main`` — see - ``banner._check_via_local_git``), NOT the branch's ``@{upstream}``. On a - feature-branch checkout ``@{upstream}`` is the branch's own tip (zero - commits), which would leave the changelog empty even though the count is - non-zero. Pinning to ``origin/main`` keeps count and changelog consistent. - - Best-effort: returns [] if not a git checkout, origin/main is unreachable, - or git is unavailable. Never raises into the request path. + ``banner._check_via_local_git``), NOT ``@{upstream}``: on a feature-branch + checkout that is the branch's own tip (zero commits), leaving the changelog + empty while the count is non-zero. Best-effort: [] if not a git checkout, + origin/main unreachable, or git unavailable. Never raises. """ - from hermes_cli.web_server import PROJECT_ROOT try: out = subprocess.run( [ "git", "-C", - str(PROJECT_ROOT), + str(_project_root()), "log", "--format=%H%x1f%s%x1f%an%x1f%ct", "HEAD..origin/main", @@ -337,10 +327,9 @@ def _recent_upstream_commits(n: int = 20) -> List[Dict[str, Any]]: ], capture_output=True, text=True, - # git log emits UTF-8 (commit subjects can carry emoji/CJK). On - # Windows text=True defaults to the ANSI code page — a byte like - # 0x90 (3rd byte of 🐛) is undefined in cp1252 and crashed the - # stdlib _readerthread, killing the desktop backend (#52649). + # git log emits UTF-8 (emoji/CJK subjects). On Windows text=True + # defaults to the ANSI code page; an undefined cp1252 byte crashed + # the stdlib _readerthread and killed the desktop backend. encoding="utf-8", errors="replace", timeout=5, @@ -351,16 +340,8 @@ def _recent_upstream_commits(n: int = 20) -> List[Dict[str, Any]]: for line in out.stdout.splitlines(): if not line.strip(): continue - parts = (line.split("\x1f") + ["", "", "", "0"])[:4] - sha, summary, author, at = parts - rows.append( - { - "sha": sha[:7], - "summary": summary, - "author": author, - "at": int(at or 0), - } - ) + sha, summary, author, at = (line.split("\x1f") + ["", "", "", "0"])[:4] + rows.append({"sha": sha[:7], "summary": summary, "author": author, "at": int(at or 0)}) return rows except Exception: return [] @@ -370,29 +351,22 @@ def _recent_upstream_commits(n: int = 20) -> List[Dict[str, Any]]: async def check_hermes_update(force: bool = False): """Report whether a Hermes update is available, without applying it. - Powers the dashboard's "check before you update" flow: the System page - shows the commit-behind count and asks the user to confirm before - ``POST /api/hermes/update`` actually runs ``hermes update``. + Powers the dashboard's "check before you update" flow. Returns: install_method: 'apt' | 'git' | 'docker' | 'nix' | 'nixos' | 'unknown' current_version: installed Hermes version string - behind: commits behind upstream (>=1), 0 if up to date, - -1 if behind by an unknown count, or null if the - check could not run (offline, no remote, etc.) + behind: commits behind upstream (>=1), 0 if up to date, -1 if behind by + an unknown count, or null if the check could not run update_available: convenience bool (behind is non-zero and not null) - can_apply: True when the dashboard's update button can apply it - in place (git); False for other install methods where the - user must update out-of-band + can_apply: True when the dashboard's update button can apply it in + place (git); False where the user must update out-of-band update_command: the recommended command for this install method message: human-readable guidance for non-applyable methods - commits: for git installs that are behind, a list of the commits - the local checkout is behind upstream by — each - {sha, summary, author, at}. Absent/empty otherwise. The - desktop's remote update overlay renders this as "what's - changed". Additive: existing consumers ignore it. + commits: for git installs that are behind, the commits the checkout is + behind by — each {sha, summary, author, at}. Absent/empty + otherwise; additive, existing consumers ignore it. """ - from hermes_cli.web_server import PROJECT_ROOT if _dashboard_local_update_managed_externally(): return { "install_method": "managed-runtime", @@ -401,13 +375,10 @@ async def check_hermes_update(force: bool = False): "update_available": False, "can_apply": False, "update_command": "managed outside dashboard", - "message": ( - "Hermes updates are managed outside this dashboard in " - "containerized environments." - ), + "message": _MANAGED_EXTERNALLY_MESSAGE, } - install_method = detect_install_method(PROJECT_ROOT) + install_method = detect_install_method(_project_root()) update_command = recommended_update_command_for_method(install_method) payload: Dict[str, Any] = { @@ -429,9 +400,8 @@ async def check_hermes_update(force: bool = False): ) return payload - # banner.check_for_updates() handles git / nix-revision paths and - # caches the result for 6h. ``force`` busts the cache so the "Check now" - # button reflects reality immediately. + # banner.check_for_updates() handles git / nix-revision paths and caches + # the result for 6h. ``force`` busts the cache so "Check now" reflects reality. try: from hermes_cli.banner import check_for_updates @@ -453,8 +423,7 @@ async def check_hermes_update(force: bool = False): payload["message"] = "You're on the latest version." else: payload["update_available"] = True - # Enrich with the actual commits we're behind by, so the desktop's - # remote update overlay can show "what's changed". git only; + # "What's changed" for the desktop's remote update overlay; git only, # best-effort (empty list on any failure). if install_method == "git": payload["commits"] = await asyncio.to_thread(_recent_upstream_commits) @@ -465,38 +434,27 @@ async def check_hermes_update(force: bool = False): @status_router.get("/api/actions/{name}/status") async def get_action_status(name: str, lines: int = 200): """Tail an action log and report whether the process is still running.""" - from hermes_cli.web_server import ( - _ACTION_COMMANDS, - _ACTION_IDS, - _ACTION_LOG_DIR, - _ACTION_LOG_FILES, - _ACTION_PROCS, - _ACTION_RESULTS, - ) log_file_name = _ACTION_LOG_FILES.get(name) if log_file_name is None: raise HTTPException(status_code=404, detail=f"Unknown action: {name}") - log_path = _ACTION_LOG_DIR / log_file_name + log_dir = _action_log_dir() requested_lines = min(max(lines, 1), 2000) - tail = _tail_lines(log_path, requested_lines) + tail = _tail_lines(log_dir / log_file_name, requested_lines) durable_update_action_id = None update_receipt_summary = None if name == "hermes-update": - durable_lines = _tail_lines(_ACTION_LOG_DIR / "update.log", 2000) + durable_lines = _tail_lines(log_dir / "update.log", 2000) durable_update_action_id = _durable_completed_update_action_id(durable_lines) if durable_update_action_id: marker = f"=== hermes-update completed {durable_update_action_id} ===" if marker not in tail: tail = [*tail, marker][-requested_lines:] - # Phase-1 bullet 3 (#91277): the update receipt is the durable, - # structured truth about the last update — written by every run - # including refused/failed ones, and it survives the dashboard - # restarting itself mid-action. Surface its summary alongside the - # log-marker recovery so clients (Desktop, dashboard) READ the - # outcome instead of inferring it from liveness probes - # (#81193/#87359 class). + # The update receipt is the durable, structured truth about the last + # update (written by every run, incl. refused/failed; survives the + # dashboard restarting itself mid-action). Surface it so clients READ + # the outcome instead of inferring it from liveness probes. update_receipt_summary = _latest_update_receipt_summary() proc = _ACTION_PROCS.get(name) @@ -513,10 +471,9 @@ async def get_action_status(name: str, lines: int = 200): and update_receipt_summary is not None and update_receipt_summary.get("outcome") in ("success", "partial") ): - # No in-memory result and no log marker (e.g. log rotated), but - # the receipt proves a completed run: report its outcome rather - # than a null that clients time out on. ``partial`` maps to - # exit 1 exactly like the CLI run itself did. + # No in-memory result and no log marker (e.g. log rotated), but the + # receipt proves a completed run: report its outcome rather than a + # null clients time out on. ``partial`` maps to exit 1 like the CLI. exit_code = 0 if update_receipt_summary["outcome"] == "success" else 1 else: exit_code = proc.poll() @@ -527,10 +484,7 @@ async def get_action_status(name: str, lines: int = 200): proc.wait(timeout=1) except Exception: pass - _ACTION_RESULTS[name] = {"exit_code": exit_code, "pid": pid} - _ACTION_PROCS.pop(name, None) - _ACTION_COMMANDS.pop(name, None) - _ACTION_IDS.pop(name, None) + _finish_action(name, exit_code, pid) response = { "name": name, @@ -546,23 +500,29 @@ async def get_action_status(name: str, lines: int = 200): return response -def _latest_update_receipt_summary() -> Optional[Dict[str, Any]]: - """Compact summary of the most recent update receipt, or None. - - Phase-1 bullet 3 (#91277): the receipt (written by EVERY ``hermes - update`` run since #91283, including refused and failed ones, with a - ``latest.json`` pointer) is the durable success signal the Desktop and - dashboard should read instead of inferring outcomes from liveness - probes across the update's stop/start gap (#81193, #87359). Summary - only — steps and skips stay in the full receipt endpoint. - Never raises. - """ +def _read_latest_receipt() -> Optional[Dict[str, Any]]: + """Latest update receipt, or None on any failure (never raises).""" try: from hermes_cli.update_receipt import read_latest_receipt - receipt = read_latest_receipt() - if not receipt: - return None + return read_latest_receipt() or None + except Exception: + return None + + +def _latest_update_receipt_summary() -> Optional[Dict[str, Any]]: + """Compact summary of the most recent update receipt, or None. + + The receipt (written by EVERY ``hermes update`` run, including refused and + failed ones, with a ``latest.json`` pointer) is the durable success signal + the Desktop and dashboard read instead of inferring outcomes from liveness + probes across the update's stop/start gap. Summary only — steps and skips + stay in the full receipt endpoint. Never raises. + """ + receipt = _read_latest_receipt() + if not receipt: + return None + try: fleet = receipt.get("fleet") or [] return { "outcome": receipt.get("outcome"), @@ -583,19 +543,13 @@ def _latest_update_receipt_summary() -> Optional[Dict[str, Any]]: async def get_update_receipt(): """The most recent update receipt — the durable update-outcome record. - Phase-1 bullet 3 (#91277): dashboards and the Desktop read this instead - of inferring update success from backend liveness (the inference misread - the update's own restart gap as 'Backend update failed' / 'boot failed' - — #81193, #87359). Returns the FULL receipt (steps, skips, gateway + Dashboards and the Desktop read this instead of inferring update success + from backend liveness (which misread the update's own restart gap as a + failed update/boot). Returns the FULL receipt (steps, skips, gateway restart outcome, fleet matrix) plus a compact ``summary``; 404 when no update has run since receipts landed. """ - try: - from hermes_cli.update_receipt import read_latest_receipt - - receipt = read_latest_receipt() - except Exception: - receipt = None + receipt = _read_latest_receipt() if not receipt: raise HTTPException( status_code=404,