From 6e1f328d133d13560200e52642c4245269012f7f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jo=C3=A3o=20Vitor=20Cunha?= Date: Wed, 22 Jul 2026 20:15:30 -0300 Subject: [PATCH 1/3] fix(gateway): wait for adapter readiness before startup notification + cold-start notify + PID liveness check Three fixes for the gateway startup notification cascade: 1. Wait for Telegram _send_path_degraded to clear before sending home-channel startup notifications (up to 15s). The old fixed 1.0s sleep raced the first getUpdates cycle on networks where Telegram needs IPv6 fallback -> DoH -> sticky IPv4 (>1s). Fixes the race documented in #66589. 2. Send home-channel startup notification on ALL starts (cold start, crash recovery, post-update restart), not only planned restarts. Chat-originated /restart still suppresses the duplicate (its reply target already covers that lifecycle). Fixes the design gap in #62512. 3. Add PID liveness check to read_runtime_status() so dashboards and /api/status report 'stopped' instead of 'running' when the gateway PID is dead but the state file is stale. Fixes the false-positive health check that masked #56524. --- gateway/run.py | 37 +++++++++++++++++++++++++++++-------- gateway/status.py | 30 +++++++++++++++++++++++++++++- 2 files changed, 58 insertions(+), 9 deletions(-) diff --git a/gateway/run.py b/gateway/run.py index 83342c39591b..43626c44253d 100644 --- a/gateway/run.py +++ b/gateway/run.py @@ -11523,8 +11523,23 @@ async def start(self) -> bool: # Give freshly connected platform adapters a brief moment to settle # before sending restart/startup lifecycle messages. In practice this # helps Discord thread deliveries right after reconnect. + # + # Telegram adapters gate their send path (_send_path_degraded) until + # the first getUpdates cycle succeeds after polling starts. On networks + # where that first cycle takes >1s (IPv6 fallback → DoH → sticky IPv4), + # a fixed 1.0s sleep races the degraded-state clear. Wait up to 15s for + # every Telegram adapter to exit degraded mode before proceeding, then + # allow a brief settle window for non-Telegram adapters (#66589). if connected_count > 0: - await asyncio.sleep(1.0) + _telegram_degraded_deadline = asyncio.get_running_loop().time() + 15.0 + for _adapter in self.adapters.values(): + if not hasattr(_adapter, "_send_path_degraded"): + continue + while asyncio.get_running_loop().time() < _telegram_degraded_deadline: + if not getattr(_adapter, "_send_path_degraded", False): + break + await asyncio.sleep(0.5) + await asyncio.sleep(0.5) # brief settle for non-Telegram adapters # Notify the chat that initiated /restart that the gateway is back. chat_restart_notification_pending = _restart_notification_pending() @@ -11539,17 +11554,23 @@ async def start(self) -> bool: await self._send_restart_notification() # Broadcast a lightweight "gateway is back" message to configured home - # channels only for non-chat planned restarts (terminal/SIGUSR1/service - # paths). Chat-originated /restart already has a precise reply target - # in .restart_notify.json, so keep that lifecycle in the originating - # chat/topic instead of also leaking it to the configured home channel. - if planned_restart_notification_pending: + # channels. Previously this was gated on planned restarts only + # (terminal/SIGUSR1/service paths), which meant crash recovery, cold + # starts, and post-update restarts never notified. Chat-originated + # /restart already has a precise reply target in .restart_notify.json, + # so keep THAT lifecycle in the originating chat/topic instead of also + # leaking it to the configured home channel (#62512). + if not _restart_notification_pending(): try: await self._send_home_channel_startup_notifications( skip_targets=None, ) - finally: - _clear_planned_restart_notification() + except Exception: + logger.warning( + "Home-channel startup notification failed", exc_info=True, + ) + if planned_restart_notification_pending: + _clear_planned_restart_notification() # Automatically continue fresh sessions that were interrupted by the # previous gateway restart/shutdown. The resume_pending flag is cleared diff --git a/gateway/status.py b/gateway/status.py index ce02648a958f..07392532f74b 100644 --- a/gateway/status.py +++ b/gateway/status.py @@ -1034,6 +1034,15 @@ def write_runtime_status( pass +def _pid_is_alive(pid: int) -> bool: + """Return True if a process with the given PID exists.""" + try: + os.kill(pid, 0) + except (OSError, ProcessLookupError): + return False + return True + + def read_runtime_status(path: Optional[Path] = None) -> Optional[dict[str, Any]]: """Read the persisted gateway runtime health/status information. @@ -1041,8 +1050,27 @@ def read_runtime_status(path: Optional[Path] = None) -> Optional[dict[str, Any]] profile's state file (e.g. the dashboard enumerating every profile) can do so without mutating ``HERMES_HOME`` in-process. Defaults to the active profile's ``gateway_state.json``. + + When the persisted state claims the gateway is running but the + corresponding PID is dead, the state is amended to reflect the real + situation so dashboards, desktop, and health probes don't report a + false-positive green light (#62512). """ - return _read_json_file(path or _get_runtime_status_path()) + record = _read_json_file(path or _get_runtime_status_path()) + if not isinstance(record, dict): + return record + + gw_state = record.get("gateway_state") + if gw_state not in ("running", "starting", "draining"): + return record + + pid = record.get("pid") + if pid is not None and not _pid_is_alive(pid): + record = dict(record) + record["gateway_state"] = "stopped" + record["exit_reason"] = record.get("exit_reason") or "pid_not_found" + + return record # Max age of a persisted ``gateway_state.json`` snapshot before its liveness From 80b0e72da05827663b11e0bc7c0444ed0aaae45e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jo=C3=A3o=20Vitor=20Cunha?= Date: Wed, 22 Jul 2026 20:41:10 -0300 Subject: [PATCH 2/3] fix(gateway): use existing _pid_exists instead of custom _pid_is_alive The _pid_exists helper already existed at gateway/status.py:733 with proper cross-platform handling (Windows-safe via ctypes, not os.kill). Reuse it instead of adding a redundant function that duplicates the same logic and breaks existing test mocks that patch _pid_exists. --- gateway/status.py | 11 +---------- 1 file changed, 1 insertion(+), 10 deletions(-) diff --git a/gateway/status.py b/gateway/status.py index 07392532f74b..992331ae0f44 100644 --- a/gateway/status.py +++ b/gateway/status.py @@ -1034,15 +1034,6 @@ def write_runtime_status( pass -def _pid_is_alive(pid: int) -> bool: - """Return True if a process with the given PID exists.""" - try: - os.kill(pid, 0) - except (OSError, ProcessLookupError): - return False - return True - - def read_runtime_status(path: Optional[Path] = None) -> Optional[dict[str, Any]]: """Read the persisted gateway runtime health/status information. @@ -1065,7 +1056,7 @@ def read_runtime_status(path: Optional[Path] = None) -> Optional[dict[str, Any]] return record pid = record.get("pid") - if pid is not None and not _pid_is_alive(pid): + if pid is not None and not _pid_exists(pid): record = dict(record) record["gateway_state"] = "stopped" record["exit_reason"] = record.get("exit_reason") or "pid_not_found" From de2cf150fef695d51e8454bb5b52f554530479f2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jo=C3=A3o=20Vitor=20Cunha?= Date: Thu, 23 Jul 2026 13:44:51 -0300 Subject: [PATCH 3/3] fix(gateway): suppress duplicate home notice after chat restart --- gateway/run.py | 6 +- tests/gateway/test_runner_startup_failures.py | 130 ++++++++++++++++++ 2 files changed, 135 insertions(+), 1 deletion(-) diff --git a/gateway/run.py b/gateway/run.py index 43626c44253d..9ac24e9fba26 100644 --- a/gateway/run.py +++ b/gateway/run.py @@ -11560,7 +11560,11 @@ async def start(self) -> bool: # /restart already has a precise reply target in .restart_notify.json, # so keep THAT lifecycle in the originating chat/topic instead of also # leaking it to the configured home channel (#62512). - if not _restart_notification_pending(): + # _send_restart_notification() always consumes the marker in its + # finally block, so checking the file again here would always report + # False and incorrectly broadcast a second lifecycle notification. + # Use the value captured before the marker was consumed. + if not chat_restart_notification_pending: try: await self._send_home_channel_startup_notifications( skip_targets=None, diff --git a/tests/gateway/test_runner_startup_failures.py b/tests/gateway/test_runner_startup_failures.py index 3655f756cbcb..c15fc3599a05 100644 --- a/tests/gateway/test_runner_startup_failures.py +++ b/tests/gateway/test_runner_startup_failures.py @@ -1,6 +1,7 @@ import pytest from unittest.mock import AsyncMock +import gateway.run as gateway_run from gateway.config import GatewayConfig, Platform, PlatformConfig from gateway.platforms.base import BasePlatformAdapter from gateway.restart import GATEWAY_FATAL_CONFIG_EXIT_CODE @@ -64,6 +65,135 @@ async def get_chat_info(self, chat_id): return {"id": chat_id} +@pytest.mark.asyncio +async def test_runner_stays_alive_for_retryable_startup_errors(monkeypatch, tmp_path): + """Retryable startup errors should leave the gateway running in + degraded mode so the reconnect watcher can recover the platform when + the underlying problem clears. Previously this returned False from + ``start()`` and exited the process, which converted a single broken + platform (e.g. unpaired WhatsApp, DNS blip on Telegram) into a + systemd restart loop and killed cron jobs in the meantime. + """ + monkeypatch.setenv("HERMES_HOME", str(tmp_path)) + config = GatewayConfig( + platforms={ + Platform.TELEGRAM: PlatformConfig(enabled=True, token="***") + }, + sessions_dir=tmp_path / "sessions", + ) + runner = GatewayRunner(config) + + monkeypatch.setattr(runner, "_create_adapter", lambda platform, platform_config: _RetryableFailureAdapter()) + + ok = await runner.start() + + # Gateway stays alive in degraded mode; reconnect watcher takes over. + assert ok is True + assert runner.should_exit_cleanly is False + state = read_runtime_status() + assert state["gateway_state"] in {"degraded", "running"} + # Telegram was queued for retry, not given up on. + assert Platform.TELEGRAM in runner._failed_platforms + assert state["platforms"]["telegram"]["state"] == "retrying" + assert state["platforms"]["telegram"]["error_code"] == "telegram_connect_error" + + +@pytest.mark.asyncio +async def test_runner_allows_cron_only_mode_when_no_platforms_are_enabled(monkeypatch, tmp_path): + monkeypatch.setenv("HERMES_HOME", str(tmp_path)) + config = GatewayConfig( + platforms={ + Platform.TELEGRAM: PlatformConfig(enabled=False, token="***") + }, + sessions_dir=tmp_path / "sessions", + ) + runner = GatewayRunner(config) + + ok = await runner.start() + + assert ok is True + assert runner.should_exit_cleanly is False + assert runner.adapters == {} + state = read_runtime_status() + assert state["gateway_state"] == "running" + + +@pytest.mark.asyncio +async def test_runner_records_connected_platform_state_on_success(monkeypatch, tmp_path): + monkeypatch.setenv("HERMES_HOME", str(tmp_path)) + config = GatewayConfig( + platforms={ + Platform.DISCORD: PlatformConfig(enabled=True, token="***") + }, + sessions_dir=tmp_path / "sessions", + ) + runner = GatewayRunner(config) + + monkeypatch.setattr(runner, "_create_adapter", lambda platform, platform_config: _SuccessfulAdapter()) + monkeypatch.setattr(runner.hooks, "discover_and_load", lambda: None) + monkeypatch.setattr(runner.hooks, "emit", AsyncMock()) + + ok = await runner.start() + + assert ok is True + state = read_runtime_status() + assert state["gateway_state"] == "running" + assert state["platforms"]["discord"]["state"] == "connected" + assert state["platforms"]["discord"]["error_code"] is None + assert state["platforms"]["discord"]["error_message"] is None + + +@pytest.mark.asyncio +async def test_chat_restart_does_not_also_broadcast_home_startup( + monkeypatch, tmp_path +): + """The restart marker is consumed before the home-broadcast decision. + + The decision must therefore use the pre-send snapshot; re-reading the + marker after ``_send_restart_notification`` would always look like a cold + start and send a duplicate lifecycle notification. + """ + monkeypatch.setenv("HERMES_HOME", str(tmp_path)) + monkeypatch.setattr(gateway_run, "_hermes_home", tmp_path) + marker = tmp_path / ".restart_notify.json" + marker.write_text("{}") + + config = GatewayConfig( + platforms={ + Platform.DISCORD: PlatformConfig(enabled=True, token="***") + }, + sessions_dir=tmp_path / "sessions", + ) + runner = GatewayRunner(config) + monkeypatch.setattr( + runner, + "_create_adapter", + lambda platform, platform_config: _SuccessfulAdapter(), + ) + monkeypatch.setattr(runner.hooks, "discover_and_load", lambda: None) + monkeypatch.setattr(runner.hooks, "emit", AsyncMock()) + + async def _consume_restart_marker(): + marker.unlink() + return ("discord", "restart-chat", None) + + monkeypatch.setattr( + runner, + "_send_restart_notification", + AsyncMock(side_effect=_consume_restart_marker), + ) + home_notification = AsyncMock(return_value=set()) + monkeypatch.setattr( + runner, + "_send_home_channel_startup_notifications", + home_notification, + ) + + assert await runner.start() is True + assert marker.exists() is False + home_notification.assert_not_awaited() + + @pytest.mark.asyncio async def test_start_gateway_verbosity_imports_redacting_formatter(monkeypatch, tmp_path): """Verbosity != None must not crash with NameError on RedactingFormatter (#8044)."""