Skip to content

fix(gateway): retry restart notification on transient send-path failures - #68569

Closed
occam-7 wants to merge 3 commits into
NousResearch:mainfrom
occam-7:fix/restart-notify-retry
Closed

fix(gateway): retry restart notification on transient send-path failures#68569
occam-7 wants to merge 3 commits into
NousResearch:mainfrom
occam-7:fix/restart-notify-retry

Conversation

@occam-7

@occam-7 occam-7 commented Jul 21, 2026

Copy link
Copy Markdown

Problem

Two related issues cause gateway restart/startup notifications to silently fail on Telegram:

1. Notifications fire before adapter is ready

After gateway restart, _send_restart_notification() and _send_home_channel_startup_notifications() fire during startup, but the Telegram adapter may not be fully ready. The notification file is deleted on failure, permanently losing the message.

2. _send_path_degraded is set prematurely

_begin_polling_generation() sets _send_path_degraded = True proactively, blocking ALL sends through the live adapter until the first getUpdates completes. Since getUpdates is a long-poll with up to ~30s timeout, this blocks lifecycle notifications for up to 30 seconds. The flag should only reflect actual errors, not the "still initializing" state.

Fix

  1. Retry on transient failures: Both _send_restart_notification() and _send_home_channel_startup_notifications() now retry up to 5 times (1s delay) on transient errors like send_path_degraded.

  2. Remove proactive degraded flag: _begin_polling_generation() no longer sets _send_path_degraded = True. The flag is only set on actual network errors and cleared after successful getUpdates. The Bot API's sendMessage uses a separate connection from getUpdates, so sends are safe during polling startup.

_send_restart_notification() fires during gateway startup, but platform
adapters may still be settling after reconnect. The Telegram adapter's
_send_path_degraded flag can still be True from the disconnect/reconnect
cycle, causing the one-shot notification to fail silently. The
notification file is deleted in the finally block regardless of outcome,
so a failed notification is permanently lost.

Add a retry loop (5 attempts, 1s delay) for transient/retryable errors
like send_path_degraded, giving the adapter time to complete its first
successful polling cycle and clear the degraded flag. Non-retryable
errors still fail immediately.
@alt-glitch alt-glitch added type/bug Something isn't working P2 Medium — degraded but workaround exists comp/gateway Gateway runner, session dispatch, delivery platform/wecom WeCom / WeChat Work adapter sweeper:risk-message-delivery Sweeper risk: may drop, duplicate, misroute, or suppress messages labels Jul 21, 2026
…nd-path failures

Same root cause as _send_restart_notification: during gateway startup,
platform adapters may not be fully ready. The Telegram adapter's
_send_path_degraded flag can still be True, causing the home-channel
startup notification to fail silently.

Add the same retry loop (5 attempts, 1s delay) for transient errors
like send_path_degraded.
@occam-7
occam-7 force-pushed the fix/restart-notify-retry branch from 7c33ac8 to 0496c2f Compare July 21, 2026 12:16
_begin_polling_generation() sets _send_path_degraded=True proactively,
blocking all sends through the live adapter until the first getUpdates
completes. Since getUpdates is a long-poll with up to ~30s timeout,
this blocks lifecycle notifications (restart, startup) for up to 30s
after gateway boot.

The flag should only reflect actual errors. Remove the proactive
set — let real network errors set it, and _record_polling_progress
clear it when getUpdates succeeds.

@teknium1 teknium1 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for targeting a real restart-delivery gap. Current main still makes one attempt, returns on a false result (gateway/run.py:17892-17899), then unconditionally removes the marker (gateway/run.py:17911).

Problems

  • Removing the _send_path_degraded assignment conflicts with the current polling-health contract: each new generation is deliberately gated until a dedicated getUpdates response succeeds (plugins/platforms/telegram/adapter.py:782-787, 2076-2093), and send() refuses delivery while the path is degraded (4332-4333).
  • The proposed five retries still fall through to the existing unconditional marker deletion after exhaustion (gateway/run.py:17911), so a longer transient outage still loses the notification.
  • The PR changes no tests, despite existing integration coverage of the polling-progress invariant in tests/test_telegram_polling_progress_ptb.py:131-148 and 233-254.

Suggested changes

  • Preserve the Telegram safety gate and rework retry at the current DeliveryTransport boundary (gateway/run.py:17852, 17882-17887), including relay routing.
  • Specify and test marker ownership and consumption for retryable failure, success, exhaustion, and shutdown.

This is an automated hermes-sweeper review.

@@ -2043,7 +2043,14 @@ def _begin_polling_generation(self) -> tuple[int, asyncio.Event]:
self._polling_generation = getattr(self, "_polling_generation", 0) + 1

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do not remove this gate without replacing its safety proof: current main deliberately keeps every new polling generation degraded until its dedicated getUpdates request succeeds (plugins/platforms/telegram/adapter.py:782-787, 2076-2093), and send() refuses delivery while that flag is set (4332-4333).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp/gateway Gateway runner, session dispatch, delivery P2 Medium — degraded but workaround exists platform/wecom WeCom / WeChat Work adapter sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:risk-message-delivery Sweeper risk: may drop, duplicate, misroute, or suppress messages type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants