feat(notifications): opt-in suppression of user-channel warning notifications (display.suppress_warning_notifications) - #112302
Conversation
|
Reopening the scope review: this implementation gates selected gateway warning callbacks, not all automatic warning/failure notifications delivered to user channels. Existing cron, Kanban, background and adapter-owned paths need an explicit coverage/precedence review before this is ready. A real-config, recording-transport probe against ef42c37 reproduces the session-stall watcher posting its warning with The lifecycle prefix tuple matches existing rendered messages; it does not add warning producers. However, it couples suppression to wording and is not a complete emission contract. A producer-to-destination audit is underway, including manual/scheduled cron execution, Kanban notify/wake delivery, background completions, replay, and native/relay paths. The existing test and independent-review results remain valid for the tested subset. They do not establish global coverage or OTLP diagnostic parity. Converted back to draft pending that review; no merge/deployment claim. |
Cron: source-owned manifest journal replaces receipt parking (B1 live-owner receipts, B2 incomplete placeholder never projected/settled while outstanding, B3 consumer race); immediate native incident alert fenced to the execution's bound occurrence (S4). Refactor: Discord oversized-file disposition explicit, not text equality (B4); emit_media_warning preserves legacy shown metadata per caller (S1); Discord admin fan-out, process watcher and Google Chat fallback on shared boundaries (S2); Slack explicit interim sends never seal the native stream (S3). Salt's probes ported as regression cells.
…ence Closes Salt R4 J1-J4. Before the first deferred (Bot Chat) send the run row records delivery_manifest_pending=1; the manifest write clears it in the same statement. While set: finish_execution, the worker wait path, reconcile (projection AND write-time CAS), interrupted/abandoned recovery and retention all treat the row as queued/unprunable. The journal is a best-effort copy and never raises; a ledger fault before any send is reported truthfully pre-send. Salt R4 probes ported verbatim as regression cells.
…adopt intent Closes Salt R5 K1, K2 and S5. The intent mark stays before any send (a ledger fault sends nothing); the delivery loop is now wrapped so EVERY exit after the mark - normal, no-receipt, or exception - records a manifest (possibly with no children) through the same durable, replayable path, so a run can never stay pending without a recovery source. Upgrade adoption: the schema migration and the journal replay each assert intent for rows whose real manifest is still only journaled, in their own committed transaction, before any reader projects the placeholder. Jobs projection refuses a stale 'queued' write once the same execution settled terminally in the ledger. Salt R5 probes ported verbatim as regression cells.
…uthority Closes Salt R6 K2a/K2b and Q1. Column existence is not the migration authority (DDL autocommits). Adoption of pre-flag journaled intent now runs as one IMMEDIATE transaction that ends by writing a schema_meta fence row; the inventory read is strict (an unreadable directory or entry raises), so any failure rolls everything back, logs, and the next initialization retries. Until the fence row exists, finish, projection, prune and interrupted-run recovery hold placeholder-only rows as queued/unprunable. A second exception inside manifest settlement is logged, never masks the original. Salt R6 probes ported verbatim as regression cells.
Closes Salt R7 K2c and Q2. A one-time adoption snapshot is not a quiescence boundary for old processes: a pre-flag writer can journal its child manifest after the fence commits. Terminalization now accounts for that writer per row: a placeholder-only row with pending=0 is complete only if its own journal entry is verifiably absent; when present the intent is adopted in the caller's transaction, when unknowable the row is held. Journal replay reopens a row whose terminal outcome was derived from the placeholder alone, in the same transaction as the intent it asserts. Adoption never nests a BEGIN inside a caller's open transaction (incident upsert); it defers to the next ledger open. Salt R7 probes ported verbatim as regression cells; each layer has an isolated mutant-killing cell.
Closes Salt R8 K2d. The per-row late-legacy accounting stat'd the journal against a pre-fetch snapshot and then wrote pending=1 unconditionally; a replay that landed the complete manifest between the stat and the write was re-armed with a flag nothing could clear, stranding settlement. The UPDATE is now predicated on the row still being placeholder-only with pending=0; on a lost comparison the helper defers to the authoritative row state. Salt R8 probes ported verbatim as regression cells.
…e row Closes Salt R9 K2e and S7. finish_execution now derives the delivery decision from the row as fetched and writes it with a CAS on the exact manifest it was derived from; a concurrent replay that lands the complete manifest in between fails the CAS and the decision is re-derived from what is there now. No stale snapshot is ever projected. Manifest replay replaces any placeholder-only manifest (the same incompleteness predicate every reader uses), not only the canonical one, so a re-recorded external placeholder cannot leave a row pending forever. Salt R9 probes ported verbatim.
…licy Closes Salt R11-B1. The one-line failure echo _ChildProgressRelay prints above the CLI delegation spinner is an automatic diagnostic presentation and now passes through render_notification under the parent's turn snapshot (platform + config), like every other sync UI sink. The relayed subagent.complete event (the gateway's classified producer) and the child result itself are untouched in every mode; absent/false is legacy. Salt's three-mode real-child probe ported as a regression cell.
…he boundary Code-flow grid sweep, wave 1 (Slack/cron/relay/gateway/kanban/delegation segments). Three pre-existing automatic notices reached channels without the warning boundary: - shutdown/restart interrupt broadcast to every active chat and home channel (present_notification under each session's profile scope; an in-chat /restart requester's own ack stays, it is the answer to their command; a suppressed active-chat target latches dedup so the home pass is quiet) - unauthorized-sender owner hint on the home channel - /bg background task crash notice (emit_warning with the logical platform) Also the -Q one-shot linger: a diagnostic-only wake still runs, but under suppression its reply no longer displaces the requested stdout answer (mirrors the interactive CLI's diagnostic_turn_muted admission rule).
…ostics Code-flow grid sweep, wave 2 (turn recovery, compression, credits/init, transports, tool executor, CLI mixins, TUI core/methods). Pre-existing automatic notices that bypassed every boundary, now presentation-gated under the owning turn snapshot; logs, model-facing bookkeeping, requested results and recovery behavior are unchanged: - compression warning replayed on turn 1 lost its DiagnosticText class, so the gateway/TUI status sinks rendered it as non-diagnostic (reached Slack) - stream reconnect / stall-mid-tool-call markers fired as raw stream deltas (the stall text stays in the model-facing partial result) - agent init: invalid context_length stderr print, codex autoraise notice - tool-name auto-repair print - CLI: session store unavailable banner, browser backend downgrade hint, provider auth fallback notice, iteration budget notice, vision fallbacks - TUI: configured-model adoption failure event, /goal compression recovery notice (the recovery prompt itself is never suppressed)
Code-flow grid sweep cross-lane finding: the preview-restart progress panel wired a second status_callback straight to progress() with no is_warning_status check, so a provider fallback or compression warning raised inside a restarted preview bypassed the TUI policy the main agent sink applies. Warning rows now follow the same gate; ordinary status and tool progress always render.
…n policy Code-flow grid sweep: the local_embedded root guard printed its warning raw to stderr from initialize(). It now goes through the wired warning_callback (agent._emit_warning on CLI, already gated) or the shared render boundary; the logger.warning and the disabled-mode outcome are unchanged.
The area gate caught a NameError on the vision-analysis error branch: the boundary import was missing at function scope and the source-slice test had pre-bound the name. Import once in _preprocess_images_with_vision, tolerate a CLI without an agent yet, and cover both fallback branches through the real method.
Semantic resolutions (7 files): - gateway/run_turn.py: streaming TTS and long-running notify are quiet when EITHER the scheduled-heartbeat flag (main) OR the muted diagnostic wake (ours) applies. - cron/bot_chat_delivery.py: receipt-projection reconcile joins the drain inside main's retirement.work() admission. - cron/scheduler_delivery.py: keep main's _format_failure_streams helpers and our _mark_manifest_intent + for_failure signature. - tui_gateway/prompt_turn.py: keep the run_body/run split (policy snapshot + notification_turn); main only reflowed the follow-up call. - tools/bot_live_delivery.py: main's _next_sequence(root) + our diagnostic category stamp. - tests: both sides' additions kept.
…sinks main added this stub after the branch introduced _emit_diagnostic_status / _buffer_diagnostic_status; the rate guard swallows the AttributeError by design, so the test fell through instead of returning.
main gates WAL by SQLite build (hermes_state_wal WAL-reset-bug guard), so new databases open in DELETE on CI. Journal mode is runtime policy, not the adoption contract; the one-COMMIT/one-ROLLBACK/single-fence assertions are.
|
Thanks @victor-kyriazakos — this landed as a salvage in #114370 with your authorship preserved (your 54 commits are squashed into one commit authored by you, What was kept: the whole suppression feature — What was split out (each is a separate PR if you want to pursue it; happy to review):
Closing this one in favour of #114370. |
…ications Squash of the 54 commits on victor-kyriazakos:feat/user-channel-warning-suppression (PR #112302, head f45c640) so the contributor's authorship survives a rebase-merge; the commits interleave with a cron delivery-ledger rework that the salvage removes in follow-up commits, so per-commit cherry-picks were not practical. Adds display.suppress_warning_notifications (global + per-platform, default false): one resolver (gateway/warning_notifications.py), BasePlatformAdapter.emit_warning / emit_media_warning / warning_text, a notification_category classification carried through wakes, queues and persistence, and render/present boundaries for CLI/TUI.
Summary
Opt-in suppression of automatic warning and diagnostic notifications that Hermes pushes into user channels (Slack, Telegram, Discord, Matrix, Signal, WeChat/Weixin, Google Chat, native Bot Chat, CLI/TUI), with a per-platform override:
Absent/
falseis byte-for-byte legacy.truesuppresses automatic diagnostics only: compression/retry/fallback notices, credit-service and subagent failure notices, inactivity and watchdog stalls, media-delivery fallbacks, startup DB warnings, shutdown cron-interrupt notices, hygiene notices, cronfailure_deliveralerts. It never touches requested results, approvals, clarifications, progress, cleanup, logs, telemetry, or failure bookkeeping — the producer runs unconditionally; only presentation is gated.How it is wired
One resolver, a small set of typed boundaries, no rendered-text/prefix heuristics:
warning_notifications_enabled(platform, user_config)— the single policy read (destination-owner/profile aware, per-turn snapshot).BasePlatformAdapter.emit_warning/emit_media_warning— async channel sends;Nonemeans suppressed (never a fabricated receipt); captions/requested parts always delivered; diagnostic sends never seal an unrelated active stream.warning_text(visible, hidden)— mixed adapter text where the requested part appears in both variants.render_notification/present_notification— sync (CLI/TUI) and async (lifecycle emitters) render boundaries.diagnostic_turn_muted— the one admission rule for diagnostic-category internal wakes across gateway/CLI/TUI/API.Producers classify (
notification_category); classification survives callbacks, threads, queues, retries, persistence/replay, ingress and streaming. Direct policy reads went from 56 scattered guards to the resolver plus the boundaries above.Cron delivery ledger
Suppressed cron failure alerts are a durable disposition, not a send, so they exposed pre-existing ledger races. Fixed in this PR, each with its own regression cells:
queuedafter terminal settle); retention keeps rows until projection completes.schema_metafence row (strict inventory, rolled back and retried on any fault); readers hold placeholder-only rows until adoption completes; per-row late-writer accounting for old processes that publish after the fence.finish_executionderives its terminal decision from the authoritative row and writes with a CAS on the exact manifest it derived from.Disclosed, non-blocking follow-ups (not this PR): SQL
instr('"bot"')vs parsed-key completeness differ for two synthetic manifest shapes no producer emits; rows whose manifest and journal persistence are both permanently lost stay pending unbounded by design (needs an operator dead-letter policy).Verification
tests/cron: 1,600 passed, 0 failed, 1 Windows-only skip. Refactor slice: 302 tests / 21 files; each shared boundary has a staging mutation that fails the expected cells when disabled._load_cfg_rawsignature collision, dashboard port-in-use, updater state).tests/cron/test_r*_salt_*.py. Round 10 at3224cb1c60: CLEAR for the refactor + cron remediation scope.f5b807e7bd, sync delegate failure echo); remaining families verified at default-off byte parity.print/send/status/notice sites in the runtime tree, clustered into 31 code-flow segments, each classified by an independent lane and every reported gap re-verified by hand): 20 pre-existing automatic diagnostics were reaching users outside any boundary. All 20 now go through the shared boundary, each with tests that fail when the gate is removed — gateway shutdown/restart broadcast, unauthorized-sender owner hint,/bgcrash notice,-Qlinger admission, turn-1 compression-warning replay (lost its diagnostic class on the way to the gateway status sink), stream reconnect/stall markers, agent-init context-length and autoraise prints, tool-name auto-repair print, CLI session-store/browser-downgrade/provider-fallback/iteration-budget/vision-fallback notices, TUI model-adoption failure,/goalcompression-recovery notice, TUI preview-restart status rows, Hindsight root-guard notice. Zero gaps in Slack, cron, relay/stream, API/base adapter, kanban, or any platform adapter. Area gate at the sweep head: 44,067 passed; every failure reproduced on baseline.