Release v0.51.225 — Release GS (stage-p7 — remote gateway health probe resolves gateway_state #3355) (#3458)
* fix(health): probe /health/detailed first and unify gateway env vars (#3355) * docs(changelog): v0.51.225 — remote gateway health probe gateway_state fix (#3355) * fix(health): normalize gateway URL health-suffix + cap remote body read (#3355 Codex follow-up) Codex review of #3355 found two issues: (1) SILENT — a gateway env var already pointing at a health path (e.g. GATEWAY_HEALTH_URL=http://host/health) produced doubled paths like /health/health/detailed once probe paths were appended. Now strip a trailing /health/detailed, /health, /v1/health, /status suffix before appending (mirrors api/updates.py). (2) CORE — the new resp.read() on a 2xx body was unbounded; a large/trickled remote response could hang /api/health/agent or balloon memory. Cap the read at _REMOTE_PROBE_BODY_LIMIT_BYTES (64KB)+1 and skip JSON parse when over cap (still report the gateway alive, just without parsed gateway_state). Adds regression tests for both (no doubled /health/health path; oversized body is capped + does not hang). Also updated _FakeResp.read to accept the size arg. --------- Co-authored-by: Rod Boev <rod.boev@gmail.com> Co-authored-by: nesquena-hermes <[email protected]>
This commit is contained in:
nesquena-hermes
GitHub
parent
bb5aa02523
commit
ec46f05c92
3 changed files with 187 additions and 24 deletions
|
|
@ -3,6 +3,11 @@
|
|||
|
||||
## [Unreleased]
|
||||
|
||||
## [v0.51.225] — 2026-06-03 — Release GS (stage-p7 — remote gateway health probe resolves gateway_state)
|
||||
|
||||
### Fixed
|
||||
- The remote-gateway health probe now correctly reports `gateway_state`, so the Tasks/Cron banner lights up for Docker / remote-gateway deployments. The probe previously hit `/health` and `/status` (neither returns `gateway_state`) and never queried `/health/detailed` (which does), so `gateway_state == "running"` was never observed remotely. The probe now tries `/health/detailed` first, parses the JSON body of a 2xx response to extract `gateway_state`, and unifies the gateway base-URL env precedence to `GATEWAY_HEALTH_URL` > `HERMES_GATEWAY_HEALTH_URL` > `HERMES_API_URL` (#3355, @rodboev).
|
||||
|
||||
## [v0.51.224] — 2026-06-03 — Release GR (stage-p6 — profile tool/skill config authoritative on the streaming worker)
|
||||
|
||||
### Fixed
|
||||
|
|
|
|||
|
|
@ -306,38 +306,63 @@ def _runtime_detail_subset(runtime_status: dict[str, Any] | None) -> dict[str, A
|
|||
|
||||
_REMOTE_PROBE_TIMEOUT_S: float = 2.0
|
||||
_REMOTE_PROBE_CACHE_TTL_S: float = 5.0
|
||||
_REMOTE_PROBE_PATHS: tuple[str, ...] = ("/health", "/status", "/api/gateway/status")
|
||||
_REMOTE_PROBE_PATHS: tuple[str, ...] = ("/health/detailed", "/health", "/v1/health")
|
||||
# A gateway health payload is small JSON; cap the 2xx body read so a large or
|
||||
# slow-trickled remote response can't hang /api/health/agent or balloon memory.
|
||||
_REMOTE_PROBE_BODY_LIMIT_BYTES: int = 64 * 1024
|
||||
|
||||
_remote_probe_lock = threading.Lock()
|
||||
_remote_probe_cache: dict[str, Any] = {"url": None, "expires_at": 0.0, "result": None}
|
||||
|
||||
|
||||
def _remote_gateway_base_url() -> str | None:
|
||||
raw = os.environ.get("HERMES_API_URL")
|
||||
if not isinstance(raw, str):
|
||||
return None
|
||||
url = raw.strip()
|
||||
if not url:
|
||||
return None
|
||||
return url.rstrip("/")
|
||||
"""Return an explicit remote gateway base URL, or None for local-only setups.
|
||||
|
||||
Priority: GATEWAY_HEALTH_URL > HERMES_GATEWAY_HEALTH_URL > HERMES_API_URL.
|
||||
Returns ``None`` when no env var is set so the caller falls through to
|
||||
local PID/state checks.
|
||||
|
||||
Any of these env vars may legitimately point AT a health endpoint
|
||||
(e.g. ``GATEWAY_HEALTH_URL=http://host:8642/health``). Since the probe
|
||||
appends ``/health/detailed`` etc. to the returned base, strip a trailing
|
||||
health-path suffix first so we don't build ``/health/health/detailed``
|
||||
(mirrors the normalization in api/updates.py).
|
||||
"""
|
||||
for var in ("GATEWAY_HEALTH_URL", "HERMES_GATEWAY_HEALTH_URL", "HERMES_API_URL"):
|
||||
val = os.environ.get(var, "").strip()
|
||||
if val:
|
||||
base = val.rstrip("/")
|
||||
for suffix in ("/health/detailed", "/health", "/v1/health", "/status"):
|
||||
if base.endswith(suffix):
|
||||
base = base[: -len(suffix)].rstrip("/")
|
||||
break
|
||||
return base
|
||||
return None
|
||||
|
||||
|
||||
def _http_probe(url: str, timeout_s: float) -> tuple[bool, int | None, str | None]:
|
||||
"""GET ``url`` and return (ok, status_code, error_name).
|
||||
def _http_probe(url: str, timeout_s: float) -> tuple[bool, int | None, str | None, bytes | None]:
|
||||
"""GET ``url`` and return (ok, status_code, error_name, body).
|
||||
|
||||
``ok`` is True only for a 2xx response. 5xx and network errors are not OK.
|
||||
4xx is also treated as "responded" (the gateway is up, just answering 404
|
||||
on this particular path) so the caller can move on to the next path.
|
||||
``body`` is the raw response bytes for 2xx responses, None otherwise.
|
||||
"""
|
||||
req = urllib_request.Request(url, method="GET")
|
||||
try:
|
||||
with urllib_request.urlopen(req, timeout=timeout_s) as resp: # noqa: S310 - trusted env var URL
|
||||
status = getattr(resp, "status", None) or resp.getcode()
|
||||
return (200 <= int(status) < 300, int(status), None)
|
||||
ok = 200 <= int(status) < 300
|
||||
# Cap the body read: we only need a small JSON health payload, and an
|
||||
# unbounded resp.read() on a large/trickled 2xx body could hang the
|
||||
# /api/health/agent handler or balloon memory. Read one byte over the
|
||||
# cap so the caller can detect (and skip) an oversized body.
|
||||
body = resp.read(_REMOTE_PROBE_BODY_LIMIT_BYTES + 1) if ok else None
|
||||
return (ok, int(status), None, body)
|
||||
except urllib_error.HTTPError as exc:
|
||||
return (False, int(exc.code), "HTTPError")
|
||||
return (False, int(exc.code), "HTTPError", None)
|
||||
except Exception as exc: # urllib_error.URLError, socket.timeout, ssl, etc.
|
||||
return (False, None, type(exc).__name__)
|
||||
return (False, None, type(exc).__name__, None)
|
||||
|
||||
|
||||
def _probe_remote_gateway(base_url: str, *, now: float | None = None) -> dict[str, Any]:
|
||||
|
|
@ -360,17 +385,28 @@ def _probe_remote_gateway(base_url: str, *, now: float | None = None) -> dict[st
|
|||
last_status: int | None = None
|
||||
last_error: str | None = None
|
||||
for path in _REMOTE_PROBE_PATHS:
|
||||
ok, status, err = _http_probe(base_url + path, _REMOTE_PROBE_TIMEOUT_S)
|
||||
ok, status, err, body = _http_probe(base_url + path, _REMOTE_PROBE_TIMEOUT_S)
|
||||
if ok:
|
||||
details: dict[str, Any] = {
|
||||
"state": "alive",
|
||||
"reason": "remote_gateway",
|
||||
"endpoint": base_url + path,
|
||||
"status_code": status,
|
||||
}
|
||||
if body and len(body) <= _REMOTE_PROBE_BODY_LIMIT_BYTES:
|
||||
try:
|
||||
data = json.loads(body)
|
||||
if isinstance(data, dict) and "gateway_state" in data:
|
||||
details["gateway_state"] = data["gateway_state"]
|
||||
except (json.JSONDecodeError, UnicodeDecodeError):
|
||||
pass
|
||||
# An over-cap body (len > limit, i.e. the +1 sentinel byte was read)
|
||||
# is treated as "alive but no parseable gateway_state" — we still
|
||||
# report the gateway as up, just without the detailed state.
|
||||
payload = {
|
||||
"alive": True,
|
||||
"checked_at": _checked_at(),
|
||||
"details": {
|
||||
"state": "alive",
|
||||
"reason": "remote_gateway",
|
||||
"endpoint": base_url + path,
|
||||
"status_code": status,
|
||||
},
|
||||
"details": details,
|
||||
}
|
||||
break
|
||||
# Remember the most informative failure signal we saw.
|
||||
|
|
|
|||
|
|
@ -1,6 +1,7 @@
|
|||
"""Tests for HERMES_API_URL remote gateway probe (#3281)."""
|
||||
"""Tests for HERMES_API_URL remote gateway probe (#3281, #3355)."""
|
||||
from __future__ import annotations
|
||||
|
||||
import json
|
||||
from unittest import mock
|
||||
|
||||
import pytest
|
||||
|
|
@ -16,14 +17,17 @@ def _clear_cache():
|
|||
|
||||
|
||||
class _FakeResp:
|
||||
def __init__(self, status: int = 200):
|
||||
def __init__(self, status: int = 200, body: bytes = b""):
|
||||
self.status = status
|
||||
self._body = body
|
||||
|
||||
def getcode(self) -> int:
|
||||
return self.status
|
||||
|
||||
def read(self) -> bytes:
|
||||
return b""
|
||||
def read(self, amt: int | None = None) -> bytes:
|
||||
if amt is None:
|
||||
return self._body
|
||||
return self._body[:amt]
|
||||
|
||||
def __enter__(self):
|
||||
return self
|
||||
|
|
@ -99,3 +103,121 @@ def test_remote_probe_result_cached_for_5s(monkeypatch):
|
|||
assert call_count["n"] == 1
|
||||
# checked_at is refreshed even on cache hit so the UI shows a current time.
|
||||
assert "checked_at" in second
|
||||
|
||||
|
||||
# ── #3355: gateway_state extraction and probe-order tests ─────────────────
|
||||
|
||||
|
||||
def test_gateway_state_populated_from_health_detailed(monkeypatch):
|
||||
"""Remote probe should extract gateway_state from /health/detailed JSON body."""
|
||||
monkeypatch.setenv("HERMES_API_URL", "http://fake-gateway:8642")
|
||||
body = json.dumps({"gateway_state": "running", "uptime": 3600}).encode()
|
||||
|
||||
def fake_urlopen(req, timeout=None):
|
||||
return _FakeResp(200, body=body)
|
||||
|
||||
with mock.patch.object(agent_health.urllib_request, "urlopen", fake_urlopen):
|
||||
payload = agent_health.build_agent_health_payload()
|
||||
|
||||
assert payload["alive"] is True
|
||||
assert payload["details"]["gateway_state"] == "running"
|
||||
assert payload["details"]["reason"] == "remote_gateway"
|
||||
|
||||
|
||||
def test_probe_order_prefers_health_detailed(monkeypatch):
|
||||
"""First probe path should be /health/detailed so gateway_state is available."""
|
||||
monkeypatch.setenv("HERMES_API_URL", "http://fake-gateway:8642")
|
||||
probed_urls: list[str] = []
|
||||
|
||||
def fake_urlopen(req, timeout=None):
|
||||
probed_urls.append(req.full_url)
|
||||
# Return 200 on first hit so the loop stops immediately
|
||||
return _FakeResp(200, body=b'{"gateway_state": "running"}')
|
||||
|
||||
with mock.patch.object(agent_health.urllib_request, "urlopen", fake_urlopen):
|
||||
agent_health.build_agent_health_payload()
|
||||
|
||||
assert len(probed_urls) == 1
|
||||
assert probed_urls[0] == "http://fake-gateway:8642/health/detailed"
|
||||
|
||||
|
||||
def test_gateway_health_url_env_used(monkeypatch):
|
||||
"""GATEWAY_HEALTH_URL should be respected by the remote probe."""
|
||||
monkeypatch.delenv("HERMES_API_URL", raising=False)
|
||||
monkeypatch.delenv("HERMES_GATEWAY_HEALTH_URL", raising=False)
|
||||
monkeypatch.setenv("GATEWAY_HEALTH_URL", "http://custom:9999")
|
||||
probed_urls: list[str] = []
|
||||
|
||||
def fake_urlopen(req, timeout=None):
|
||||
probed_urls.append(req.full_url)
|
||||
return _FakeResp(200, body=b'{}')
|
||||
|
||||
with mock.patch.object(agent_health.urllib_request, "urlopen", fake_urlopen):
|
||||
payload = agent_health.build_agent_health_payload()
|
||||
|
||||
assert payload["alive"] is True
|
||||
assert probed_urls[0].startswith("http://custom:9999/")
|
||||
|
||||
|
||||
def test_default_url_when_no_env(monkeypatch):
|
||||
"""Without any gateway env vars, should fall back to local detection (not remote)."""
|
||||
monkeypatch.delenv("HERMES_API_URL", raising=False)
|
||||
monkeypatch.delenv("GATEWAY_HEALTH_URL", raising=False)
|
||||
monkeypatch.delenv("HERMES_GATEWAY_HEALTH_URL", raising=False)
|
||||
|
||||
# Force the local importlib path to fail so we hit "gateway_status_unavailable",
|
||||
# proving the remote probe was NOT invoked.
|
||||
def boom(name):
|
||||
raise ModuleNotFoundError(name)
|
||||
|
||||
with mock.patch.object(agent_health.importlib, "import_module", boom):
|
||||
payload = agent_health.build_agent_health_payload()
|
||||
|
||||
assert payload["alive"] is None
|
||||
assert payload["details"]["reason"] == "gateway_status_unavailable"
|
||||
|
||||
|
||||
def test_gateway_health_url_with_health_suffix_is_normalized(monkeypatch):
|
||||
"""A gateway env var that already points AT a health path must not produce
|
||||
a doubled path like /health/health/detailed (#3355 Codex follow-up)."""
|
||||
monkeypatch.delenv("HERMES_API_URL", raising=False)
|
||||
monkeypatch.delenv("HERMES_GATEWAY_HEALTH_URL", raising=False)
|
||||
monkeypatch.setenv("GATEWAY_HEALTH_URL", "http://gw:8642/health")
|
||||
probed: list[str] = []
|
||||
|
||||
def fake_urlopen(req, timeout=None):
|
||||
probed.append(req.full_url)
|
||||
if req.full_url == "http://gw:8642/health/detailed":
|
||||
return _FakeResp(200, body=json.dumps({"gateway_state": "running"}).encode())
|
||||
return _FakeResp(404)
|
||||
|
||||
with mock.patch.object(agent_health.urllib_request, "urlopen", fake_urlopen):
|
||||
payload = agent_health.build_agent_health_payload()
|
||||
|
||||
assert all("/health/health" not in u for u in probed), probed
|
||||
assert "http://gw:8642/health/detailed" in probed
|
||||
assert payload["alive"] is True
|
||||
assert payload["details"].get("gateway_state") == "running"
|
||||
|
||||
|
||||
def test_oversized_remote_body_does_not_hang_and_skips_parse(monkeypatch):
|
||||
"""A 2xx response with a huge body must be capped (not read unbounded) and
|
||||
still report the gateway alive, just without a parsed gateway_state."""
|
||||
monkeypatch.setenv("HERMES_API_URL", "http://gateway:8080")
|
||||
huge = b"x" * (agent_health._REMOTE_PROBE_BODY_LIMIT_BYTES * 4)
|
||||
captured_amt: list[int | None] = []
|
||||
|
||||
class _HugeResp(_FakeResp):
|
||||
def read(self, amt: int | None = None):
|
||||
captured_amt.append(amt)
|
||||
return huge[:amt] if amt is not None else huge
|
||||
|
||||
def fake_urlopen(req, timeout=None):
|
||||
return _HugeResp(200, body=huge)
|
||||
|
||||
with mock.patch.object(agent_health.urllib_request, "urlopen", fake_urlopen):
|
||||
payload = agent_health.build_agent_health_payload()
|
||||
|
||||
assert captured_amt and all(a is not None for a in captured_amt)
|
||||
assert payload["alive"] is True
|
||||
assert "gateway_state" not in payload["details"]
|
||||
|
|
|
|||
Loading…
Reference in a new issue