Skip to content

fix(gateway): notify home channel after service-managed restart - #34739

Closed
spiky02plateau wants to merge 1 commit into
NousResearch:mainfrom
spiky02plateau:fix/gateway-service-restart-startup-notify
Closed

fix(gateway): notify home channel after service-managed restart#34739
spiky02plateau wants to merge 1 commit into
NousResearch:mainfrom
spiky02plateau:fix/gateway-service-restart-startup-notify

Conversation

@spiky02plateau

@spiky02plateau spiky02plateau commented May 29, 2026

Copy link
Copy Markdown
Contributor

What does this PR do?

Fixes gateway lifecycle notifications for service-manager / graceful-signal restarts. When the gateway successfully sends the pre-shutdown home-channel warning, it now records a short-lived startup marker and sends a matching "back up" notification to that same home target after the next startup settles.

Related Issue

No linked issue.

Type of Change

  • 🐛 Bug fix (non-breaking change that fixes an issue)
  • ✨ New feature (non-breaking change that adds functionality)
  • 🔒 Security fix
  • 📝 Documentation update
  • ✅ Tests (adding or improving test coverage)
  • ♻️ Refactor (no behavior change)
  • 🎯 New skill (bundled or hub)

Changes Made

  • Keeps .restart_notify.json scoped to direct /restart requester completion messages.
  • Adds a separate .startup_notify.json marker for graceful signal/service-manager restarts.
  • Writes the startup marker after the shutdown warning is successfully delivered to the configured home channel, including when that home target was already notified via the active-session shutdown path.
  • Stores a per-platform list of home targets so every connected home channel (not just the last one written) receives the back-up message on startup — previously the marker held a single target and the shutdown loop overwrote it per platform (last-platform-wins).
  • Guards the marker write so a disk / read-only / permission error is logged and swallowed instead of propagating out of the shutdown teardown sequence (it runs inside _notify_active_sessions_of_shutdown, awaited by _stop_impl with no surrounding try, so an unguarded raise would skip the rest of teardown).
  • Consumes the marker on next startup and sends: ♻ Gateway restarted successfully. Hermes is back and ready.
  • Preserves thread/topic metadata.
  • Honors gateway_restart_notification=false.
  • Unlinks stale/invalid/consumed markers to avoid delayed cold-start spam.
  • Avoids duplicate startup messages across direct /restart, marker, and generic home-channel startup notifications.
  • Tolerates a legacy single-target marker on disk so a marker written by an older build is still consumed on the next startup.

How to Test

Targeted verification run after rebasing onto current origin/main:

  1. python -m pytest tests/gateway/test_restart_notification.py -q -o 'addopts='
  2. python -m pytest tests/gateway/test_restart_redelivery_dedup.py -q -o 'addopts='
  3. python -m pytest tests/gateway/test_restart_notification.py tests/gateway/test_restart_redelivery_dedup.py -q -o 'addopts='
  4. python -m py_compile gateway/run.py tests/gateway/test_restart_notification.py tests/gateway/test_restart_redelivery_dedup.py

Checklist

Code

  • I've read the Contributing Guide
  • My commit messages follow Conventional Commits (fix(scope):, feat(scope):, etc.)
  • I searched for existing PRs to make sure this isn't a duplicate
  • My PR contains only changes related to this fix/feature
  • I've run pytest tests/ -q and all tests pass
  • I've added tests for my changes
  • I've tested on my platform: macOS

Documentation & Housekeeping

  • I've updated relevant documentation — or N/A
  • I've updated cli-config.yaml.example if I added/changed config keys — or N/A
  • I've updated CONTRIBUTING.md or AGENTS.md if I changed architecture or workflows — or N/A
  • I've considered cross-platform impact (Windows, macOS) — or N/A
  • I've updated tool descriptions/schemas if I changed tool behavior — or N/A

Notes

A broader python -m pytest tests/gateway -q run hit OSError: [Errno 24] Too many open files on macOS — pre-existing gateway test-infra/file-descriptor debt, not a regression from this patch. The targeted restart suites stay clean (43 passed).

@spiky02plateau
spiky02plateau force-pushed the fix/gateway-service-restart-startup-notify branch 2 times, most recently from b0ee74b to ea03fd9 Compare May 29, 2026 17:16
@alt-glitch alt-glitch added type/feature New feature or request P3 Low — cosmetic, nice to have comp/gateway Gateway runner, session dispatch, delivery labels May 29, 2026
@alt-glitch

Copy link
Copy Markdown
Collaborator

Related: #20892 (merged — per-platform restart notification flag), #27278 and #32147 (both open — unconditional startup notifications), #20943 (open — scope restart notices to active chats). This PR adds a marker-file approach for service-managed restarts specifically.

@spiky02plateau
spiky02plateau force-pushed the fix/gateway-service-restart-startup-notify branch from ea03fd9 to 9a421c8 Compare May 29, 2026 19:12
@teknium1

Copy link
Copy Markdown
Contributor

Thanks for the careful lifecycle-notification work. This is an automated hermes-sweeper review; current main already implements the requested service-managed restart notification behavior through a different marker contract.

  • b14e15c48 (fix(gateway): clean service restart notifications) introduced .restart_pending.json for non-chat planned/service restarts.
  • gateway/run.py:7292-7313 waits for connected adapters, sends configured home-channel startup notifications for that marker, then clears it.
  • gateway/run.py:14900-14945 fans delivery out across connected configured home targets, preserves thread routing, and respects gateway_restart_notification.
  • Existing coverage is present in tests/gateway/test_restart_notification.py:35-45 and :249-370.

The member discussion correctly distinguished this from direct /restart notification behavior; that service-managed path is now covered on main, so this separate .startup_notify.json implementation is redundant.

@teknium1 teknium1 closed this Jul 13, 2026
@teknium1 teknium1 added the sweeper:implemented-on-main Sweeper: behavior already present on current main label Jul 13, 2026
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 P3 Low — cosmetic, nice to have sweeper:implemented-on-main Sweeper: behavior already present on current main type/feature New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants