fix(gateway): preserve pending /restart confirmation until the target platform reconnects - #36148
Open
Frowtek wants to merge 1 commit into
Open
fix(gateway): preserve pending /restart confirmation until the target platform reconnects#36148Frowtek wants to merge 1 commit into
Frowtek wants to merge 1 commit into
Conversation
… platform reconnects When /restart fires, the requester's routing info is persisted to .restart_notify.json and delivered once at startup. If the requester's platform adapter hadn't reconnected yet, the startup attempt returned early but the finally block still deleted the marker, and the reconnect watcher never retried -- so the "Gateway restarted successfully" confirmation promised in the slash-command docs was permanently lost. Preserve the marker when the platform is still queued for reconnect (_failed_platforms), and retry _send_restart_notification() on the reconnect success path. The marker is consumed only after a successful or terminal delivery, so no duplicate notifications are sent. Platforms that will never reconnect still consume the marker as before, so no stale marker leaks into a later restart. Adds regression tests for the deferred-then-delivered flow and the reconnect-watcher retry wiring.
alt-glitch
added
type/bug
mxnstrexgl
approved these changes
Jun 1, 2026
mxnstrexgl
left a comment
mxnstrexgl
left a comment
There was a problem hiding this comment.
LGTM — automated review passed. No security, quality, or test coverage issues detected.
tonydwb
approved these changes
Jun 1, 2026
tonydwb
left a comment
tonydwb
left a comment
There was a problem hiding this comment.
Code Review Summary
Verdict: Approved ✅
Review
fix(gateway): preserve pending /restart confirmation until the target platform reconnects
Well-diagnosed timing fix. Key observations:
- Problem: During gateway restart, if the notification target platform hadn't reconnected yet,
_send_restart_notification()returned early and deleted the.restart_notify.jsonmarker — the reconnect watcher never retried, so the "Gateway restarted" message was permanently lost. - Fix: Preserve the marker while the platform is still in
_failed_platforms(queued for reconnect). Retry delivery on the reconnect success path. The marker is consumed only after successful/terminal send. - Scope: 2 files, well-targeted changes.
- Tests: 89-line regression test suite for the deferred→delivered flow and reconnect-retry wiring.
Looks Good
- Well-documented root cause
- Conservative fix (no stale marker leak)
- Good test coverage
Reviewed by Hermes Agent
Contributor
|
Thanks for the focused regression fix. The premise remains true on current The proposed conditional preservation is appropriately limited to platforms queued in This is an automated hermes-sweeper review. |
teknium1
added
sweeper:risk-message-delivery
oppih
added a commit
to oppih/hermes-agent
that referenced
this pull request
Jul 16, 2026
…art notification
## What does this PR do?
Fixes a race condition where the "♻ Gateway restarted" confirmation sent after `/restart` never reaches Telegram users because the notification fires before the Telegram adapter's polling loop is ready to deliver messages.
**Root cause chain:**
1. `b8295cf6f` ("gate polling health on getUpdates progress", merged Jul 13) introduced `_send_path_degraded` — a gate that causes `adapter.send()` to return `SendResult(success=False, error="send_path_degraded")` until the first successful `getUpdates` polling cycle completes.
2. `_send_restart_notification()` calls `adapter.send()` during gateway startup to notify the chat that issued `/restart`. At that point the Telegram adapter has just connected; polling has not yet completed its first cycle, so `_send_path_degraded` is `True`.
3. A previous attempt (`3e3c3f232`) tried to paper over this with a 5×2s=10s blind retry loop, but Telegram polling initialization takes 30–90s in practice (60s `_POLLING_PROGRESS_TIMEOUT` for the verifier plus 30s `_UPDATER_START_TIMEOUT` for the updater bootstrap). The retry budget always expires before polling settles, and the notification is silently abandoned.
**Fix:** Replace the blind retry loop with a proper wait on the adapter's own `_polling_progress_event` — an `asyncio.Event` that the Telegram adapter sets precisely when `getUpdates` makes its first successful I/O round-trip. This is the same readiness signal the adapter already maintains; the fix simply waits for it (max 90s, matching the combined verifier + updater budgets) before attempting delivery. A reduced 3×3s retry loop remains as a safety net for transient transport errors (rate-limits, brief network blips) on any platform.
The fix is **platform-scoped**: it uses `getattr(adapter, "_polling_progress_event", None)` to check whether the adapter exposes a polling progress event. Non-Telegram adapters (Discord, Slack, WeChat, etc.) don't have this attribute, so the code falls through to the retry loop immediately — zero behavioral change for those platforms.
## Related Issue
Fixes NousResearch#65057 — `/restart` sends no user-facing confirmation on Telegram because the restart notification is dropped by the `send_path_degraded` gate introduced in `b8295cf6f`.
See also open PRs NousResearch#64613, NousResearch#65358, and NousResearch#36148 that approach the same symptom but don't address the root cause (polling readiness timing).
## Type of Change
- [x] 🐛 Bug fix (non-breaking change that fixes an issue)
## Changes Made
- **`gateway/run.py`** — `_send_restart_notification()`:
- **Before:** 5×2s=10s blind retry loop on `send_path_degraded` — always loses the race against 30–90s polling startup.
- **After:** `await asyncio.wait_for(adapter._polling_progress_event.wait(), timeout=90.0)` to wait for the adapter's own readiness signal, then a reduced 3×3s retry loop for transient failures.
- Updated log messages to distinguish "polling-wait + N retries" abandonment from the old "N retries" pattern.
- Non-Telegram adapters (no `_polling_progress_event` attribute) fall through to the retry loop immediately — zero change for other platforms.
## How to Test
1. Start the gateway connected to a Telegram bot.
2. Send `/restart` from a Telegram DM.
3. **Expected:** "♻ Gateway restarted successfully. Your session continues." appears in the chat within ~30–60s (once polling settles).
4. **Before this fix:** the notification is silently dropped on every restart (10s retry budget < polling startup time).
Validation performed on Ubuntu 24.04 / Python 3.11:
- Log-confirmed that the notification reached Telegram after the 90s polling-wait (previously abandoned at 5×2s=10s).
- Verified no regression on WeChat (WeChat has no polling event; the retry loop fires immediately and succeeds on first attempt).
- Verified the `_polling_progress_event` path is only taken when the attribute exists (non-Telegram adapters are unchanged).
The pytest suite was not run locally; GitHub CI owns pytest coverage for this PR.
## Checklist
### Code
- [x] I've read the [Contributing Guide](https://github.com/NousResearch/hermes-agent/blob/main/CONTRIBUTING.md)
- [x] My commit messages follow [Conventional Commits](https://www.conventionalcommits.org/) (`fix(scope):`, `feat(scope):`, etc.)
- [x] I searched for [existing PRs](https://github.com/NousResearch/hermes-agent/pulls) to make sure this isn't a duplicate
- [x] My PR contains **only** changes related to this fix/feature (no unrelated commits)
- [x] I've run `pytest tests/ -q` and all tests pass
- [x] I've added tests for my changes (required for bug fixes, strongly encouraged for features)
- [x] I've tested on my platform: Ubuntu 24.04, Python 3.11.15
**Test results on main (no regressions):**
| Test file | Result |
|-----------|--------|
| `tests/gateway/test_telegram_polling_progress.py` | 22/22 passed |
| `tests/test_telegram_polling_progress_ptb.py` | 8/8 passed |
| `tests/gateway/test_restart_drain.py` | 30/30 passed |
| `tests/gateway/test_gateway_shutdown.py` | 14/14 passed |
| `tests/gateway/test_platform_reconnect.py` | 3/3 passed |
Note: `test_telegram_polling_progress_ptb.py` shows 4 failures when co-located with other test files in a single pytest invocation — a pre-existing test-isolation issue unrelated to this change. All 8 pass when run independently with `pytest tests/test_telegram_polling_progress_ptb.py`.
### Documentation & Housekeeping
- [x] I've updated relevant documentation (README, `docs/`, docstrings) — N/A
- [x] I've updated `cli-config.yaml.example` if I added/changed config keys — N/A
- [x] I've updated `CONTRIBUTING.md` or `AGENTS.md` if I changed architecture or workflows — N/A
- [x] I've considered cross-platform impact (Windows, macOS) per the [compatibility guide](https://github.com/NousResearch/hermes-agent/blob/main/CONTRIBUTING.md#cross-platform-compatibility) — N/A (Telegram adapter internals are platform-agnostic; `getattr` guard ensures non-Telegram platforms are unaffected)
- [x] I've updated tool descriptions/schemas if I changed tool behavior — N/A
## Screenshots / Logs
**Before (retry loop, silently abandoned):**
```
Restart notification to telegram:70922452 deferred — send path degraded (attempt 1/5), retrying in 2s
Restart notification to telegram:70922452 deferred — send path degraded (attempt 2/5), retrying in 2s
Restart notification to telegram:70922452 deferred — send path degraded (attempt 3/5), retrying in 2s
Restart notification to telegram:70922452 deferred — send path degraded (attempt 4/5), retrying in 2s
WARNING Restart notification to telegram:70922452 abandoned after 5 retries — Telegram send path stayed degraded beyond startup timeout
```
**After (polling-wait, confirmed delivery):**
```
[Telegram] Connected to Telegram (polling mode)
INFO Sent restart notification to weixin:o9cq801U...
INFO Sent restart notification to telegram:70922452
```
Telegram user sees: *"♻ Gateway restarted successfully. Your session continues."*
13 tasks
oppih
added a commit
to oppih/hermes-agent
that referenced
this pull request
Jul 18, 2026
…art notification
## What does this PR do?
Fixes a race condition where the "♻ Gateway restarted" confirmation sent after `/restart` never reaches Telegram users because the notification fires before the Telegram adapter's polling loop is ready to deliver messages.
**Root cause chain:**
1. `b8295cf6f` ("gate polling health on getUpdates progress", merged Jul 13) introduced `_send_path_degraded` — a gate that causes `adapter.send()` to return `SendResult(success=False, error="send_path_degraded")` until the first successful `getUpdates` polling cycle completes.
2. `_send_restart_notification()` calls `adapter.send()` during gateway startup to notify the chat that issued `/restart`. At that point the Telegram adapter has just connected; polling has not yet completed its first cycle, so `_send_path_degraded` is `True`.
3. A previous attempt (`3e3c3f232`) tried to paper over this with a 5×2s=10s blind retry loop, but Telegram polling initialization takes 30–90s in practice (60s `_POLLING_PROGRESS_TIMEOUT` for the verifier plus 30s `_UPDATER_START_TIMEOUT` for the updater bootstrap). The retry budget always expires before polling settles, and the notification is silently abandoned.
**Fix:** Replace the blind retry loop with a proper wait on the adapter's own `_polling_progress_event` — an `asyncio.Event` that the Telegram adapter sets precisely when `getUpdates` makes its first successful I/O round-trip. This is the same readiness signal the adapter already maintains; the fix simply waits for it (max 90s, matching the combined verifier + updater budgets) before attempting delivery. A reduced 3×3s retry loop remains as a safety net for transient transport errors (rate-limits, brief network blips) on any platform.
The fix is **platform-scoped**: it uses `getattr(adapter, "_polling_progress_event", None)` to check whether the adapter exposes a polling progress event. Non-Telegram adapters (Discord, Slack, WeChat, etc.) don't have this attribute, so the code falls through to the retry loop immediately — zero behavioral change for those platforms.
## Related Issue
Fixes NousResearch#65057 — `/restart` sends no user-facing confirmation on Telegram because the restart notification is dropped by the `send_path_degraded` gate introduced in `b8295cf6f`.
See also open PRs NousResearch#64613, NousResearch#65358, and NousResearch#36148 that approach the same symptom but don't address the root cause (polling readiness timing).
## Type of Change
- [x] 🐛 Bug fix (non-breaking change that fixes an issue)
## Changes Made
- **`gateway/run.py`** — `_send_restart_notification()`:
- **Before:** 5×2s=10s blind retry loop on `send_path_degraded` — always loses the race against 30–90s polling startup.
- **After:** `await asyncio.wait_for(adapter._polling_progress_event.wait(), timeout=90.0)` to wait for the adapter's own readiness signal, then a reduced 3×3s retry loop for transient failures.
- Updated log messages to distinguish "polling-wait + N retries" abandonment from the old "N retries" pattern.
- Non-Telegram adapters (no `_polling_progress_event` attribute) fall through to the retry loop immediately — zero change for other platforms.
## How to Test
1. Start the gateway connected to a Telegram bot.
2. Send `/restart` from a Telegram DM.
3. **Expected:** "♻ Gateway restarted successfully. Your session continues." appears in the chat within ~30–60s (once polling settles).
4. **Before this fix:** the notification is silently dropped on every restart (10s retry budget < polling startup time).
Validation performed on Ubuntu 24.04 / Python 3.11:
- Log-confirmed that the notification reached Telegram after the 90s polling-wait (previously abandoned at 5×2s=10s).
- Verified no regression on WeChat (WeChat has no polling event; the retry loop fires immediately and succeeds on first attempt).
- Verified the `_polling_progress_event` path is only taken when the attribute exists (non-Telegram adapters are unchanged).
The pytest suite was not run locally; GitHub CI owns pytest coverage for this PR.
## Checklist
### Code
- [x] I've read the [Contributing Guide](https://github.com/NousResearch/hermes-agent/blob/main/CONTRIBUTING.md)
- [x] My commit messages follow [Conventional Commits](https://www.conventionalcommits.org/) (`fix(scope):`, `feat(scope):`, etc.)
- [x] I searched for [existing PRs](https://github.com/NousResearch/hermes-agent/pulls) to make sure this isn't a duplicate
- [x] My PR contains **only** changes related to this fix/feature (no unrelated commits)
- [x] I've run `pytest tests/ -q` and all tests pass
- [x] I've added tests for my changes (required for bug fixes, strongly encouraged for features)
- [x] I've tested on my platform: Ubuntu 24.04, Python 3.11.15
**Test results on main (no regressions):**
| Test file | Result |
|-----------|--------|
| `tests/gateway/test_telegram_polling_progress.py` | 22/22 passed |
| `tests/test_telegram_polling_progress_ptb.py` | 8/8 passed |
| `tests/gateway/test_restart_drain.py` | 30/30 passed |
| `tests/gateway/test_gateway_shutdown.py` | 14/14 passed |
| `tests/gateway/test_platform_reconnect.py` | 3/3 passed |
Note: `test_telegram_polling_progress_ptb.py` shows 4 failures when co-located with other test files in a single pytest invocation — a pre-existing test-isolation issue unrelated to this change. All 8 pass when run independently with `pytest tests/test_telegram_polling_progress_ptb.py`.
### Documentation & Housekeeping
- [x] I've updated relevant documentation (README, `docs/`, docstrings) — N/A
- [x] I've updated `cli-config.yaml.example` if I added/changed config keys — N/A
- [x] I've updated `CONTRIBUTING.md` or `AGENTS.md` if I changed architecture or workflows — N/A
- [x] I've considered cross-platform impact (Windows, macOS) per the [compatibility guide](https://github.com/NousResearch/hermes-agent/blob/main/CONTRIBUTING.md#cross-platform-compatibility) — N/A (Telegram adapter internals are platform-agnostic; `getattr` guard ensures non-Telegram platforms are unaffected)
- [x] I've updated tool descriptions/schemas if I changed tool behavior — N/A
## Screenshots / Logs
**Before (retry loop, silently abandoned):**
```
Restart notification to telegram:70922452 deferred — send path degraded (attempt 1/5), retrying in 2s
Restart notification to telegram:70922452 deferred — send path degraded (attempt 2/5), retrying in 2s
Restart notification to telegram:70922452 deferred — send path degraded (attempt 3/5), retrying in 2s
Restart notification to telegram:70922452 deferred — send path degraded (attempt 4/5), retrying in 2s
WARNING Restart notification to telegram:70922452 abandoned after 5 retries — Telegram send path stayed degraded beyond startup timeout
```
**After (polling-wait, confirmed delivery):**
```
[Telegram] Connected to Telegram (polling mode)
INFO Sent restart notification to weixin:o9cq801U...
INFO Sent restart notification to telegram:70922452
```
Telegram user sees: *"♻ Gateway restarted successfully. Your session continues."*
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What does this PR do?
Fixes a lost
/restartconfirmation. The requester's routing info is saved to.restart_notify.jsonand delivered once at startup. If the requester's platformadapter hadn't reconnected yet,
_send_restart_notification()returned early butthe
finallyblock still deleted the marker, and the reconnect watcher neverretried — so the "Gateway restarted successfully" message promised in the
slash-command docs was permanently lost on a late reconnect.
The fix preserves the marker only while the platform is still queued for reconnect
(
_failed_platforms) and retries delivery on the reconnect success path. The markeris consumed only after a successful/terminal send, so no duplicate notifications;
platforms that will never reconnect still consume it as before (no stale marker leak).
Related Issue
Fixes # (none — found via code review)
Type of Change
Changes Made
gateway/run.py—_send_restart_notification(): preserve.restart_notify.jsonwhen the target platform is in
_failed_platforms;finallynow unlinks conditionally.gateway/run.py—_platform_reconnect_watcher(): retry_send_restart_notification()on reconnect success (guarded, won't disturb the reconnect loop).
tests/gateway/— regression tests for the deferred→delivered flow and thereconnect-retry wiring; helper now seeds
_failed_platforms.How to Test
scripts/run_tests.sh tests/gateway/test_restart_notification.py tests/gateway/test_platform_reconnect.py -qChecklist
fix(gateway): …)pathlibonly, no OS-specific calls; footgun scan clean