feat(push): suppress notifications to devices where the app is visible (presence-based send-time filter) #160

New issue

Closed

opened 2026-07-13 19:45:25 +02:00 by dominik.polakovics · 1 comment

dominik.polakovics commented

2026-07-13 19:45:25 +02:00

Owner

Problem

Push notifications (chat needs-input/question, AFK done) are broadcast to every enrolled device — including the one where the operator is currently looking at the app. A device on which the app is visible/focused should not receive a push; all other enrolled devices still should.

Decision record (grill 2026-07-13)

"Open" = visible/focused only (document.visibilityState === 'visible'). A backgrounded tab or minimized PWA still gets notifications — the user isn't looking at it.
Suppression is 100% server-side at the trigger layer. ADR-0039 stands untouched: the SW push handler continues to unconditionally showNotification (iOS revokes subscriptions after silent pushes; no documented focused-client exemption in WebKit). No SW fallback, no UA sniffing. A Chromium-only SW carve-out was considered and dropped.
Staleness bias: err toward notifying. Presence dies the moment its SSE connection does. No TTL tuning, no grace window, no delayed-send queue. Failure mode is a benign extra notification, never a lost one.
Presence transport: SSE liveness + visibility beacon.
- Client generates a per-connection UUID and appends it to the EventSource URL (/api/v1/events?conn=<uuid>).
- The SSE handler registers the conn on connect and deletes its presence entry on disconnect (free, instant "not present").
- New authenticated endpoint POST /api/v1/presence with {conn, device, visible}; client sends it on visibilitychange, once after the stream opens, and via navigator.sendBeacon on pagehide (closes the laptop-lid / app-swipe gap).
Device key: SHA-256 hex of the push subscription endpoint. Client computes it from its live subscription (crypto.subtle); server hashes stored endpoints when filtering. Zero schema change, capability URL never appears in URLs/logs, self-heals when the browser rotates the subscription.
Filter lives inside Sender.Broadcast() (internal/push/sender.go) — single choke point, presence view injected into the Sender. The chat/afk notify closures in cmd/lab/main.go stay untouched (ADR-0038 boundary). Send() (single subscription) deliberately bypasses the filter, so the Settings "Send test" button always delivers.
Multi-tab: a device is present iff ANY of its connections is alive AND visible.
Suppressed = dropped. No in-app toast/badge replacement — the live SSE-driven UI is the notification when the app is visible. If being on another page proves lossy in practice, an in-app nudge is a separate follow-up issue.
Unconditional behavior, no settings knob.

Mechanics & edge cases

Presence store is in-memory (conn → {deviceHash, visible}); restart clears it → everyone counts absent → notify (the safe direction).
Beacon for an unknown conn: ignore; client re-sends after the stream (re)opens — hook into the existing wake/reconnect logic in web/src/sse.ts (it already handles visibilitychange/focus/online/pageshow).
Devices without a push subscription never send a device hash; they have no push_subscriptions row, nothing to suppress.
Debounce interplay: the filter runs at send time inside Broadcast, i.e. after the 2s notifyGate debounce in internal/chat/notify.go — presence is evaluated when the push actually fires.
Accepted residual risk: a hard kill that fires no pagehide leaves a stale visible presence until the next SSE heartbeat write fails (~25s); a notification in that window is lost. sendBeacon on pagehide narrows this to rare cases.
iOS: backgrounding the PWA fires visibilitychange (beacon flips to hidden) and the OS suspends the page (SSE dies) — so a non-foregrounded iPhone reliably receives pushes. Every push that is delivered still shows a notification: Apple-compliant, SW unchanged.

Touch points

internal/push/sender.go — presence interface + filter in Broadcast()
new presence registry (small; internal/httpapi or its own package) + POST /api/v1/presence handler
internal/httpapi/sse.go — conn param registration/deregistration
web/src/sse.ts — conn UUID, beacon wiring (visibilitychange, pagehide, post-open)
small web util: SHA-256 of the subscription endpoint
cmd/lab/main.go — wiring
ADR: new docs/adr/00xx-presence-based-push-suppression.md referencing ADR-0038/0039 (written alongside the implementation)

Acceptance

