feat(display): unified per-platform proactive-push opt-out gate (cron + review + kanban + restart) + TUI parity - #59364
Conversation
…notifications Foundation for a unified per-platform proactive-push gate. Adds two keys to the display resolver (gateway/display_config.py): - proactive_push (bool, default true): master switch for whether a platform accepts UNSOLICITED / background pushes (cron responses, background-review memory notifications, kanban notifiers, background-task results, restart notices). Set false per-platform via display.platforms.<p>.proactive_push or globally via display.proactive_push. Interactive replies never consult it. - memory_notifications (off|on|verbose, default on): now registered in _GLOBAL_DEFAULTS so it resolves per-platform via resolve_display_setting and coexists, layered, with proactive_push (③). Adds helper platform_accepts_proactive_push(user_config, platform_key) — the single source of truth every background delivery path will consult. Both keys are normalised in _normalise (proactive_push→bool; memory_notifications→mode). No gate wired yet (that's the following commits). No behaviour change. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Cron responses are unsolicited background pushes. _resolve_delivery_targets now filters out any target whose platform opted out of proactive push (display.platforms.<p>.proactive_push=false, or global display.proactive_push= false), via the shared platform_accepts_proactive_push() gate. This is the single choke point, so it covers deliver=origin, deliver=all, and explicit platform:chat alike — a platform-level "do not disturb" wins over a job's routing intent. Each skip is logged (job id / platform / chat). Fail-open: a config read error delivers as before rather than silently dropping. If all targets are opted out, delivery resolves to empty and the job still runs + saves last_output (no push). Interactive replies are unaffected — this gate is only on the cron delivery path. Tests: opt-out drops that platform / keeps others, deliver=all still respects opt-out, global opt-out, no-optout keeps all, fail-open on config error, skip is logged, integration via _resolve_delivery_targets. Existing routing/delivery scheduler tests still green. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…ve_push Background-review "memory updated" notifications are a proactive push to the chat platform. Resolve memory_notifications PER-PLATFORM (was global-only) and layer it under the proactive-push gate via effective_memory_notifications(): a notification is delivered only when proactive_push != false AND memory_notifications != off. A platform opted out of proactive push gets no memory notifications regardless of its memory_notifications mode. gateway/run.py sets agent.memory_notifications from effective_memory_notifications (user_config, platform_key) — platform_key already in scope. Interactive replies are never gated; this only affects the background-review push. Tests: proactive-off forces off (even if verbose), proactive-on respects off/verbose/default, global proactive-off forces off. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…tions Extend the per-platform proactive-push gate to the remaining truly-proactive background delivery paths, all consulting the shared platform_accepts_proactive_push() helper: - Kanban notifier (gateway/kanban_watchers.py): terminal-event (completed/ blocked/crashed) pushes to subscribers are skipped for opted-out platforms; the cursor is advanced so the event isn't replayed forever. Logged. Fail-open. - Restart notice (gateway/run.py _send_restart_notification): chat-originated "gateway is back" — gated (unifies with the existing per-platform gateway_restart_notification flag). The marker is still consumed. - Home-channel startup broadcast (_send_home_channel_startup_notifications): per-platform loop gated the same way. NOT gated — background agent-task delivery (run.py:11518): that result is the answer to a task the USER explicitly requested (a delayed interactive reply), not an unsolicited push. Gating it would suppress an answer the user is waiting for — which violates the "never gate interactive replies" scoping rule. Left untouched intentionally; flagged for review. Tests: restart notice suppressed by proactive_push opt-out (marker still consumed, adapter.send not called). Existing restart (28) + kanban notifier (10) suites green. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
_load_memory_notifications() read display.memory_notifications directly, bypassing per-platform overrides. Route it through resolve_display_setting (default platform_key "cli", the TUI's single surface tier) so it honors display.platforms.<platform>.memory_notifications the same way the messaging gateway now does. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
tonydwb
left a comment
There was a problem hiding this comment.
Code Review Summary
Verdict: Approved
Changes
PR adds a _filter_proactive_push_optout() function that respects per-platform display.platforms.<p>.proactive_push=false and a global display.proactive_push=false setting. The filter is fail-open (falls back to delivering if config can't be read) and logs each skip. Clean integration into _resolve_delivery_targets().
Quality
- Well-scoped: single responsibility, clear docstring explaining the behavioral contract
- Fail-open design: correct for a delivery layer where silently dropping is worse than over-delivering
- Logging each skip: good observability for operators
- No security or correctness concerns
Suggestions
- Minor: the
platform_accepts_proactive_pushfunction is imported inside the try block — consider moving the import to the top of the file if this function grows. Not a blocker.
Reviewed by Hermes Agent
tonydwb
left a comment
There was a problem hiding this comment.
Code Review
COMMENT: feat(display): unified per-platform proactive-push opt-out gate (cron + review + kanban + restart) + TUI parity
Substantial feature addition (1418 additions) with multi-platform scope. Prior COMMENT review present. High surface area — flagging for human deep-dive. TUI parity and multi-platform opt-out gate is good but affects multiple subsystems.
Reviewed by Hermes Agent
teknium1
left a comment
There was a problem hiding this comment.
Thanks for consolidating several delivery controls behind one resolver. The core cron and gateway direction is useful, but the current implementation does not yet meet its stated unified contract.
Problems
gateway/run.py:5815-6004still sends shutdown/restart notices to active sessions and home channels after checking onlygateway_restart_notification. The PR gates post-boot restart paths but not this sibling notification path, soproactive_push: falsedoes not suppress all restart-related unsolicited delivery.tui_gateway/server.py:2387usesresolve_display_setting(..., "memory_notifications", ...)rather than the PR'seffective_memory_notifications(). A globaldisplay.proactive_push: falsetherefore suppresses gateway review notices but still allows the TUIreview.summarycallback.- The new public setting is not added to
hermes_cli/config.py:1775-1887or the display-settings documentation atwebsite/docs/user-guide/configuration.md:1457-1576.
Suggested changes
- Gate
_notify_active_sessions_of_shutdown()and cover active-session plus home-channel shutdown delivery in tests. - Route the TUI through
effective_memory_notifications(..., "cli")and test global/per-platform opt-out behavior. - Document and include the default configuration key.
This is an automated hermes-sweeper review.
| try: | ||
| from hermes_cli.config import load_config as _load_push_cfg | ||
| from gateway.display_config import platform_accepts_proactive_push | ||
| if not platform_accepts_proactive_push(_load_push_cfg(), _platform_config_key(platform)): |
There was a problem hiding this comment.
This covers only the post-boot _send_restart_notification() path. _notify_active_sessions_of_shutdown() still sends shutdown/restart notices to active sessions and home channels without this gate; apply the same resolver there or proactive_push: false does not suppress all restart-related unsolicited notifications.
| raw = (_load_cfg().get("display") or {}).get("memory_notifications") | ||
| from gateway.display_config import resolve_display_setting | ||
|
|
||
| raw = resolve_display_setting(_load_cfg(), platform_key, "memory_notifications", "on") |
There was a problem hiding this comment.
This resolves only memory_notifications, so global display.proactive_push: false still permits the TUI review-summary event while the gateway uses effective_memory_notifications() to suppress it. Use that effective helper here (with cli) if TUI parity includes the new master gate.
What
Adds one setting that lets a platform opt out of all unsolicited / background pushes, enforced in code at every background-delivery choke point — instead of each subsystem inventing its own switch.
Why / prior art
Control over unsolicited background pushes is currently fragmented and partly unresolved across several issues/PRs:
This PR consolidates them into a single, unified per-platform
proactive_pushgate enforced at every background choke point, and brings the TUI to parity (resolvememory_notificationsper-platform) so the gateway and the TUI behave identically.What's gated (all consult
platform_accepts_proactive_push())last_output).proactive_push != falseANDmemory_notifications != off.Every skip is logged; all gates are fail-open (a config read error delivers as before rather than silently dropping).
NOT gated (by design)
Changes (5 commits)
feat(display)— addproactive_pushkey, registermemory_notificationsas per-platform overrideable, addplatform_accepts_proactive_push()helper.feat(cron)— gate cron delivery.feat(gateway)— gate background-review memory notifications.feat(gateway)— gate kanban + restart notifications.fix(tui)— resolvememory_notificationsper-platform for gateway/TUI parity.Testing
pytest tests/cron/test_proactive_push_gate.py tests/gateway/test_display_config.py tests/gateway/test_restart_notification.py -q→ 102 passed.Cleanly cherry-picked onto current
main; touches only the 8 files above (no unrelated changes).