fix(push/sw): consolidate push dropout + SW session heartbeat fixes#828
Draft
Just-Insane wants to merge 4 commits into
Draft
fix(push/sw): consolidate push dropout + SW session heartbeat fixes#828Just-Insane wants to merge 4 commits into
Just-Insane wants to merge 4 commits into
Conversation
Two root causes for notifications stopping after a while: 1. sw.ts hasVisibleClient used OR logic (appIsVisible || matchAll visible): On iOS the SW is killed between pushes so appIsVisible resets to false on restart. With OR logic, a stale matchAll() result with visibilityState='visible' still set hasVisibleClient=true, silently suppressing every notification after the first. Fix: switch to AND logic so BOTH appIsVisible AND a visible client are required to suppress. A cold-start SW (appIsVisible=false) never suppresses, regardless of stale matchAll() data. 2. useAppVisibility.ts was passing isMobile as keepEnabledWhenVisible, meaning on desktop the pusher was deleted from the homeserver whenever the tab was visible. If the async re-enable in enablePushNotifications didn't complete before the page was torn down, the homeserver was left with no pusher — so no more push notifications until a manual background/foreground cycle. Fix: always pass true for keepEnabledWhenVisible so the pusher stays registered permanently. The SW's hasVisibleClient check handles OS-notification suppression in the foreground; the homeserver never needs to be without a pusher.
…und resync - Pass userId in both pushSessionToSW calls in ClientRoot (was missing) - On tab foreground, immediately resync session to SW - 10-minute interval heartbeat keeps persisted session fresh (prevents iOS from invalidating session while app is backgrounded)
This was referenced May 14, 2026
Contributor
There was a problem hiding this comment.
Pull request overview
This PR consolidates multiple service worker (SW) and web push reliability fixes to reduce notification “dropout” and keep SW session state accurate across restarts (notably on iOS PWAs).
Changes:
- Adjust SW foreground-suppression logic to avoid silently dropping notifications after SW restarts.
- Ensure SW session association is more robust by including
userIdinpushSessionToSWand adding a foreground resync + periodic heartbeat. - Improve push/pusher behavior and cleanup: keep pusher enabled (avoid visibility races), rename
app_display_nameto “Sable”, and unregister SWs duringclearLoginData.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| src/sw.ts | Tightens the criteria for suppressing OS notifications to avoid false “visible” states causing drops after SW restarts. |
| src/client/initMatrix.ts | Adds SW unregistration during clear-login-data flows to ensure a fresh SW on next load. |
| src/app/pages/client/ClientRoot.tsx | Sends userId with SW session messages and adds foreground resync + periodic heartbeat to keep SW session warm. |
| src/app/hooks/useAppVisibility.ts | Changes pusher toggling strategy to keep push enabled regardless of tab visibility (avoid enable/disable races). |
| src/app/features/settings/notifications/PushNotifications.tsx | Updates pusher app_display_name from “Cinny” to “Sable”. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+809
to
+810
| const registrations = await navigator.serviceWorker.getRegistrations(); | ||
| await Promise.all(registrations.map((r) => r.unregister())); |
| // is in the foreground, so we never need to delete the pusher. Keeping | ||
| // it permanently avoids the enable/disable race that can leave the | ||
| // homeserver without a valid pusher after rapid tab-focus changes. | ||
| togglePusher(mx, clientConfig, isVisible, usePushNotifications, pushSubAtom, true); |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Consolidates two related service-worker fix branches into one PR:
From
fix/notification-dropout(was PR #823)From
fix/sw-session-heartbeat(was PR #825)userIdtopushSessionToSWcalls so the SW can associate sessions correctlyapp_display_namefrom "Cinny" to "Sable" in pusher registrationclearLoginDatafor a clean reload on mobileCloses #823, #825