#290 feat(notifications): centralised notification outbox (native + in-app banner)

[Anonymous]

Originally created by: adityaharishch

Stacked on [#288] (base = feat/daily-plan-today-tasks for migration lineage + plan.nudge's daily_plan dependency). Retarget to main once [#288] merges.

Replaces ad-hoc, hardcoded tray notification triggers with a centralised, redundant, industry-standard delivery system — and fixes the bug that made the existing toasts silently never appear.

The bug fixed

The tray initialised the notification plugin and held the capability, but never requested OS authorization — so .show() was a silent no-op on macOS. The tray now requests permission on startup; the health/pause toasts that previously did nothing now work.

Architecture (outbox pattern)

daemon producers ─enqueue()→ notifications outbox ─┬─drain→ tray relay ─→ macOS toast
 (plan/worklog/fault)         (dedup_key, channels, └─SSE──→ dashboard banner
                               scheduled/expiry,            (NotificationBanner)
                               per-channel state)
prefs + quiet-hours filter applied ONCE at the delivery layer (lib/notifications.ts)

• Centralised emit — src/notifications.rs::enqueue() (idempotent on dedup_key), mirrors notices::raise. New events = one call, no new plumbing.
• One delivery point — the tray drains /api/notifications/pending, delivers, acks /delivered (at-least-once; re-fires until acked). No per-feature dedup.
• Redundant — native toast and in-app banner. Faults keep their existing system_notices banner and gain a native toast (no double banner).
• Not hardcoded — events are data rows; preference + quiet-hours filtering is data-driven in one place.

Migration / phases (each commit builds + clippy-clean)

1. migration 042 notifications + src/notifications.rs emit module.
2. tray relay loop + permission fix; removed the hardcoded worklog toast + last_notified_drafts.
3. UI: /api/notifications/{pending,[id]/delivered,[id]/dismiss,stream}, banner store + NotificationBanner, prefs in lib/settings.ts.
4. producers: plan.nudge (morning, once/day, working-hours-gated), worklog.ready (scheduler), system.fault (notices bridge) + Settings → Notifications (master + per-type toggles + quiet hours).

Preferences

Master switch · per-type toggles (plan / worklog / faults) · quiet-hours window (wraps past midnight; gates toasts, not passive banners). Daemon tolerates the new settings fields (#[serde(default)]).

Tests

Daemon + tray cargo build/clippy -D warnings clean; UI npm run build clean; integration tests (etl, worklog_provider, task_linker_smoke — all run migrations incl. 042) pass.

🤖 Generated with Claude Code

[Anonymous]

Originally posted by: Akarsh-Hegde

🔍 Code review — centralised notification outbox

High-effort recall pass (4 finder angles, every survivor verified against source). Well-architected PR — the outbox/dedup design is clean and the consumer-side preference/quiet-hours filtering is in the right place. No crash- or data-loss-class bugs. Findings are about error visibility, a React lifecycle leak, and one channel asymmetry.

Findings (ranked)

1. Worklog-ready notification enqueue error is silently swallowed — src/pm_worklog/scheduler.rs:254

let _ = crate::notifications::enqueue(pool, …).await;

The parallel plan-nudge path logs its failure (src/main.rs:813 → tracing::debug!(error = %e, …)), but here the result is discarded. On any enqueue failure (e.g. a pre-migration-042 DB) the worklog-ready toast silently never fires and nothing is logged — drafts show ready in the dashboard but the tray stays quiet with no diagnostic. Mirror the nudge site: if let Err(e) = enqueue(…).await { tracing::warn!(error = %e, "worklog-ready notification enqueue failed") }. (Also the project convention is .context() + log on DB calls.)

2. Banner reconnect timer leaks on unmount — ui/components/NotificationBanner.tsx:31-36
es.onerror does setTimeout(connect, 5_000), but the effect cleanup only closes the current EventSource — it never clears that pending timeout. If the component unmounts during the 5s backoff window, the timeout fires connect() on an unmounted component: a new EventSource is created after cleanup ran (so it's never closed → leak) and setItems runs on an unmounted component. Track the timeout id in a ref (or a cancelled flag) and clear it in the cleanup.

3. Native toasts silently drop deep_link (and severity) — tray/src-tauri/src/poll.rs:46-50 & :325
PendingNotif deserializes only {id, title, body} and notify() sets only title+body. Producers set click-throughs (.link("/plan"), .link("/worklogs")) and the banner channel renders an "Open →" link — but the macOS toast has no click action, so clicking a native notification does nothing. Asymmetry between the two channels; fine if intended for v1, but worth a tracking note since producers are already populating deep_link.

Minor / cleanup

• drain_notifications swallows JSON parse errors — poll.rs:275 r.json().await.unwrap_or_default() returns [] silently on a shape change. Consistent with the tray's best-effort style, but a debug! on the parse error would make a future regression visible.
• Banner-store interval is never torn down — ui/lib/notifications-banner-store.ts:57-60 once created the 30s setInterval lives forever even after the last subscriber leaves. Low impact (broadcast() early-returns when controllers().size === 0, and it mirrors notices-store), but it's a permanent no-op timer on globalThis.
• delivered/dismiss routes accept any integer id (incl. <= 0) → no-op UPDATE returns 200. Harmless given idempotency, but id > 0 validation would be tighter.

Candidates checked and cleared (so they aren't re-raised)

• "idx_notifications_native_pending can't serve delivered_native_at IS NULL" — false; SQLite stores NULLs in indexes and can seek col IS NULL.
• "NOW_ISO() called twice → inconsistent timestamps within a query" — false; it's read once into now and reused (.all(now, now)).
• "Rows get marked delivered during quiet hours and never re-fire" — false; pendingNative() returns [] while quiet, so the tray fires/acks nothing and delivered_native_at stays NULL until the window ends.
• "SSE ctrl may be used uninitialized" (stream/route.ts) — false; start() runs synchronously during ReadableStream construction, so ctrl is assigned before cancel() can run (same pattern as /api/notices/stream).

_{🤖 Assisted review via Claude Code.}

[Anonymous]

Originally posted by: adityaharishch

Thanks @Akarsh-Hegde — high-signal review. Addressed in e34e80d.

1. Worklog-ready enqueue error swallowed — ✅ Fixed. let _ = → if let Err(e) … { tracing::warn!(error = %e, "worklog-ready notification enqueue failed") }, mirroring the plan-nudge site (src/pm_worklog/scheduler.rs:253).

2. Banner reconnect timer leaks on unmount — ✅ Fixed. The 5s reconnect setTimeout is now tracked in a retryRef and there's a cancelled flag; cleanup clears the timer, closes the EventSource, and connect() early-returns if cancelled — so no post-unmount EventSource/setItems (ui/components/NotificationBanner.tsx).

3. Native toast drops deep_link/severity — Confirmed and intentionally deferred for v1, as you suspected. Click-to-navigate on a macOS toast needs Tauri notification actions + a focus/navigate handler, which is more than a field add. Rather than half-wire it, I documented the asymmetry at notify() (tray/src-tauri/src/poll.rs): the banner channel carries the link, the native toast is title+body. Happy to open a follow-up issue to wire native click-through if you'd like it tracked.

Minor — id <= 0 validation — ✅ Fixed in both delivered and dismiss routes (!Number.isInteger(nid) || nid <= 0).

Minor — banner-store interval never torn down — ✅ Fixed. unsubscribe() now clears the shared 30s interval when the last subscriber leaves; subscribe() re-arms it via ensureInterval().

Minor — drain_notifications swallows JSON parse errors — Intentionally not changed. The tray has zero logging infrastructure (no tracing/log dep, no eprintln! anywhere in tray/src-tauri/src/) — it's a deliberately silent menubar app. Adding a logging facility for one best-effort parse would break that consistency for little gain, and as you noted it's consistent with the tray's best-effort style. Glad to revisit if we ever add structured logging to the tray.

Also thanks for the four cleared-candidate notes — saved a re-investigation pass. Builds: daemon cargo check, tray cargo check, and npm run build all green; full pre-push suite passed.

[Anonymous]

Ticket changed by: adityaharishch

• status: open --> closed