diff --git a/gateway/run.py b/gateway/run.py index 855f28766784..09904a81f195 100644 --- a/gateway/run.py +++ b/gateway/run.py @@ -23351,6 +23351,32 @@ async def _flush_buffer() -> None: if _up_timeout_state is not None: _up_timeout_state.persistent.update_prompt_pending = False + def _update_target_platform_is_enabled(self, platform) -> bool: + """Whether ``platform`` can ever produce an adapter for this profile. + + ``config.platforms`` is pre-seeded with disabled placeholders for the + whole platform catalog, so mere key presence proves nothing — the + ``enabled`` flag is the load-bearing part (same trap documented on + ``_scale_to_zero_active_messaging_platforms``). + + Fails OPEN: any missing/failed config lookup returns True so an + unexpected shape keeps the existing retry behaviour and can never + discard a deliverable notification. Only a positive, readable + ``enabled=False`` is treated as "no adapter will ever connect". + """ + config = getattr(self, "config", None) + if config is None: + return True + try: + platforms = config.platforms + if platform not in platforms: + # Not in the catalog at all: unknown rather than provably + # disabled. Defer instead of discarding. + return True + return bool(getattr(platforms[platform], "enabled", True)) + except Exception: # noqa: BLE001 + return True + async def _send_update_notification(self) -> bool: """If an update finished, notify the user. @@ -23408,14 +23434,37 @@ async def _send_update_notification(self) -> bool: adapter = self.adapters.get(platform) if not adapter and chat_id: - # The update finished, but the target platform has not - # reconnected yet (common right after the restart that - # `hermes update` triggers). Treating "adapter missing" as a - # definitive skip would delete the markers and silently lose the - # completion notification — the user never learns whether the - # update succeeded or timed out. Preserve the markers instead so - # a later retry (the watcher poll loop, or the next gateway - # startup) can deliver the result once the adapter is back. + # The update finished, but the target platform has no adapter. + # Two very different cases hide behind that one condition. + # + # 1. The platform IS enabled for this profile and is simply + # still reconnecting (common right after the restart that + # `hermes update` triggers). Treating "adapter missing" as a + # definitive skip would delete the markers and silently lose + # the completion notification — the user never learns whether + # the update succeeded or timed out. Preserve the markers so + # a later retry (the watcher poll loop, or the next gateway + # startup) delivers the result once the adapter is back. + # + # 2. The platform is NOT enabled for this profile at all. No + # adapter will ever appear, so preserving the markers retries + # forever: every poll re-reads the marker, logs, and rewrites + # it. A stale marker naming a platform this profile does not + # run (e.g. left behind by an earlier config, another + # profile, or an interrupted update) therefore pins a + # permanent 0.5 lines/sec log loop for the life of the + # gateway. That is not a recoverable target — consume the + # markers and record why. + if not self._update_target_platform_is_enabled(platform): + logger.warning( + "Update notification discarded: %s is not enabled for " + "this profile, so no adapter will ever connect " + "(stale marker for chat %s)", + platform_str, + chat_id, + ) + return True + logger.info( "Update notification deferred: %s adapter not connected yet", platform_str, diff --git a/tests/gateway/test_update_command.py b/tests/gateway/test_update_command.py index 22cc9cd419b5..21edab240380 100644 --- a/tests/gateway/test_update_command.py +++ b/tests/gateway/test_update_command.py @@ -492,10 +492,119 @@ async def test_completion_notification_tolerates_invalid_utf8_output(self, tmp_p assert not output_path.exists() assert not exit_code_path.exists() + @pytest.mark.asyncio + async def test_disabled_platform_marker_is_discarded_not_retried(self, tmp_path): + """A marker naming a platform this profile does not run is consumed. + + Regression for the permanent retry loop: ``config.platforms`` is + pre-seeded with disabled placeholders for the whole catalog, so a stale + marker naming a platform with ``enabled=False`` produced an adapter + that could never appear. The deferral path then rewrote and re-read the + marker on every poll — forever — pinning a steady log loop for the life + of the gateway. Such a target is unreachable by construction, so the + markers must be consumed instead of preserved. + """ + runner = _make_runner() + hermes_home = tmp_path / "hermes" + hermes_home.mkdir() -# --------------------------------------------------------------------------- -# /update in help and known_commands -# --------------------------------------------------------------------------- + pending = {"platform": "telegram", "chat_id": "111", "user_id": "222"} + pending_path = hermes_home / ".update_pending.json" + output_path = hermes_home / ".update_output.txt" + exit_code_path = hermes_home / ".update_exit_code" + pending_path.write_text(json.dumps(pending)) + output_path.write_text("Done") + exit_code_path.write_text("0") + + # This profile runs Slack only; telegram is present but disabled. + runner.config = MagicMock() + runner.config.platforms = { + Platform.SLACK: MagicMock(enabled=True), + Platform.TELEGRAM: MagicMock(enabled=False), + } + slack_adapter = AsyncMock() + runner.adapters = {Platform.SLACK: slack_adapter} + + with patch("gateway.run._hermes_home", hermes_home): + result = await runner._send_update_notification() + + # Definitive decision: no retry is scheduled. + assert result is True + # Nothing is misdelivered to the wrong platform. + slack_adapter.send.assert_not_called() + # Every marker is consumed, so the next poll finds nothing to do. + assert not pending_path.exists() + assert not output_path.exists() + assert not exit_code_path.exists() + assert not (hermes_home / ".update_pending.claimed.json").exists() + + @pytest.mark.asyncio + async def test_enabled_platform_still_defers_while_reconnecting(self, tmp_path): + """An enabled-but-reconnecting platform keeps the existing deferral. + + Guards the discard path from over-reaching: when the platform IS + configured for this profile the adapter is merely late (the normal case + right after the restart ``hermes update`` triggers), so the markers must + still survive for a later retry. + """ + runner = _make_runner() + hermes_home = tmp_path / "hermes" + hermes_home.mkdir() + + pending = {"platform": "discord", "chat_id": "111", "user_id": "222"} + pending_path = hermes_home / ".update_pending.json" + output_path = hermes_home / ".update_output.txt" + exit_code_path = hermes_home / ".update_exit_code" + pending_path.write_text(json.dumps(pending)) + output_path.write_text("Done") + exit_code_path.write_text("0") + + # Discord IS enabled here — it just has not reconnected yet. + runner.config = MagicMock() + runner.config.platforms = {Platform.DISCORD: MagicMock(enabled=True)} + runner.adapters = {} + + with patch("gateway.run._hermes_home", hermes_home): + result = await runner._send_update_notification() + + assert result is False + assert pending_path.exists() + assert output_path.exists() + assert exit_code_path.exists() + assert not (hermes_home / ".update_pending.claimed.json").exists() + + @pytest.mark.asyncio + async def test_unreadable_platform_config_fails_open_to_deferral(self, tmp_path): + """An unusable config must not cost the user their notification. + + The enabled-check exists to stop an unreachable retry, never to invent + a new way to drop a deliverable result. If the config cannot be read, + the safe answer is the pre-existing deferral. + """ + runner = _make_runner() + hermes_home = tmp_path / "hermes" + hermes_home.mkdir() + + pending = {"platform": "discord", "chat_id": "111", "user_id": "222"} + pending_path = hermes_home / ".update_pending.json" + exit_code_path = hermes_home / ".update_exit_code" + pending_path.write_text(json.dumps(pending)) + (hermes_home / ".update_output.txt").write_text("Done") + exit_code_path.write_text("0") + + broken_config = MagicMock() + type(broken_config).platforms = property( + lambda self: (_ for _ in ()).throw(RuntimeError("config unavailable")) + ) + runner.config = broken_config + runner.adapters = {} + + with patch("gateway.run._hermes_home", hermes_home): + result = await runner._send_update_notification() + + assert result is False + assert pending_path.exists() + assert exit_code_path.exists() class TestUpdateInHelp: