Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
65 changes: 57 additions & 8 deletions gateway/run.py
Original file line number Diff line number Diff line change
Expand Up @@ -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.

Expand Down Expand Up @@ -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,
Expand Down
115 changes: 112 additions & 3 deletions tests/gateway/test_update_command.py
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
Loading