App visible on device A: needs-input push is NOT delivered to A, IS delivered to enrolled device B.
App hidden/backgrounded/closed on A: push IS delivered to A.
Settings "Send test" always delivers, even while looking at the app.
web/public/sw.js unchanged: every delivered push still shows a notification.
Presence entry disappears on SSE disconnect (verified in tests); server restart suppresses nothing.

## Problem Push notifications (chat needs-input/question, AFK done) are broadcast to **every** enrolled device — including the one where the operator is currently looking at the app. A device on which the app is visible/focused should not receive a push; all other enrolled devices still should. ## Decision record (grill 2026-07-13) 1. **"Open" = visible/focused only** (`document.visibilityState === 'visible'`). A backgrounded tab or minimized PWA still gets notifications — the user isn't looking at it. 2. **Suppression is 100% server-side at the trigger layer.** ADR-0039 stands untouched: the SW push handler continues to unconditionally `showNotification` (iOS revokes subscriptions after silent pushes; no documented focused-client exemption in WebKit). No SW fallback, no UA sniffing. A Chromium-only SW carve-out was considered and dropped. 3. **Staleness bias: err toward notifying.** Presence dies the moment its SSE connection does. No TTL tuning, no grace window, no delayed-send queue. Failure mode is a benign extra notification, never a lost one. 4. **Presence transport: SSE liveness + visibility beacon.** - Client generates a per-connection UUID and appends it to the EventSource URL (`/api/v1/events?conn=<uuid>`). - The SSE handler registers the conn on connect and deletes its presence entry on disconnect (free, instant "not present"). - New authenticated endpoint `POST /api/v1/presence` with `{conn, device, visible}`; client sends it on `visibilitychange`, once after the stream opens, and via `navigator.sendBeacon` on `pagehide` (closes the laptop-lid / app-swipe gap). 5. **Device key: SHA-256 hex of the push subscription endpoint.** Client computes it from its live subscription (`crypto.subtle`); server hashes stored endpoints when filtering. Zero schema change, capability URL never appears in URLs/logs, self-heals when the browser rotates the subscription. 6. **Filter lives inside `Sender.Broadcast()`** (`internal/push/sender.go`) — single choke point, presence view injected into the Sender. The chat/afk notify closures in `cmd/lab/main.go` stay untouched (ADR-0038 boundary). `Send()` (single subscription) deliberately bypasses the filter, so the Settings **"Send test" button always delivers**. 7. **Multi-tab:** a device is present iff ANY of its connections is alive AND visible. 8. **Suppressed = dropped.** No in-app toast/badge replacement — the live SSE-driven UI *is* the notification when the app is visible. If being on another page proves lossy in practice, an in-app nudge is a separate follow-up issue. 9. **Unconditional behavior, no settings knob.** ## Mechanics & edge cases - Presence store is **in-memory** (`conn → {deviceHash, visible}`); restart clears it → everyone counts absent → notify (the safe direction). - Beacon for an unknown conn: ignore; client re-sends after the stream (re)opens — hook into the existing wake/reconnect logic in `web/src/sse.ts` (it already handles `visibilitychange`/`focus`/`online`/`pageshow`). - Devices without a push subscription never send a device hash; they have no `push_subscriptions` row, nothing to suppress. - Debounce interplay: the filter runs at **send time** inside `Broadcast`, i.e. after the 2s `notifyGate` debounce in `internal/chat/notify.go` — presence is evaluated when the push actually fires. - Accepted residual risk: a hard kill that fires no `pagehide` leaves a stale visible presence until the next SSE heartbeat write fails (~25s); a notification in that window is lost. `sendBeacon` on `pagehide` narrows this to rare cases. - iOS: backgrounding the PWA fires `visibilitychange` (beacon flips to hidden) and the OS suspends the page (SSE dies) — so a non-foregrounded iPhone reliably receives pushes. Every push that *is* delivered still shows a notification: Apple-compliant, SW unchanged. ## Touch points - `internal/push/sender.go` — presence interface + filter in `Broadcast()` - new presence registry (small; `internal/httpapi` or its own package) + `POST /api/v1/presence` handler - `internal/httpapi/sse.go` — `conn` param registration/deregistration - `web/src/sse.ts` — conn UUID, beacon wiring (`visibilitychange`, `pagehide`, post-open) - small web util: SHA-256 of the subscription endpoint - `cmd/lab/main.go` — wiring - ADR: new `docs/adr/00xx-presence-based-push-suppression.md` referencing ADR-0038/0039 (written alongside the implementation) ## Acceptance - App visible on device A: needs-input push is NOT delivered to A, IS delivered to enrolled device B. - App hidden/backgrounded/closed on A: push IS delivered to A. - Settings "Send test" always delivers, even while looking at the app. - `web/public/sw.js` unchanged: every delivered push still shows a notification. - Presence entry disappears on SSE disconnect (verified in tests); server restart suppresses nothing.

