feat(gateway): add opt-in startup online notification - #24271
feat(gateway): add opt-in startup online notification#24271leprincep35700 wants to merge 1 commit into
Conversation
2b0d68d to
7216148
Compare
|
CI status note after the latest push:
So I’m treating that e2e failure as upstream-wide noise rather than something caused by this gateway startup notification change. |
|
Additional CI note for the broad The full test job is also red, but the failure set matches current The scoped validation for this PR remains green:
I’m leaving those broad failures out of scope for this focused startup-notification change. |
7216148 to
15f32fa
Compare
15f32fa to
f684461
Compare
f684461 to
e9c3932
Compare
|
Branch refreshed onto current
GitHub now shows the PR as clean with successful current checks. |
|
Post-current-main reassessment: I rechecked this after #36028 and #36188 landed. Conclusion: this PR is now partially superseded and should be updated/downscoped rather than merged as-is. What upstream now covers:
What this PR still uniquely adds:
Recommendation:
|
408901c to
0aad64a
Compare
|
Rebased onto current
|
d43d727 to
c04c679
Compare
6954711 to
4ff3b4c
Compare
teknium1
left a comment
There was a problem hiding this comment.
Thanks for preserving the planned-restart and chat-originated /restart distinctions. The ordinary-cold-start gap is still real on current main: GatewayRunner.start() invokes the home-channel sender only when _planned_restart_notification_pending() is true (gateway/run.py:7272-7293).
Problems
- The PR only bridges a top-level
gateway_startup_notification(gateway/config.py:877-880in PR head). Current gateway settings convention uses the nestedgateway:section; current main already readsgateway.max_concurrent_sessionsthere (gateway/config.py:870-875). The nested startup setting would be ignored. - The added tests call the predicate directly (
tests/gateway/test_restart_notification.py:48-83in PR head), not theGatewayRunner.start()path that must invoke the sender.
Suggested changes
- Support
gateway.gateway_startup_notificationas well as the compatibility top-level form, with a nested-config regression test. - Add a lifecycle-level startup test for default quiet, opt-in cold boot, planned restart, and chat
/restartnon-broadcast behavior. - Reconcile the env override with current profile-scoped config reads (
gateway/config.py:169-189) if it remains part of the feature.
Automated hermes-sweeper review.
4ff3b4c to
540a65a
Compare
|
Thanks for the detailed review — all requested follow-ups are now included in Addressed
The lifecycle tests assert the real adapter sends and marker cleanup rather than only calling the predicate. Validation
The PR body has also been updated with the canonical configuration and current validation evidence. Ready for re-review. |
540a65a to
362f9e4
Compare
|
Correction to my previous readiness note: two independent reviews completed just after that comment and identified valid edge cases, so the earlier “ready” status is superseded. Fixed in
Updated local validation: 186 focused tests passed, including 37/37 restart-notification tests; ruff, Windows footgun, compilation, and diff checks pass. GitHub CI is rerunning on this corrected head; I will only mark it ready again once that run is fully green. |
362f9e4 to
6a663ca
Compare
|
Final readiness update for corrected head
The PR is now ready for re-review. |
Support the canonical nested gateway config with legacy compatibility, honor profile-scoped environment overrides, and exercise the real GatewayRunner.start lifecycle for cold, planned, and chat-originated restarts.
6a663ca to
c9080d7
Compare
|
Rebased cleanly onto current Validation:
Updated head: |
Why
Hermes already emits a completion notification after
/restart, but ordinary cold starts caused by Docker, systemd, or a host reboot remain silent. Operators with a dedicated home/back-channel need an explicit opt-in online signal without making startup pings noisy by default.What changed
gateway_startup_notificationruntime flag, defaulting tofalse;GATEWAY_STARTUP_NOTIFICATIONas an override and resolves it through Hermes' profile-scoped secret environment;/restartreplies targeted to the original chat/topic and suppresses duplicate home-channel broadcasts;/restartmarker priority when it coexists with a stale planned-restart marker, while consuming both one-shot markers on that boot;cli-config.yaml.example.Configuration
Canonical YAML:
Environment override:
The compatibility top-level YAML key remains accepted for existing users:
Validation
186 passed: gateway config, restart notification, platform reconnect, and home-target environment suites;37 passed: complete restart-notification suite after the final rebase;ruff check .passed;git diff --checkpassed;HERMES_HOME: nested config loaded,GatewayRunner.start()returnedtrue, andGatewayRunner.stop()completed cleanly;GatewayRunner.start()path and assert real adapter sends for default cold start, opted-in cold start, planned restart, chat/restart, and simultaneous chat/planned markers without duplicates or residual markers.