diff --git a/api/agent_health.py b/api/agent_health.py index cc3d354a..5660d28f 100644 --- a/api/agent_health.py +++ b/api/agent_health.py @@ -1,10 +1,21 @@ -"""Hermes agent/gateway heartbeat payload helpers (#716). +"""Hermes agent/gateway heartbeat payload helpers (#716, #1879). The WebUI process is not always paired with a long-running Hermes gateway. Some setups use WebUI only, while self-hosted messaging deployments run a separate Hermes gateway daemon that records runtime metadata in the Hermes Agent home. This module turns those existing safe runtime signals into a small UI-facing heartbeat without shelling out or adding psutil as a hard dependency. + +Cross-container note (#1879): ``gateway.status.get_running_pid()`` uses +``fcntl.flock`` and ``os.kill(pid, 0)``, both of which require the caller to +share a PID namespace with the gateway process. In multi-container deployments +where the WebUI runs separately from ``hermes-agent`` and only a Hermes data +volume is shared, those checks always return ``None`` and the dashboard +incorrectly shows "Gateway not running". To stay accurate without forcing a +``pid: "service:hermes-agent"`` compose workaround, we accept a recent +``updated_at`` timestamp on ``gateway_state.json`` (combined with +``gateway_state == "running"``) as an equivalent live-process signal — the +gateway already writes that file on every tick. """ from __future__ import annotations @@ -14,10 +25,67 @@ from datetime import datetime, timezone from typing import Any +# Two cron ticks (~60s each). Chosen to avoid false negatives during brief +# gateway restarts while still surfacing a true outage within a couple of +# minutes. Override is intentionally not exposed: keep the check deterministic +# and identical across deployments so support diagnostics are reproducible. +GATEWAY_FRESHNESS_THRESHOLD_S: float = 120.0 + + def _checked_at() -> str: return datetime.now(timezone.utc).isoformat() +def _runtime_status_is_fresh( + runtime_status: dict[str, Any] | None, + *, + now: datetime | None = None, + threshold_s: float = GATEWAY_FRESHNESS_THRESHOLD_S, +) -> bool: + """Return ``True`` when ``gateway_state.json`` looks freshly written. + + "Fresh" means the gateway self-reported ``running`` and the ``updated_at`` + ISO-8601 timestamp is no older than ``threshold_s`` seconds. This is the + cross-container liveness signal used when ``get_running_pid()`` returns + ``None`` purely because of PID-namespace isolation (#1879). + + Any unparseable input is treated as "not fresh" — a stale or missing + timestamp must never report alive. + """ + if not isinstance(runtime_status, dict): + return False + if runtime_status.get("gateway_state") != "running": + return False + + raw_updated_at = runtime_status.get("updated_at") + if not isinstance(raw_updated_at, str) or not raw_updated_at: + return False + + # ``datetime.fromisoformat`` accepts the exact format gateway/status.py + # writes (``datetime.now(timezone.utc).isoformat()``). We deliberately + # don't pull in dateutil — keeping this stdlib-only matches the rest of + # this module. + try: + updated_at = datetime.fromisoformat(raw_updated_at) + except (TypeError, ValueError): + return False + + if updated_at.tzinfo is None: + # A naive timestamp could mean anything across containers / hosts. + # Refuse to interpret it rather than assume UTC. + return False + + reference = now if now is not None else datetime.now(timezone.utc) + age_s = (reference - updated_at).total_seconds() + if age_s < 0: + # Clock skew between containers can produce small negatives. A future + # timestamp is still a "fresh" signal — the gateway clearly wrote it + # very recently — so accept it. A wildly-future timestamp (> threshold + # in the future) is rejected to avoid trusting a broken clock. + return -age_s <= threshold_s + return age_s <= threshold_s + + def _gateway_status_module(): """Load gateway.status lazily so tests and WebUI-only installs stay isolated.""" return importlib.import_module("gateway.status") @@ -111,6 +179,23 @@ def build_agent_health_payload() -> dict[str, Any]: }, } + # Cross-container fallback (#1879): when ``get_running_pid()`` cannot see + # the gateway because we're in a different PID namespace, a recent + # ``updated_at`` on ``gateway_state.json`` is a reliable equivalent signal + # since the gateway writes it on every tick. We only trust this fallback + # when the gateway also self-reports ``gateway_state == "running"`` so + # crash-without-cleanup scenarios still surface as "down". + if _runtime_status_is_fresh(runtime_status): + return { + "alive": True, + "checked_at": checked_at, + "details": { + "state": "alive", + "reason": "cross_container_freshness", + **safe_details, + }, + } + if isinstance(runtime_status, dict): return { "alive": False, diff --git a/tests/test_issue1879_cross_container_gateway_liveness.py b/tests/test_issue1879_cross_container_gateway_liveness.py new file mode 100644 index 00000000..262b8f23 --- /dev/null +++ b/tests/test_issue1879_cross_container_gateway_liveness.py @@ -0,0 +1,290 @@ +"""Regression coverage for #1879 — gateway liveness across PID namespaces. + +The gateway's ``get_running_pid()`` uses ``fcntl.flock`` and ``os.kill(pid, 0)``, +both of which require the caller to share a PID namespace with the gateway +process. In multi-container deployments (gateway in one container, WebUI in +another, no ``pid: "service:hermes-agent"`` workaround) those checks always +fail and the dashboard incorrectly reports "Gateway not running". + +The fix in ``api/agent_health.py`` adds a freshness fallback: when +``get_running_pid()`` returns ``None`` but ``gateway_state.json`` reports +``gateway_state == "running"`` AND ``updated_at`` is within +``GATEWAY_FRESHNESS_THRESHOLD_S`` (two cron ticks), trust the timestamp as a +cross-container liveness signal. + +These tests pin every behavior the fix promises: + + * fresh + running gateway_state, no PID → alive (cross-container path) + * stale updated_at + running → down (no false positives) + * fresh updated_at + non-running state → down (crash-without-cleanup case) + * malformed / missing / naive timestamp → down (no parser-quirk false alive) + * future timestamp within threshold → alive (clock skew tolerance) + * future timestamp beyond threshold → down (broken clock rejected) + * PID-based path still wins when PID exists (no behavior change for + same-namespace deployments — backward compat with #716 contract) +""" + +from __future__ import annotations + +from datetime import datetime, timedelta, timezone + +import pytest + + +class _FakeGatewayStatus: + def __init__(self, runtime_status, running_pid): + self._runtime_status = runtime_status + self._running_pid = running_pid + + def read_runtime_status(self): + return self._runtime_status + + def get_running_pid(self, cleanup_stale=False): + assert cleanup_stale is False + return self._running_pid + + +def _runtime_status(updated_at: str | None, **overrides): + payload = { + "gateway_state": "running", + "updated_at": updated_at, + "active_agents": 1, + "platforms": {"telegram": {"state": "connected"}}, + } + payload.update(overrides) + return payload + + +def _iso(dt: datetime) -> str: + return dt.isoformat() + + +# -- Fresh updated_at, no PID ------------------------------------------------- + + +def test_fresh_runtime_status_reports_alive_when_pid_lookup_returns_none(monkeypatch): + """Container A's WebUI cannot see Container B's PID, but sees the file.""" + from api import agent_health + + fresh_ts = _iso(datetime.now(timezone.utc) - timedelta(seconds=30)) + + monkeypatch.setattr( + agent_health, + "_gateway_status_module", + lambda: _FakeGatewayStatus(_runtime_status(fresh_ts), running_pid=None), + ) + + payload = agent_health.build_agent_health_payload() + + assert payload["alive"] is True + assert payload["details"]["state"] == "alive" + assert payload["details"]["reason"] == "cross_container_freshness" + assert payload["details"]["gateway_state"] == "running" + assert payload["details"]["updated_at"] == fresh_ts + + +def test_cross_container_alive_path_does_not_leak_raw_process_fields(monkeypatch): + """Same redaction guarantees as the in-namespace alive path (#716).""" + from api import agent_health + + fresh_ts = _iso(datetime.now(timezone.utc) - timedelta(seconds=10)) + runtime = _runtime_status( + fresh_ts, + pid=7, + argv=["hermes", "gateway", "--token", "secret-token"], + command="hermes gateway --token secret-token", + executable="/opt/hermes/.venv/bin/python", + env={"OPENAI_API_KEY": "sk-secret"}, + ) + monkeypatch.setattr( + agent_health, + "_gateway_status_module", + lambda: _FakeGatewayStatus(runtime, running_pid=None), + ) + + payload = agent_health.build_agent_health_payload() + rendered = repr(payload) + + assert payload["alive"] is True + for forbidden in ("secret-token", "sk-secret", "argv", "command", "executable"): + assert forbidden not in rendered + assert "pid" not in payload["details"] + + +# -- Stale / missing / malformed timestamps ----------------------------------- + + +def test_stale_updated_at_reports_down_even_when_gateway_state_running(monkeypatch): + """A long-dead gateway with a fossilised state file must surface as down.""" + from api import agent_health + + stale_ts = _iso(datetime.now(timezone.utc) - timedelta(seconds=300)) + + monkeypatch.setattr( + agent_health, + "_gateway_status_module", + lambda: _FakeGatewayStatus(_runtime_status(stale_ts), running_pid=None), + ) + + payload = agent_health.build_agent_health_payload() + + assert payload["alive"] is False + assert payload["details"]["state"] == "down" + assert payload["details"]["reason"] == "gateway_not_running" + + +def test_fresh_updated_at_with_non_running_state_reports_down(monkeypatch): + """Crash-without-cleanup: file is fresh but gateway said it was stopping.""" + from api import agent_health + + fresh_ts = _iso(datetime.now(timezone.utc) - timedelta(seconds=10)) + runtime = _runtime_status(fresh_ts, gateway_state="stopping") + + monkeypatch.setattr( + agent_health, + "_gateway_status_module", + lambda: _FakeGatewayStatus(runtime, running_pid=None), + ) + + payload = agent_health.build_agent_health_payload() + + assert payload["alive"] is False + assert payload["details"]["state"] == "down" + + +@pytest.mark.parametrize( + "broken_value", + [ + None, + "", + "not-a-timestamp", + "2026-13-40T99:99:99", # parse error + 12345, # wrong type + "2026-05-08T12:00:00", # naive (no tz) — refuse to guess + ], +) +def test_malformed_or_naive_updated_at_does_not_report_alive(monkeypatch, broken_value): + """Any non-aware ISO-8601 UTC timestamp is treated as not fresh.""" + from api import agent_health + + monkeypatch.setattr( + agent_health, + "_gateway_status_module", + lambda: _FakeGatewayStatus(_runtime_status(broken_value), running_pid=None), + ) + + payload = agent_health.build_agent_health_payload() + + assert payload["alive"] is False + assert payload["details"]["state"] == "down" + + +# -- Clock-skew tolerance ----------------------------------------------------- + + +def test_slightly_future_updated_at_is_accepted_for_clock_skew(monkeypatch): + """Containers may have small clock drift; <=threshold future is fresh.""" + from api import agent_health + + near_future = _iso(datetime.now(timezone.utc) + timedelta(seconds=15)) + monkeypatch.setattr( + agent_health, + "_gateway_status_module", + lambda: _FakeGatewayStatus(_runtime_status(near_future), running_pid=None), + ) + + payload = agent_health.build_agent_health_payload() + + assert payload["alive"] is True + assert payload["details"]["reason"] == "cross_container_freshness" + + +def test_far_future_updated_at_is_rejected(monkeypatch): + """A timestamp implausibly far in the future signals a broken clock.""" + from api import agent_health + + far_future = _iso(datetime.now(timezone.utc) + timedelta(hours=1)) + monkeypatch.setattr( + agent_health, + "_gateway_status_module", + lambda: _FakeGatewayStatus(_runtime_status(far_future), running_pid=None), + ) + + payload = agent_health.build_agent_health_payload() + + assert payload["alive"] is False + + +# -- Backward compatibility with #716 PID path -------------------------------- + + +def test_pid_based_alive_path_unchanged_when_namespace_is_shared(monkeypatch): + """In-namespace deployments must keep the existing #716 contract: when + ``get_running_pid`` returns a real PID, ``reason`` is NOT set (only the + cross-container path adds a reason key on success).""" + from api import agent_health + + runtime = _runtime_status(_iso(datetime.now(timezone.utc))) + monkeypatch.setattr( + agent_health, + "_gateway_status_module", + lambda: _FakeGatewayStatus(runtime, running_pid=4242), + ) + + payload = agent_health.build_agent_health_payload() + + assert payload["alive"] is True + assert payload["details"]["state"] == "alive" + assert "reason" not in payload["details"] + + +def test_no_runtime_status_still_reports_unknown(monkeypatch): + """No runtime status + no PID = WebUI-only deployment, still ``unknown``.""" + from api import agent_health + + monkeypatch.setattr( + agent_health, + "_gateway_status_module", + lambda: _FakeGatewayStatus(runtime_status=None, running_pid=None), + ) + + payload = agent_health.build_agent_health_payload() + + assert payload["alive"] is None + assert payload["details"] == {"state": "unknown", "reason": "gateway_not_configured"} + + +# -- _runtime_status_is_fresh unit-level coverage ----------------------------- + + +def test_runtime_status_is_fresh_unit_helper(): + """Direct coverage of the boundary helper for future maintainers.""" + from api import agent_health + + now = datetime(2026, 5, 8, 12, 0, 0, tzinfo=timezone.utc) + + # Boundary: exactly threshold = fresh. + on_boundary = _iso(now - timedelta(seconds=agent_health.GATEWAY_FRESHNESS_THRESHOLD_S)) + assert agent_health._runtime_status_is_fresh( + {"gateway_state": "running", "updated_at": on_boundary}, + now=now, + ) + + # Just past threshold = not fresh. + just_past = _iso( + now - timedelta(seconds=agent_health.GATEWAY_FRESHNESS_THRESHOLD_S + 0.001) + ) + assert not agent_health._runtime_status_is_fresh( + {"gateway_state": "running", "updated_at": just_past}, + now=now, + ) + + # gateway_state must be exactly "running" — anything else is not fresh. + assert not agent_health._runtime_status_is_fresh( + {"gateway_state": "RUNNING", "updated_at": _iso(now)}, + now=now, + ) + + # Non-dict input rejected. + assert not agent_health._runtime_status_is_fresh(None, now=now) + assert not agent_health._runtime_status_is_fresh("running", now=now)