dominik.polakovics added the

ready-for-agent

label

2026-07-13 19:45:25 +02:00

dominik.polakovics referenced this issue from a commit

2026-07-13 21:12:49 +02:00

feat(push): suppress notifications to devices where the app is visible

dominik.polakovics referenced this issue from a pull request that will close it,

2026-07-13 21:13:12 +02:00

feat(push): suppress notifications to devices where the app is visible (presence-based send-time filter) #162

dominik.polakovics commented

2026-07-13 22:02:51 +02:00

Author

Owner

This was generated by AI while landing a PR.

Landing audit — PR #162 → PASS

Verification signal relied on: Forgejo Actions ci / native (the required check) — success, 5m48s, run 179. Not re-run locally. The hermetic ci-nix gate did not run, correctly: it is path-gated to **/*.nix / flake.lock / go.mod / go.sum, none of which this diff touches, so no coverage was skipped.

Conflicts: none — afk/160 merges cleanly into current main (probed with git merge-tree).

Conventions: title is Conventional Commits (feat(push): …); body carries a working Closes #160; ADR-0044 is the next free number (main is at 0043).

Diff review (24 files, +1460/-31):

Implementation maps 1:1 onto the nine grill decisions in this issue: visible-only definition, 100% server-side suppression, in-memory registry, SSE-owned liveness + beacon refinement, SHA-256 endpoint device key, filter inside Broadcast() with Send() bypassing, multi-tab = ANY-live-AND-visible, suppressed = dropped, no settings knob.
internal/presence.Registry: Update no-ops on an unknown conn, so a stray beacon cannot resurrect a reaped entry; Visible("") is false, so an under-initialised tab cannot silence pushes. Every staleness path errs toward notifying, as decision 3 requires.
internal/httpapi/sse.go: Connect happens-before the 200 and Disconnect is deferred — the ordering is load-bearing and is correct.
CSRF waiver reviewed in detail. csrfMiddleware waives only the X-Lab-Csrf header, only for POST /api/v1/presence (exact path, exact method). The Origin checks stay fully enforced, including the reject-absent-Origin rule, so a cross-site page still cannot forge the request; the custom header was redundant defence on this endpoint, not the defence. The worst forgeable outcome is a wrong presence bit — an extra or a suppressed notification — never a data change. Pinned by TestCSRFPresenceBeaconWaivesHeader.
Also fixes a pre-existing data race in Broadcast: each dispatch now gets its own payload copy (webpush-go pads the slice it is handed, in place).
Every push.NewSender caller updated; sw.js is untouched (ADR-0039 holds); presence route is behind requireAuth and stays unmounted when the registry is nil.

Acceptance mapping verified against the tree: TestBroadcastSuppressesVisibleDevice, TestSendBypassesPresence, TestSSERegistersPresenceForLifetimeOfStream, plus TestPresenceUnknownConnDoesNotResurrect / TestPresenceRequiresAuth / TestPresenceDisabledLeavesRouteUnmounted — all present.

Non-blocking observations (accepted residuals, not regressions):

The stale-visible window this issue already accepts (a hard kill firing no pagehide leaves a visible entry until the SSE heartbeat write fails, ~25s; a push in that window is lost) also covers a network-blip reconnect: the old server handler may not have noticed the dead socket while the new conn is already registered, and since Visible is ANY-of, the device reads visible from the stale entry. Bounded by the same ~25s and biased the same way the design chose.
The CSRF waiver hardcodes the literal path /api/v1/presence inside the middleware; if the route is ever moved or gains a trailing-slash variant the waiver drifts silently. The test pins the current shape, so this is a note for future maintainers only.

Verdict: PASS — no blockers. Merge is gated on the maintainer's explicit go-ahead.

> *This was generated by AI while landing a PR.* ## Landing audit — PR #162 → **PASS** **Verification signal relied on:** Forgejo Actions `ci / native` (the required check) — **success**, 5m48s, run 179. Not re-run locally. The hermetic `ci-nix` gate did not run, correctly: it is path-gated to `**/*.nix` / `flake.lock` / `go.mod` / `go.sum`, none of which this diff touches, so no coverage was skipped. **Conflicts:** none — `afk/160` merges cleanly into current `main` (probed with `git merge-tree`). **Conventions:** title is Conventional Commits (`feat(push): …`); body carries a working `Closes #160`; ADR-0044 is the next free number (main is at 0043). **Diff review (24 files, +1460/-31):** - Implementation maps 1:1 onto the nine grill decisions in this issue: visible-only definition, 100% server-side suppression, in-memory registry, SSE-owned liveness + beacon refinement, SHA-256 endpoint device key, filter inside `Broadcast()` with `Send()` bypassing, multi-tab = ANY-live-AND-visible, suppressed = dropped, no settings knob. - `internal/presence.Registry`: `Update` no-ops on an unknown conn, so a stray beacon cannot resurrect a reaped entry; `Visible("")` is false, so an under-initialised tab cannot silence pushes. Every staleness path errs toward notifying, as decision 3 requires. - `internal/httpapi/sse.go`: `Connect` happens-before the 200 and `Disconnect` is deferred — the ordering is load-bearing and is correct. - **CSRF waiver reviewed in detail.** `csrfMiddleware` waives only the `X-Lab-Csrf` header, only for `POST /api/v1/presence` (exact path, exact method). The Origin checks stay fully enforced, including the reject-absent-Origin rule, so a cross-site page still cannot forge the request; the custom header was redundant defence on this endpoint, not the defence. The worst forgeable outcome is a wrong presence bit — an extra or a suppressed notification — never a data change. Pinned by `TestCSRFPresenceBeaconWaivesHeader`. - Also fixes a **pre-existing data race** in `Broadcast`: each dispatch now gets its own payload copy (webpush-go pads the slice it is handed, in place). - Every `push.NewSender` caller updated; `sw.js` is untouched (ADR-0039 holds); presence route is behind `requireAuth` and stays unmounted when the registry is nil. **Acceptance mapping verified against the tree:** `TestBroadcastSuppressesVisibleDevice`, `TestSendBypassesPresence`, `TestSSERegistersPresenceForLifetimeOfStream`, plus `TestPresenceUnknownConnDoesNotResurrect` / `TestPresenceRequiresAuth` / `TestPresenceDisabledLeavesRouteUnmounted` — all present. **Non-blocking observations (accepted residuals, not regressions):** 1. The stale-visible window this issue already accepts (a hard kill firing no `pagehide` leaves a visible entry until the SSE heartbeat write fails, ~25s; a push in that window is lost) also covers a network-blip reconnect: the old server handler may not have noticed the dead socket while the new conn is already registered, and since `Visible` is ANY-of, the device reads visible from the stale entry. Bounded by the same ~25s and biased the same way the design chose. 2. The CSRF waiver hardcodes the literal path `/api/v1/presence` inside the middleware; if the route is ever moved or gains a trailing-slash variant the waiver drifts silently. The test pins the current shape, so this is a note for future maintainers only. Verdict: **PASS** — no blockers. Merge is gated on the maintainer's explicit go-ahead.

dominik.polakovics closed this issue

2026-07-13 22:19:00 +02:00

dominik.polakovics referenced this issue

2026-07-18 21:46:57 +02:00

web: SSE-driven refetch/render churn makes sends and chat switching sluggish as the fleet grows #175

dominik.polakovics referenced this issue

2026-07-18 22:23:16 +02:00

perf(chat): tail-only SSE refetch with content-hash identity to stop render churn #178