From 439c2362453ce48779044715ed9207ffa629f4c1 Mon Sep 17 00:00:00 2001 From: Sanjay Santhanam <51058514+Sanjays2402@users.noreply.github.com> Date: Sun, 31 May 2026 23:25:49 -0700 Subject: [PATCH] fix: probe remote gateway via HERMES_API_URL before local fallback (#3281) --- api/agent_health.py | 134 ++++++++++++++++++++++++++++++ tests/test_agent_health_remote.py | 102 +++++++++++++++++++++++ 2 files changed, 236 insertions(+) create mode 100644 tests/test_agent_health_remote.py diff --git a/api/agent_health.py b/api/agent_health.py index 897827abbf9..ef68f101b0e 100644 --- a/api/agent_health.py +++ b/api/agent_health.py @@ -24,9 +24,14 @@ import importlib import json +import os +import threading +import time from datetime import datetime, timezone from pathlib import Path from typing import Any +from urllib import error as urllib_error +from urllib import request as urllib_request _GATEWAY_PID_FILE = "gateway.pid" _GATEWAY_RUNTIME_STATUS_FILE = "gateway_state.json" @@ -285,6 +290,125 @@ def _runtime_detail_subset(runtime_status: dict[str, Any] | None) -> dict[str, A return details +# Remote-gateway probe (#3281) +# ------------------------------------------------------------------ +# In multi-container Docker deployments the WebUI container does not ship the +# ``gateway`` Python package. The lazy ``importlib.import_module("gateway.status")`` +# therefore raises ``ModuleNotFoundError`` and the payload falls through to +# ``gateway_not_configured`` even though ``HERMES_API_URL`` points at a perfectly +# reachable remote gateway. The Tasks/Cron banner then shows a spurious amber +# "Gateway not configured" warning. +# +# When ``HERMES_API_URL`` is set we treat that as an explicit declaration that +# the gateway lives elsewhere, and probe it over HTTP before touching any local +# filesystem / module signal. The probe result is cached briefly so a dashboard +# rerender that fans out to multiple panels does not hammer the gateway. + +_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_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("/") + + +def _http_probe(url: str, timeout_s: float) -> tuple[bool, int | None, str | None]: + """GET ``url`` and return (ok, status_code, error_name). + + ``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. + """ + 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) + except urllib_error.HTTPError as exc: + return (False, int(exc.code), "HTTPError") + except Exception as exc: # urllib_error.URLError, socket.timeout, ssl, etc. + return (False, None, type(exc).__name__) + + +def _probe_remote_gateway(base_url: str, *, now: float | None = None) -> dict[str, Any]: + """Return an agent-health payload dict for a remote gateway base URL. + + Result is cached for ``_REMOTE_PROBE_CACHE_TTL_S`` seconds per base_url. + """ + current = time.monotonic() if now is None else now + with _remote_probe_lock: + if ( + _remote_probe_cache.get("url") == base_url + and _remote_probe_cache.get("expires_at", 0.0) > current + and _remote_probe_cache.get("result") is not None + ): + cached = _remote_probe_cache["result"] + # Refresh checked_at so the UI shows a current timestamp without + # actually re-hitting the gateway. + return {**cached, "checked_at": _checked_at()} + + 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) + if ok: + payload = { + "alive": True, + "checked_at": _checked_at(), + "details": { + "state": "alive", + "reason": "remote_gateway", + "endpoint": base_url + path, + "status_code": status, + }, + } + break + # Remember the most informative failure signal we saw. + if status is not None: + last_status = status + if err is not None: + last_error = err + else: + details: dict[str, Any] = { + "state": "down", + "reason": "remote_gateway_unreachable", + "endpoint": base_url, + } + if last_status is not None: + details["status_code"] = last_status + if last_error is not None: + details["error"] = last_error + payload = { + "alive": False, + "checked_at": _checked_at(), + "details": details, + } + + with _remote_probe_lock: + _remote_probe_cache["url"] = base_url + _remote_probe_cache["expires_at"] = current + _REMOTE_PROBE_CACHE_TTL_S + _remote_probe_cache["result"] = payload + return payload + + +def _reset_remote_probe_cache_for_tests() -> None: + """Test hook: clear the in-process remote-probe cache.""" + with _remote_probe_lock: + _remote_probe_cache["url"] = None + _remote_probe_cache["expires_at"] = 0.0 + _remote_probe_cache["result"] = None + + def build_agent_health_payload() -> dict[str, Any]: """Return `{alive, checked_at, details}` for the Hermes gateway/agent. @@ -295,6 +419,16 @@ def build_agent_health_payload() -> dict[str, Any]: probably not configured with a separate gateway process. """ checked_at = _checked_at() + + # Multi-container deployments (#3281): when HERMES_API_URL is set the + # gateway lives in another container/host. Probe it over HTTP before + # touching local module/pid/state-file signals, otherwise a missing + # ``gateway`` Python package in this image masquerades as + # "gateway_not_configured" and produces a spurious banner. + remote_base = _remote_gateway_base_url() + if remote_base is not None: + return _probe_remote_gateway(remote_base) + try: gateway_status = _gateway_status_module() except Exception as exc: diff --git a/tests/test_agent_health_remote.py b/tests/test_agent_health_remote.py new file mode 100644 index 00000000000..cd861c24900 --- /dev/null +++ b/tests/test_agent_health_remote.py @@ -0,0 +1,102 @@ +"""Tests for HERMES_API_URL remote gateway probe (#3281).""" +from __future__ import annotations + +import io +from unittest import mock + +import pytest + +from api import agent_health + + +@pytest.fixture(autouse=True) +def _clear_cache(): + agent_health._reset_remote_probe_cache_for_tests() + yield + agent_health._reset_remote_probe_cache_for_tests() + + +class _FakeResp: + def __init__(self, status: int = 200): + self.status = status + + def getcode(self) -> int: + return self.status + + def read(self) -> bytes: + return b"" + + def __enter__(self): + return self + + def __exit__(self, *_a): + return False + + +def test_remote_gateway_healthy_when_200(monkeypatch): + monkeypatch.setenv("HERMES_API_URL", "http://gateway:8080") + calls: list[str] = [] + + def fake_urlopen(req, timeout=None): + calls.append(req.full_url) + return _FakeResp(200) + + 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"]["reason"] == "remote_gateway" + assert payload["details"]["status_code"] == 200 + assert calls and calls[0].startswith("http://gateway:8080/") + + +def test_remote_gateway_unreachable_when_network_error(monkeypatch): + monkeypatch.setenv("HERMES_API_URL", "http://gateway:8080/") + + def fake_urlopen(req, timeout=None): + raise OSError("connection refused") + + with mock.patch.object(agent_health.urllib_request, "urlopen", fake_urlopen): + payload = agent_health.build_agent_health_payload() + + assert payload["alive"] is False + assert payload["details"]["reason"] == "remote_gateway_unreachable" + assert payload["details"]["endpoint"] == "http://gateway:8080" + assert "error" in payload["details"] + + +def test_falls_back_to_local_when_no_env(monkeypatch): + monkeypatch.delenv("HERMES_API_URL", raising=False) + + # Force the local importlib path to fail so we hit the well-known + # "gateway_not_configured" terminal state — proving the remote probe was + # NOT invoked and the legacy local path ran. + 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_remote_probe_result_cached_for_5s(monkeypatch): + monkeypatch.setenv("HERMES_API_URL", "http://gateway:8080") + call_count = {"n": 0} + + def fake_urlopen(req, timeout=None): + call_count["n"] += 1 + return _FakeResp(200) + + with mock.patch.object(agent_health.urllib_request, "urlopen", fake_urlopen): + first = agent_health.build_agent_health_payload() + second = agent_health.build_agent_health_payload() + + assert first["alive"] is True + assert second["alive"] is True + assert second["details"]["reason"] == "remote_gateway" + # Second call must NOT have hit the network. + 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