mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-07-22 16:25:58 +00:00
fix(status): make gateway_updated_at a stable RFC3339-or-null contract (#68657)
The /api/status gateway_updated_at field and the gateway /health/detailed updated_at field passed through whatever gateway_state.json contained, untyped. All current writers emit RFC3339 via _utc_now_iso(), but legacy gateways wrote unix epoch floats, and a corrupt or hand-edited state file can inject numbers or arbitrary garbage — while the frontend types (web/src/lib/api.ts) declare string | null. Add normalize_updated_at() in gateway/status.py as the single funnel: - str: accepted iff datetime.fromisoformat parses (trailing Z tolerated); naive timestamps coerced to UTC; canonical isoformat returned - int/float: treated as unix epoch seconds -> UTC ISO string, with a plausibility guard (reject < 2000-01-01, > now+1day, non-finite) - bool: rejected explicitly (int subclass, but never a timestamp) - anything else: None Apply it at both emit sites: the dashboard /api/status handler (covers both the local read_runtime_status() branch and the remote /health/detailed cross-container fallback branch) and the gateway API server's /health/detailed response. Contract tests: parametrized /api/status normalization (epoch float/int, garbage string, None, bool, dict, absent key), remote-health numeric and garbage bodies, dashboard shape test round-trip assertion, direct normalize_updated_at units (range guards, Z suffix, naive coercion, non-finite floats), and a write_runtime_status -> read_runtime_status round-trip proving the writer side stays tz-aware parseable.
This commit is contained in:
parent
8fd5b25898
commit
a2c2ec6332
6 changed files with 326 additions and 2 deletions
|
|
@ -1910,6 +1910,7 @@ class APIServerAdapter(BasePlatformAdapter):
|
|||
from gateway.status import (
|
||||
derive_gateway_busy,
|
||||
derive_gateway_drainable,
|
||||
normalize_updated_at,
|
||||
parse_active_agents,
|
||||
read_runtime_status,
|
||||
)
|
||||
|
|
@ -1948,7 +1949,9 @@ class APIServerAdapter(BasePlatformAdapter):
|
|||
gateway_state=gw_state,
|
||||
),
|
||||
"exit_reason": runtime.get("exit_reason"),
|
||||
"updated_at": runtime.get("updated_at"),
|
||||
# Contract: updated_at is RFC3339 string | null, never a number —
|
||||
# the state file may carry legacy epoch floats or hand-edited junk.
|
||||
"updated_at": normalize_updated_at(runtime.get("updated_at")),
|
||||
"pid": os.getpid(),
|
||||
})
|
||||
|
||||
|
|
|
|||
|
|
@ -186,6 +186,62 @@ def _utc_now_iso() -> str:
|
|||
return datetime.now(timezone.utc).isoformat()
|
||||
|
||||
|
||||
# Reject epoch values before 2000-01-01T00:00:00Z: nothing in Hermes' lifetime
|
||||
# legitimately produced a gateway heartbeat last century, so anything older is
|
||||
# a corrupt or hand-edited state file (e.g. an accidental 0 / tiny int).
|
||||
_EPOCH_MIN_PLAUSIBLE = 946684800.0 # 2000-01-01T00:00:00Z
|
||||
|
||||
|
||||
def normalize_updated_at(value: Any) -> Optional[str]:
|
||||
"""Coerce a persisted ``updated_at`` value to an RFC3339 string or ``None``.
|
||||
|
||||
The ``gateway_state.json`` writers all emit RFC3339 via :func:`_utc_now_iso`,
|
||||
but the file can also be produced by legacy gateways (which wrote unix
|
||||
epoch floats), hand edits, or partial corruption. Every read/emit surface
|
||||
(``/api/status``'s ``gateway_updated_at``, the gateway's
|
||||
``/health/detailed`` ``updated_at``) promises consumers ``string | null``
|
||||
(see ``web/src/lib/api.ts``), so this funnel enforces that contract:
|
||||
|
||||
- ``str``: accepted iff :meth:`datetime.fromisoformat` parses it (a
|
||||
trailing ``Z`` is tolerated). Naive timestamps are coerced to UTC.
|
||||
Returns the canonical ``datetime.isoformat()`` rendering.
|
||||
- ``int`` / ``float``: treated as unix epoch **seconds** and converted to
|
||||
a UTC ISO string. Implausible values — before 2000-01-01, more than a
|
||||
day in the future, or non-finite — return ``None``.
|
||||
- ``bool``: returns ``None``. Although ``bool`` is an ``int`` subclass,
|
||||
``True``/``False`` as a timestamp is always garbage (epoch 0/1 would be
|
||||
rejected by the range guard anyway); rejecting explicitly keeps the
|
||||
behaviour documented rather than incidental.
|
||||
- anything else (``None``, dict, list, ...): ``None``.
|
||||
"""
|
||||
if isinstance(value, bool):
|
||||
return None
|
||||
if isinstance(value, str):
|
||||
raw = value.strip()
|
||||
# Python < 3.11 fromisoformat rejects a trailing 'Z'; tolerate it.
|
||||
if raw.endswith(("Z", "z")):
|
||||
raw = raw[:-1] + "+00:00"
|
||||
try:
|
||||
parsed = datetime.fromisoformat(raw)
|
||||
except ValueError:
|
||||
return None
|
||||
if parsed.tzinfo is None:
|
||||
parsed = parsed.replace(tzinfo=timezone.utc)
|
||||
return parsed.isoformat()
|
||||
if isinstance(value, (int, float)):
|
||||
seconds = float(value)
|
||||
if seconds != seconds or seconds in (float("inf"), float("-inf")):
|
||||
return None
|
||||
now = datetime.now(timezone.utc).timestamp()
|
||||
if seconds < _EPOCH_MIN_PLAUSIBLE or seconds > now + 86400:
|
||||
return None
|
||||
try:
|
||||
return datetime.fromtimestamp(seconds, tz=timezone.utc).isoformat()
|
||||
except (OverflowError, OSError, ValueError):
|
||||
return None
|
||||
return None
|
||||
|
||||
|
||||
def terminate_pid(pid: int, *, force: bool = False) -> None:
|
||||
"""Terminate a PID with platform-appropriate force semantics.
|
||||
|
||||
|
|
|
|||
|
|
@ -89,6 +89,7 @@ from gateway.status import (
|
|||
get_running_pid_cached,
|
||||
get_running_pid,
|
||||
get_runtime_status_running_pid,
|
||||
normalize_updated_at,
|
||||
parse_active_agents,
|
||||
read_runtime_status,
|
||||
)
|
||||
|
|
@ -3034,7 +3035,11 @@ async def get_status(profile: Optional[str] = None):
|
|||
if key in configured_gateway_platforms
|
||||
}
|
||||
gateway_exit_reason = runtime.get("exit_reason")
|
||||
gateway_updated_at = runtime.get("updated_at")
|
||||
# Contract: gateway_updated_at is RFC3339 string | null, never a
|
||||
# number. ``runtime`` here may be the local gateway_state.json
|
||||
# (legacy gateways wrote epoch floats; hand edits can inject
|
||||
# anything) or a remote /health/detailed body — normalize both.
|
||||
gateway_updated_at = normalize_updated_at(runtime.get("updated_at"))
|
||||
if not gateway_running:
|
||||
gateway_state = gateway_state if gateway_state in {"stopped", "startup_failed"} else "stopped"
|
||||
gateway_platforms = {}
|
||||
|
|
|
|||
|
|
@ -2104,3 +2104,102 @@ class TestPermissionErrorOnLockFile:
|
|||
monkeypatch.setattr(Path, "unlink", deny_unlink)
|
||||
|
||||
assert status.acquire_gateway_runtime_lock() is False
|
||||
|
||||
|
||||
class TestNormalizeUpdatedAt:
|
||||
"""Unit tests for the updated_at RFC3339|None normalization funnel."""
|
||||
|
||||
def test_epoch_int_converts_to_utc_iso(self):
|
||||
from datetime import datetime, timezone
|
||||
|
||||
result = status.normalize_updated_at(1750000000)
|
||||
assert isinstance(result, str)
|
||||
parsed = datetime.fromisoformat(result)
|
||||
assert parsed.tzinfo is not None
|
||||
assert parsed == datetime.fromtimestamp(1750000000, tz=timezone.utc)
|
||||
|
||||
def test_epoch_float_converts_to_utc_iso(self):
|
||||
from datetime import datetime, timezone
|
||||
|
||||
result = status.normalize_updated_at(1750000000.5)
|
||||
assert isinstance(result, str)
|
||||
parsed = datetime.fromisoformat(result)
|
||||
assert parsed == datetime.fromtimestamp(1750000000.5, tz=timezone.utc)
|
||||
|
||||
def test_iso_with_z_suffix_accepted(self):
|
||||
from datetime import datetime, timezone
|
||||
|
||||
result = status.normalize_updated_at("2026-07-21T12:00:00Z")
|
||||
assert result is not None
|
||||
parsed = datetime.fromisoformat(result)
|
||||
assert parsed.tzinfo is not None
|
||||
assert parsed == datetime(2026, 7, 21, 12, 0, 0, tzinfo=timezone.utc)
|
||||
|
||||
def test_naive_iso_coerced_to_utc(self):
|
||||
from datetime import datetime, timezone
|
||||
|
||||
result = status.normalize_updated_at("2026-07-21T12:00:00")
|
||||
assert result is not None
|
||||
parsed = datetime.fromisoformat(result)
|
||||
assert parsed.tzinfo is not None
|
||||
assert parsed.utcoffset().total_seconds() == 0
|
||||
assert parsed == datetime(2026, 7, 21, 12, 0, 0, tzinfo=timezone.utc)
|
||||
|
||||
def test_offset_aware_iso_round_trips_canonically(self):
|
||||
canonical = "2026-07-21T12:00:00+00:00"
|
||||
assert status.normalize_updated_at(canonical) == canonical
|
||||
|
||||
def test_garbage_string_returns_none(self):
|
||||
assert status.normalize_updated_at("not-a-timestamp") is None
|
||||
|
||||
def test_none_returns_none(self):
|
||||
assert status.normalize_updated_at(None) is None
|
||||
|
||||
def test_structured_garbage_returns_none(self):
|
||||
assert status.normalize_updated_at({"a": 1}) is None
|
||||
assert status.normalize_updated_at([1750000000]) is None
|
||||
|
||||
def test_bool_returns_none(self):
|
||||
# bool is an int subclass, but True/False as an epoch timestamp is
|
||||
# always garbage (and 0/1 would fail the range guard regardless).
|
||||
# The funnel rejects bools explicitly — documented behaviour.
|
||||
assert status.normalize_updated_at(True) is None
|
||||
assert status.normalize_updated_at(False) is None
|
||||
|
||||
def test_epoch_before_2000_rejected(self):
|
||||
assert status.normalize_updated_at(0) is None
|
||||
assert status.normalize_updated_at(946684799) is None # 1999-12-31T23:59:59Z
|
||||
assert status.normalize_updated_at(-1750000000) is None
|
||||
|
||||
def test_epoch_far_future_rejected(self):
|
||||
assert status.normalize_updated_at(time.time() + 90000) is None # > now+1day
|
||||
assert status.normalize_updated_at(4e18) is None
|
||||
|
||||
def test_epoch_slightly_future_accepted(self):
|
||||
# Clock skew tolerance: up to a day ahead is plausible.
|
||||
assert status.normalize_updated_at(time.time() + 3600) is not None
|
||||
|
||||
def test_non_finite_floats_rejected(self):
|
||||
assert status.normalize_updated_at(float("nan")) is None
|
||||
assert status.normalize_updated_at(float("inf")) is None
|
||||
assert status.normalize_updated_at(float("-inf")) is None
|
||||
|
||||
|
||||
class TestRuntimeStatusUpdatedAtContract:
|
||||
def test_write_then_read_updated_at_parses_tz_aware(self, tmp_path, monkeypatch):
|
||||
"""write_runtime_status persists an updated_at that fromisoformat
|
||||
parses as a timezone-aware datetime — the writer side of the
|
||||
string|null contract every emit surface relies on."""
|
||||
from datetime import datetime
|
||||
|
||||
monkeypatch.setenv("HERMES_HOME", str(tmp_path))
|
||||
|
||||
status.write_runtime_status(gateway_state="running")
|
||||
|
||||
payload = status.read_runtime_status()
|
||||
updated_at = payload["updated_at"]
|
||||
assert isinstance(updated_at, str)
|
||||
parsed = datetime.fromisoformat(updated_at)
|
||||
assert parsed.tzinfo is not None
|
||||
# And it survives the normalization funnel unchanged (canonical form).
|
||||
assert status.normalize_updated_at(updated_at) == updated_at
|
||||
|
|
|
|||
|
|
@ -92,6 +92,18 @@ def test_status_preserves_existing_fields(loopback_client):
|
|||
}
|
||||
missing = expected_keys - set(body.keys())
|
||||
assert not missing, f"/api/status dropped fields: {missing}"
|
||||
# gateway_updated_at is a typed contract (web/src/lib/api.ts declares
|
||||
# string | null): it must never be a number, and any string must
|
||||
# round-trip through fromisoformat as a timezone-aware timestamp.
|
||||
updated_at = body["gateway_updated_at"]
|
||||
assert isinstance(updated_at, (str, type(None))), (
|
||||
f"gateway_updated_at must be str|None, got {type(updated_at).__name__}: {updated_at!r}"
|
||||
)
|
||||
if updated_at is not None:
|
||||
from datetime import datetime
|
||||
parsed = datetime.fromisoformat(updated_at)
|
||||
assert parsed.tzinfo is not None
|
||||
assert parsed.isoformat() == updated_at, "gateway_updated_at is not canonical isoformat"
|
||||
|
||||
|
||||
# Host-local detail (absolute paths, PID, internal gateway URL) is deployment
|
||||
|
|
|
|||
|
|
@ -7216,6 +7216,155 @@ class TestGatewayBusyReadout:
|
|||
assert data["gateway_busy"] is False
|
||||
|
||||
|
||||
class TestGatewayUpdatedAtContract:
|
||||
"""Contract tests for /api/status ``gateway_updated_at``.
|
||||
|
||||
The field is promised to consumers (web/src/lib/api.ts declares
|
||||
``string | null``) as an RFC3339 timestamp or null — NEVER a number.
|
||||
Legacy gateways wrote epoch floats into gateway_state.json and the file
|
||||
is hand-editable, so the endpoint must normalize whatever it reads.
|
||||
"""
|
||||
|
||||
@pytest.fixture(autouse=True)
|
||||
def _setup_test_client(self):
|
||||
try:
|
||||
from starlette.testclient import TestClient
|
||||
except ImportError:
|
||||
pytest.skip("fastapi/starlette not installed")
|
||||
|
||||
from hermes_cli.web_server import app, _SESSION_HEADER_NAME, _SESSION_TOKEN
|
||||
self.client = TestClient(app)
|
||||
self.client.headers[_SESSION_HEADER_NAME] = _SESSION_TOKEN
|
||||
|
||||
@staticmethod
|
||||
def _assert_contract(value):
|
||||
"""gateway_updated_at is None or a tz-aware-parseable ISO string."""
|
||||
from datetime import datetime
|
||||
|
||||
assert not isinstance(value, bool), f"bool leaked: {value!r}"
|
||||
assert not isinstance(value, (int, float)), f"number leaked: {value!r}"
|
||||
if value is not None:
|
||||
assert isinstance(value, str)
|
||||
parsed = datetime.fromisoformat(value)
|
||||
assert parsed.tzinfo is not None, f"naive timestamp leaked: {value!r}"
|
||||
|
||||
@pytest.mark.parametrize("updated_at", [
|
||||
1750000000.5, # legacy epoch float
|
||||
1750000000, # legacy epoch int
|
||||
"not-a-timestamp", # garbage string
|
||||
None, # explicit null
|
||||
True, # bool (int subclass) garbage
|
||||
{"nested": "junk"}, # structured garbage
|
||||
])
|
||||
def test_local_runtime_updated_at_normalized(self, monkeypatch, updated_at):
|
||||
import hermes_cli.web_server as ws
|
||||
|
||||
monkeypatch.setattr(ws, "get_running_pid_cached", lambda: 1234)
|
||||
monkeypatch.setattr(ws, "read_runtime_status", lambda: {
|
||||
"gateway_state": "running",
|
||||
"platforms": {},
|
||||
"active_agents": 0,
|
||||
"updated_at": updated_at,
|
||||
})
|
||||
|
||||
resp = self.client.get("/api/status")
|
||||
assert resp.status_code == 200
|
||||
self._assert_contract(resp.json()["gateway_updated_at"])
|
||||
|
||||
def test_local_runtime_updated_at_absent(self, monkeypatch):
|
||||
"""Key missing entirely from the status file → null, not a crash."""
|
||||
import hermes_cli.web_server as ws
|
||||
|
||||
monkeypatch.setattr(ws, "get_running_pid_cached", lambda: 1234)
|
||||
monkeypatch.setattr(ws, "read_runtime_status", lambda: {
|
||||
"gateway_state": "running",
|
||||
"platforms": {},
|
||||
"active_agents": 0,
|
||||
})
|
||||
|
||||
data = self.client.get("/api/status").json()
|
||||
assert data["gateway_updated_at"] is None
|
||||
|
||||
def test_local_runtime_valid_epoch_becomes_iso_string(self, monkeypatch):
|
||||
"""A plausible legacy epoch value is converted, not dropped."""
|
||||
from datetime import datetime, timezone
|
||||
import hermes_cli.web_server as ws
|
||||
|
||||
epoch = 1750000000
|
||||
monkeypatch.setattr(ws, "get_running_pid_cached", lambda: 1234)
|
||||
monkeypatch.setattr(ws, "read_runtime_status", lambda: {
|
||||
"gateway_state": "running",
|
||||
"platforms": {},
|
||||
"active_agents": 0,
|
||||
"updated_at": epoch,
|
||||
})
|
||||
|
||||
value = self.client.get("/api/status").json()["gateway_updated_at"]
|
||||
assert isinstance(value, str)
|
||||
parsed = datetime.fromisoformat(value)
|
||||
assert parsed.tzinfo is not None
|
||||
assert parsed == datetime.fromtimestamp(epoch, tz=timezone.utc)
|
||||
|
||||
def test_local_runtime_valid_iso_passes_through_parseable(self, monkeypatch):
|
||||
"""The canonical writer format survives normalization round-trip."""
|
||||
from datetime import datetime
|
||||
import hermes_cli.web_server as ws
|
||||
|
||||
monkeypatch.setattr(ws, "get_running_pid_cached", lambda: 1234)
|
||||
monkeypatch.setattr(ws, "read_runtime_status", lambda: {
|
||||
"gateway_state": "running",
|
||||
"platforms": {},
|
||||
"active_agents": 0,
|
||||
"updated_at": "2026-07-21T12:00:00+00:00",
|
||||
})
|
||||
|
||||
value = self.client.get("/api/status").json()["gateway_updated_at"]
|
||||
assert isinstance(value, str)
|
||||
assert datetime.fromisoformat(value).tzinfo is not None
|
||||
|
||||
def test_remote_health_numeric_updated_at_normalized(self, monkeypatch):
|
||||
"""Cross-container path: the remote /health/detailed body is the
|
||||
runtime source, and a numeric updated_at from an older gateway build
|
||||
must still come out as string|null."""
|
||||
import hermes_cli.web_server as ws
|
||||
|
||||
monkeypatch.setattr(ws, "get_running_pid_cached", lambda: None)
|
||||
monkeypatch.setattr(ws, "read_runtime_status", lambda: None)
|
||||
monkeypatch.setattr(ws, "_GATEWAY_HEALTH_URL", "http://gw:8642")
|
||||
monkeypatch.setattr(ws, "_probe_gateway_health", lambda: (True, {
|
||||
"status": "ok",
|
||||
"gateway_state": "running",
|
||||
"platforms": {},
|
||||
"updated_at": 1750000000.25,
|
||||
"pid": 999,
|
||||
}))
|
||||
|
||||
resp = self.client.get("/api/status")
|
||||
assert resp.status_code == 200
|
||||
data = resp.json()
|
||||
assert data["gateway_running"] is True
|
||||
self._assert_contract(data["gateway_updated_at"])
|
||||
# A plausible epoch is converted, not nulled.
|
||||
assert isinstance(data["gateway_updated_at"], str)
|
||||
|
||||
def test_remote_health_garbage_updated_at_nulled(self, monkeypatch):
|
||||
"""Remote body with unparseable updated_at → null, never verbatim."""
|
||||
import hermes_cli.web_server as ws
|
||||
|
||||
monkeypatch.setattr(ws, "get_running_pid_cached", lambda: None)
|
||||
monkeypatch.setattr(ws, "read_runtime_status", lambda: None)
|
||||
monkeypatch.setattr(ws, "_GATEWAY_HEALTH_URL", "http://gw:8642")
|
||||
monkeypatch.setattr(ws, "_probe_gateway_health", lambda: (True, {
|
||||
"status": "ok",
|
||||
"gateway_state": "running",
|
||||
"platforms": {},
|
||||
"updated_at": "yesterday-ish",
|
||||
}))
|
||||
|
||||
data = self.client.get("/api/status").json()
|
||||
assert data["gateway_updated_at"] is None
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Dashboard theme normaliser tests
|
||||
# ---------------------------------------------------------------------------
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue