Skip to content

fix(gateway): notify home channels after cold start and crash recovery - #69676

Open
plcunha wants to merge 3 commits into
NousResearch:mainfrom
plcunha:fix/gateway-startup-notification-and-health
Open

fix(gateway): notify home channels after cold start and crash recovery#69676
plcunha wants to merge 3 commits into
NousResearch:mainfrom
plcunha:fix/gateway-startup-notification-and-health

Conversation

@plcunha

@plcunha plcunha commented Jul 22, 2026

Copy link
Copy Markdown
Contributor

1|## Summary
2|
3|Notify configured home channels when the gateway becomes available after a true cold start or crash recovery, while preserving mode-specific behavior for chat-originated /restart.
4|
5|This PR has been re-scoped against current main in response to review. It no longer duplicates Telegram adapter readiness or dashboard PID-liveness logic already implemented upstream.
6|
7|## Current behavior
8|
9|Current main sends the home-channel lifecycle notification only for a non-chat planned restart. A cold service start or crash recovery completes silently even when a home channel is configured.
10|
11|## Change
12|
13|- Send the existing lightweight home-channel startup notification when no chat-originated restart marker was present.
14|- Keep chat /restart scoped to its originating chat/topic and suppress a duplicate home-channel broadcast.
15|- Preserve planned-restart marker cleanup.
16|- Treat home-channel notification delivery as best effort so it cannot abort gateway startup.
17|
18|## Explicitly not included
19|
20|- No runner-level Telegram _send_path_degraded polling; adapter readiness remains owned by the Telegram adapter.
21|- No runtime-status PID override; dashboard liveness remains owned by resolve_gateway_liveness() and get_runtime_status_running_pid().
22|
23|## Tests
24|
25|Added mode-specific regressions for:
26|
27|1. cold/crash-recovery startup broadcasts exactly through the home-channel path;
28|2. chat-originated /restart does not also broadcast to home.
29|
30|Validation on current origin/main:
31|
32|- 21 passed across startup and restart-notification suites
33|- Ruff passed
34|- py_compile passed
35|- git diff --check passed
36|
37|## Policy note
38|
39|This intentionally isolates the remaining maintainer decision identified by the sweeper: whether configured home channels should receive an online notification after cold start/crash recovery. The implementation and tests are now limited to that policy choice.
40|

@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/telegram Telegram bot adapter sweeper:risk-message-delivery Sweeper risk: may drop, duplicate, misroute, or suppress messages needs-decision Awaiting maintainer decision before any implementation labels Jul 22, 2026
@alt-glitch

Copy link
Copy Markdown
Collaborator

This was generated by AI during triage.

Related to #69240: both address Telegram startup readiness, but this PR waits after connection for the send path while #69240 makes initial polling readiness part of connection establishment. Maintainer decision needed before combining the policies.

@plcunha

plcunha commented Jul 23, 2026

Copy link
Copy Markdown
Contributor Author

Found and fixed a lifecycle-notification race in 1dab4c7d9: _send_restart_notification() consumes .restart_notify.json in finally, so re-reading the marker afterward always looked like a cold start and triggered an additional Home broadcast after chat-originated /restart. Startup now uses the pre-send snapshot. Added an end-to-end startup regression test. Validation: 13 startup tests + 153 restart/status tests passed locally; ruff and git diff --check are clean. Hosted CI is fully green (26 successful checks, no failures or pending checks).

@teknium1

Copy link
Copy Markdown
Contributor

Thanks for investigating the startup-notification failures. Two parts of this PR have since been addressed on current main at more appropriate boundaries.

Problems

  • Telegram cold startup now passes require_progress=not is_reconnect at plugins/platforms/telegram/adapter.py:4041-4048; its strict gate waits for generation-scoped getUpdates progress before success at :2419-2459 (c8ff720508). The runner-level _send_path_degraded polling is therefore redundant.
  • Dashboard liveness already validates runtime PIDs through resolve_gateway_liveness() in hermes_cli/web_server.py:2995-3005 and get_runtime_status_running_pid() in gateway/status.py:1293-1334.
  • The all-start broadcast remains a policy decision: current main explicitly limits it to non-chat planned restarts at gateway/run.py:10941-10952.

Suggested changes

  • Re-scope any salvage to the cold-start/crash-recovery broadcast policy after maintainer agreement, with mode-specific delivery and duplicate-suppression tests.
  • Keep the existing adapter readiness and dashboard-liveness paths rather than duplicating them in GatewayRunner or read_runtime_status().

Automated hermes-sweeper review.

@teknium1 teknium1 added sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform labels Jul 30, 2026
@plcunha plcunha changed the title fix(gateway): wait for adapter readiness + cold-start notify + PID liveness check fix(gateway): notify home channels after cold start and crash recovery Jul 30, 2026
@alt-glitch alt-glitch added type/feature New feature or request P3 Low — cosmetic, nice to have and removed type/bug Something isn't working platform/telegram Telegram bot adapter P2 Medium — degraded but workaround exists labels Jul 30, 2026
@alt-glitch

Copy link
Copy Markdown
Collaborator

This was generated by AI during triage.

Related to #62512 and #27870. Current main already contains the Telegram-readiness half; this PR's remaining cold/crash home-channel broadcast is a maintainer policy decision.

@plcunha

plcunha commented Jul 31, 2026

Copy link
Copy Markdown
Contributor Author

Reconciled with current main and re-scoped on head fbf193c8d. The Telegram readiness and PID-liveness portions already implemented upstream were removed from the net diff. The PR now covers only the remaining cold-start/crash-recovery home-channel notification policy, while chat-originated /restart continues to notify only its originating chat/topic and avoids a duplicate Home broadcast. Startup regressions passed locally; the branch is mergeable with 27 successful checks and no failures. The title/body now describe this narrower policy decision explicitly.

@alt-glitch alt-glitch removed the sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades label Jul 31, 2026
plcunha added 3 commits August 7, 2026 15:11
…+ cold-start notify + PID liveness check

Three fixes for the gateway startup notification cascade:

1. Wait for Telegram _send_path_degraded to clear before sending
   home-channel startup notifications (up to 15s). The old fixed
   1.0s sleep raced the first getUpdates cycle on networks where
   Telegram needs IPv6 fallback -> DoH -> sticky IPv4 (>1s).
   Fixes the race documented in NousResearch#66589.

2. Send home-channel startup notification on ALL starts (cold
   start, crash recovery, post-update restart), not only planned
   restarts. Chat-originated /restart still suppresses the
   duplicate (its reply target already covers that lifecycle).
   Fixes the design gap in NousResearch#62512.

3. Add PID liveness check to read_runtime_status() so dashboards
   and /api/status report 'stopped' instead of 'running' when
   the gateway PID is dead but the state file is stale.
   Fixes the false-positive health check that masked NousResearch#56524.
The _pid_exists helper already existed at gateway/status.py:733 with
proper cross-platform handling (Windows-safe via ctypes, not os.kill).
Reuse it instead of adding a redundant function that duplicates the
same logic and breaks existing test mocks that patch _pid_exists.
@plcunha
plcunha force-pushed the fix/gateway-startup-notification-and-health branch from fbf193c to de2cf15 Compare August 7, 2026 18:13
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 needs-decision Awaiting maintainer decision before any implementation P3 Low — cosmetic, nice to have sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform sweeper:risk-message-delivery Sweeper risk: may drop, duplicate, misroute, or suppress messages type/feature New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants