feat: add confirmed Telegram gateway restart - #65358
Conversation
fbc0f8d to
9badec6
Compare
teknium1
left a comment
There was a problem hiding this comment.
Thanks for reusing the existing restart path and adding confirmation coverage.
Problems
gateway/slash_commands.py:1372rebuilds a narrower authorization set than the gateway uses. Current main treats pairing approval as an authorization grant alongside allowlists (gateway/authz_mixin.py:439-456), with coverage for a paired user absent fromTELEGRAM_ALLOWED_USERS(tests/gateway/test_pairing_allowlist_bypass.py:60-71). That user would be able to use the gateway but would be denied this command.gateway/slash_commands.py:1347adds a second chat-originated restart entry point without the stale Telegram redelivery guard used by/restart(gateway/slash_commands.py:1244;gateway/run.py:12765-12830). The confirmation callback writes the normal marker, so a redelivered/gateway-restartcan prompt again after boot instead of being ignored.
Suggested changes
- Reuse the gateway authorization/slash-access semantics rather than maintaining a partial allowlist in this handler, with a paired-user regression test.
- Guard
/gateway-restartredeliveries before requesting confirmation and test the post-restart duplicate-update path.
Automated hermes-sweeper review.
| allowed_ids.update(str(s).strip() for s in raw if str(s).strip()) | ||
| except Exception: | ||
| pass | ||
| if str(source.user_id or "") not in allowed_ids: |
There was a problem hiding this comment.
This local allowlist is narrower than gateway authorization: current main accepts a paired Telegram user even when they are absent from TELEGRAM_ALLOWED_USERS (gateway/authz_mixin.py:439-456, covered by tests/gateway/test_pairing_allowlist_bypass.py:60-71). Please reuse the established authorization/slash-access semantics instead of reconstructing selected sources here.
| if active_agents: | ||
| return t("gateway.draining", count=active_agents) | ||
| return EphemeralReply(t("gateway.restart.restarting")) | ||
|
|
||
| async def _handle_gateway_restart_command(self, event: MessageEvent) -> Union[str, EphemeralReply, None]: |
There was a problem hiding this comment.
Please apply the existing stale Telegram restart-redelivery guard before registering this confirmation. /restart performs it before writing .restart_last_processed.json; this path writes that same marker only after confirmation, so a redelivered /gateway-restart after boot reaches a new prompt instead of being suppressed.
…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."*
9badec6 to
155e9f2
Compare
…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."*
Summary
Tests