Skip to content

Notifications have no per-type mute and no notion of "you caused this" — #176's visibility gate cannot cover your other devices #360

Description

@serge-ivo

Not a re-open of #176 — that fix works, and it cannot cover this

#176 ("suppress when the user is actively using the app") shipped in 5c38591 and is real:
store/sw.js:25 skips showNotification() when a /console client reports
visibilityState === "visible", and forwards the payload to those tabs so the bell badge still
updates. Verified at HEAD.

It is a per-device decision, made by each device's own service worker. The send is per-user:
sendPushToUser (routes/push.ts:145) selects every subscription row for the user and pushes to
all of them. So the laptop you are looking at goes quiet and the phone in your pocket does not — by
construction, a visibility gate can never cover a device you are not looking at. #176's own closing
comment identified the per-USER vs per-DEVICE tension as a reason not to build a server-side presence
gate, and was right to; it just left the consequence unaddressed.

Three gaps remain, in the order they bite.

1. Nothing knows you caused the event

The deploy notifications that prompted this fire because you pushed the commit. There is no notion
anywhere that the recipient initiated the thing they are being told about. Even with the console
focused and the SW suppressing correctly, being told four times that your own push deployed is not
information.

Cheapest honest version: notifyUser takes an optional actor, and skips the push (keeping the in-app
row) when the actor is the recipient. The deploy watcher has the run's triggering actor available
from the GitHub API and does not currently read it.

2. Push is all-or-nothing

There is no per-type preference of any kind. The push routes are vapid-key, subscribe,
subscribe (DELETE) and test — subscribe or unsubscribe, nothing between. Four types already exist
and are already threaded through as the push tag:

type sites
apply 6
coding 2
deploy 1
trigger (skipped-run notices)

So the vocabulary for "mute deploys, keep handoffs" is already in the payload and simply has no
control attached. A user who finds deploy notifications noisy today has exactly one remedy: turn off
push entirely, which also loses the CAPTCHA handoff — the one notification the product genuinely
needs to interrupt for.

This is the highest value-per-line fix of the three: a per-type mute map on the account, checked in
notifyUser before sendPushToUser, plus checkboxes on Preferences next to the other account-wide
settings.

3. No quiet window, no floor

Nothing bounds the rate. See #359 for how that compounds — a single watcher bug turned ~20 pushes into
40–80 notifications, and nothing in the notification layer resisted it. A per-type mute would have let
the user stop it; a duplicate floor would have bounded it even unmuted.

Suggested shape

  1. Per-type mute on the account (Preferences → Notifications), enforced in notifyUser. Covers
    the reported complaint completely and is independent of everything else.
  2. Actor suppression — do not push to the person who caused it. Small, and removes a whole class
    rather than muting it.
  3. Leave the visibility gate alone. It is correct, it is per-device, and that is the right layer for
    it.

Related: #176 (the shipped visibility gate), #359 (the watcher bug that made this loud).

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    backendBackend / Worker / API workenhancementNew feature or request

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions