fix(#1879): cross-container gateway liveness via state-file freshness
The dashboard banner 'Hermes agent is not responding' fires on every multi-container deployment that doesn't set 'pid: "service:hermes-agent"' in compose, because get_running_pid() relies on fcntl.flock and os.kill(pid, 0) — both PID-namespace-scoped and invisible across container boundaries. Fix: when get_running_pid() returns None, fall back to a freshness check on gateway_state.json. The gateway already writes that file on every tick with gateway_state == 'running' and an aware ISO-8601 updated_at timestamp, so a recent (<= 120s) timestamp is an equivalent live-process signal that needs only a shared volume — no PID namespace, no compose workaround, no extra HTTP probe URL. Behavior preserved: - In-namespace deployments still hit the PID-based path first; payload shape unchanged (no 'reason' key) so #716 contract holds. - Cross-container alive path adds reason='cross_container_freshness' so support diagnostics can tell which signal succeeded. - Stale updated_at, non-running gateway_state, malformed/naive/missing timestamps, and timestamps far in the future all still report 'down' — the fallback never produces a false positive. - Same redaction rules: argv/command/executable/env/raw pid never leak. Tests: 15 new cases in test_issue1879_cross_container_gateway_liveness.py covering the cross-container alive path, every refusal case, clock-skew tolerance, and backward compat with the #716 PID path. Existing #716 heartbeat tests (8) continue to pass.
This commit is contained in:
1 parent
82aa628317
commit
efcfff3d7f
2 files changed
+376
-1
No files matched your search
+86
-1
@@ -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,
|
||||
|
||||
@@ -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)
|
||||
Reference in new issue
Block a user