From 52dbe0a7cd875ad9c87acd5c8dca09d806e212d7 Mon Sep 17 00:00:00 2001 From: yoyodine-industries <311904754+yoyodine-industries@users.noreply.github.com> Date: Sun, 13 Sep 2026 14:24:37 -0400 Subject: [PATCH] fix(gateway): restore served-profile liveness when gateway.pid is gone `live_default_gateway_pid()` (hermes_cli/gateway_multiplex_served.py) read only the pid record, so it returned None for a gateway that is alive but has no gateway.pid. Consumers of the helper then reported the gateway as down: - `hermes -p cron list` printed "Gateway is not running" with "jobs won't fire automatically" while the multiplexer was firing that profile's jobs - `hermes -p status` dropped its "running (via the default-profile multiplexer)" line - `named_profile_served_by_running_multiplexer()` returned False for a profile the live gateway serves The rest of the liveness surface already handles a missing pid file: the `runtime_pid_probe` seam of `resolve_gateway_liveness()` exists for "launch-service gateways with no live PID file" (hermes_cli/profiles.py, hermes_cli/web_routers/), and `hermes_cli/gateway_migrate._live_gateway_pid()` reads "pid file, then runtime status". This probe was the one call site that never got either. Read the pid record first, then the PID in `gateway_state.json` validated against the process table, matching `_live_gateway_pid()`. A record naming a dead pid still resolves to None, so a stopped gateway keeps reporting stopped and cron keeps warning. Related to #99631. --- hermes_cli/gateway_multiplex_served.py | 28 ++++++-- .../test_gateway_multiplex_served_record.py | 52 +++++++++++++++ .../test_gateway_multiplex_status.py | 64 +++++++++++++++++++ 3 files changed, 140 insertions(+), 4 deletions(-) diff --git a/hermes_cli/gateway_multiplex_served.py b/hermes_cli/gateway_multiplex_served.py index c2eadf6ff3..1514efce97 100644 --- a/hermes_cli/gateway_multiplex_served.py +++ b/hermes_cli/gateway_multiplex_served.py @@ -17,12 +17,32 @@ logger = logging.getLogger(__name__) def live_default_gateway_pid() -> Optional[int]: - """PID of the default profile's gateway when its pid record names a live process, else None.""" + """PID of the default profile's gateway when it names a live process, else None. + + ``gateway.pid`` first, then the runtime record the gateway process itself writes: a + launch-service-managed gateway can be live with no PID file at all (a replace/cleanup path unlinks + it while the process keeps serving), and ``get_running_pid()`` cannot answer for this scoped home -- + an explicit ``pid_path`` deliberately suppresses its own runtime-status fallback. Same order and + same call as ``hermes_cli.gateway_migrate._live_gateway_pid``. Never key this off the record's + ``updated_at``: an idle gateway never advances it, so "recent" would read a live-but-quiet + multiplexer as stopped. + """ from hermes_constants import get_default_hermes_root - from gateway.status import _pid_exists, _pid_from_record, _read_pid_record - rec = _read_pid_record(get_default_hermes_root() / "gateway.pid") + from gateway.status import ( + _pid_exists, + _pid_from_record, + _read_pid_record, + get_runtime_status_running_pid, + read_runtime_status, + ) + default_root = get_default_hermes_root() + rec = _read_pid_record(default_root / "gateway.pid") pid = _pid_from_record(rec) if rec else None - return pid if pid and _pid_exists(pid) else None + if pid and _pid_exists(pid): + return pid + return get_runtime_status_running_pid( + read_runtime_status(default_root / "gateway_state.json"), expected_home=default_root + ) def recorded_served_profiles(default_root: Optional[Path] = None) -> Optional[list[str]]: diff --git a/tests/hermes_cli/test_gateway_multiplex_served_record.py b/tests/hermes_cli/test_gateway_multiplex_served_record.py index a0867c8f81..efd9705bce 100644 --- a/tests/hermes_cli/test_gateway_multiplex_served_record.py +++ b/tests/hermes_cli/test_gateway_multiplex_served_record.py @@ -52,6 +52,58 @@ def test_probe_falls_back_to_config_only_without_recorded_key(served_root): assert named_profile_served_by_running_multiplexer("coder") is True +def test_probe_survives_a_missing_default_pid_file(served_root, monkeypatch): + """A launch-service-managed multiplexer can be live with no ``gateway.pid``: a replace/cleanup path + unlinks it while the process keeps serving. Keying liveness off that file alone made every surface + (``hermes -p X status``, ``cron list``, the dashboard ladder) say "not running" about the gateway + that was in fact serving the profile.""" + import gateway.status as status + from hermes_cli.gateway import named_profile_served_by_running_multiplexer + from hermes_cli.gateway_multiplex_served import live_default_gateway_pid + (served_root / "gateway_state.json").write_text(json.dumps({ + "pid": os.getpid(), "hermes_home": str(served_root), "gateway_state": "running", + "served_profiles": ["default", "coder"]})) + (served_root / "gateway.pid").unlink() + # The PID is this test process, so the record's identity check has to see a gateway command line: + # without it the fallback correctly refuses (see the recycled-PID test below). + monkeypatch.setattr( + status, "_read_process_cmdline", lambda pid: "python -m hermes_cli.main gateway run --replace" + ) + assert live_default_gateway_pid() == os.getpid() + assert named_profile_served_by_running_multiplexer("coder") is True + + +@pytest.mark.parametrize( + ("gateway_state", "pid_alive"), [("running", False), ("stopped", True), ("startup_failed", True)] +) +def test_missing_pid_file_still_never_reports_a_dead_gateway( + served_root, monkeypatch, gateway_state, pid_alive +): + """Fail closed: the runtime fallback must not resurrect a dead PID or a stopped/failed record.""" + import gateway.status as status + from hermes_cli.gateway_multiplex_served import live_default_gateway_pid + (served_root / "gateway_state.json").write_text(json.dumps({ + "pid": os.getpid(), "hermes_home": str(served_root), "gateway_state": gateway_state, + "served_profiles": ["default", "coder"]})) + (served_root / "gateway.pid").unlink() + if not pid_alive: + monkeypatch.setattr(status, "_pid_exists", lambda pid: False) + assert live_default_gateway_pid() is None + + +def test_missing_pid_file_ignores_a_recycled_pid(served_root, monkeypatch): + """A PID recycled onto a non-gateway process must not lend a stale record an identity: the live + command line decides, so the fallback cannot report a foreign process as the multiplexer.""" + import gateway.status as status + from hermes_cli.gateway_multiplex_served import live_default_gateway_pid + (served_root / "gateway_state.json").write_text(json.dumps({ + "pid": os.getpid(), "hermes_home": str(served_root), "gateway_state": "running", + "served_profiles": ["default", "coder"]})) + (served_root / "gateway.pid").unlink() + monkeypatch.setattr(status, "_read_process_cmdline", lambda pid: "/usr/bin/pytest tests/") + assert live_default_gateway_pid() is None + + @pytest.mark.parametrize("verb", ["start", "install", "restart"]) def test_service_verbs_refuse_served_profile_with_exit_78(served_root, monkeypatch, verb): import hermes_cli.gateway as gw diff --git a/tests/hermes_cli/test_gateway_multiplex_status.py b/tests/hermes_cli/test_gateway_multiplex_status.py index 0c516db590..9000d32447 100644 --- a/tests/hermes_cli/test_gateway_multiplex_status.py +++ b/tests/hermes_cli/test_gateway_multiplex_status.py @@ -15,6 +15,8 @@ import os from contextlib import redirect_stdout from types import SimpleNamespace +import pytest + def _fake_multiplexer(monkeypatch, tmp_path, *, multiplex: bool): import hermes_constants @@ -59,3 +61,65 @@ def test_unserved_named_profile_still_reports_stopped(monkeypatch, tmp_path): beta = next(p for p in list_profiles() if p.name == "beta") assert beta.gateway_running is False assert _run_status().startswith("✗ Gateway is not running") + + +def _fake_launchd_multiplexer( + monkeypatch, tmp_path, *, multiplex: bool = True, gateway_state: str = "running", pid_alive: bool = True +): + """A launch-service-managed default gateway: live process + runtime status record, no gateway.pid. + + The PID file is absent (a replace/cleanup path unlinks it while the process keeps serving); the + process is the live multiplexer the ``gateway_state.json`` record points at. + """ + import json + + import hermes_constants + import gateway.status as status + + (tmp_path / "profiles" / "beta").mkdir(parents=True) + (tmp_path / "config.yaml").write_text( + f"gateway:\n multiplex_profiles: {'true' if multiplex else 'false'}\n" + ) + (tmp_path / "gateway_state.json").write_text(json.dumps({ + "pid": os.getpid(), + "kind": "hermes-gateway", + "gateway_state": gateway_state, + # Same call the production PID-reuse guard makes, so the guard compares like with like. + "start_time": status._get_process_start_time(os.getpid()), + "argv": ["hermes", "gateway", "run", "--replace"], + "hermes_home": str(tmp_path), + })) + monkeypatch.setenv("HERMES_HOME", str(tmp_path / "profiles" / "beta")) + monkeypatch.setattr(hermes_constants, "_default_hermes_root_memo", None) + monkeypatch.setattr(status, "_pid_exists", lambda pid: pid_alive) + monkeypatch.setattr( + status, "_read_process_cmdline", lambda pid: "python -m hermes_cli.main gateway run --replace" + ) + + +def test_served_named_profile_reports_running_without_default_pid_file(monkeypatch, tmp_path): + """A live multiplexer whose PID file is missing still serves the profile it ticks.""" + from hermes_cli.profiles import list_profiles + + _fake_launchd_multiplexer(monkeypatch, tmp_path) + + beta = next(p for p in list_profiles() if p.name == "beta") + assert beta.gateway_running is True + assert _run_status().startswith("✓ Gateway is running via the default-profile multiplexer") + + +@pytest.mark.parametrize( + ("gateway_state", "pid_alive"), + [("stopped", True), ("startup_failed", True), ("running", False)], +) +def test_not_live_multiplexer_without_default_pid_file_reports_stopped( + monkeypatch, tmp_path, gateway_state, pid_alive +): + """Fails closed: a stopped/failed state or a dead PID must never be reported as running.""" + from hermes_cli.profiles import list_profiles + + _fake_launchd_multiplexer(monkeypatch, tmp_path, gateway_state=gateway_state, pid_alive=pid_alive) + + beta = next(p for p in list_profiles() if p.name == "beta") + assert beta.gateway_running is False + assert _run_status().startswith("✗ Gateway is not running")