fix(notifications): push notification reliability and reaction context#699
Draft
Just-Insane wants to merge 44 commits intoSableClient:devfrom
Draft
fix(notifications): push notification reliability and reaction context#699Just-Insane wants to merge 44 commits intoSableClient:devfrom
Just-Insane wants to merge 44 commits intoSableClient:devfrom
Conversation
…t fallback Matrix access tokens are long-lived and only invalidated on logout or server revocation. The previous 60s TTL caused iOS push handlers (which restart the SW per push) to reject cached sessions as stale, resulting in generic 'New Message' notifications. Also adds a requestSessionWithTimeout fallback in handleMinimalPushPayload that asks live window clients for a fresh session when neither the in-memory map nor the persisted cache contains a usable session.
…ssion from push handler When phase3AdaptiveBackoffJitter is enabled, successful foreground/focus session pushes (phase1ForegroundResync) now reset heartbeatFailuresRef to 0. Previously a period of SW controller absence (e.g. SW update) could inflate the heartbeat interval to its maximum (30 min) even after the SW became healthy again, reducing session-refresh frequency below the intended 10-minute rate. Also captures the loadPersistedSession() result in onPushNotification and assigns it to preloadedSession, avoiding a redundant second cache read in handleMinimalPushPayload when the SW is restarted by iOS for a push event.
When iOS backgrounds the PWA, the WKWebView JS thread can be frozen before visibilitychange fires. This leaves appIsVisible stuck at true and clients.matchAll() returning a stale 'visible' state — both signals stale simultaneously — causing the dual AND gate to wrongly suppress push notifications for backgrounded apps. Replace the stale-flag check with checkLiveVisibility(): ping each window client via postMessage and require a response within 500 ms to confirm the app is genuinely in the foreground. A frozen/backgrounded page cannot respond, so the timeout causes checkLiveVisibility to return false and the notification is shown correctly. The encrypted-event path already uses this pattern (requestDecryptionFromClient acts as the live check) and is unaffected. Also added the matching checkVisibility/visibilityCheckResult message pair to HandleDecryptPushEvent so the page can respond to the new ping.
The postMessage round-trip ping (checkLiveVisibility) introduced a new race: iOS can background the app without immediately freezing the JS thread, so the page can still respond 'visible' in the brief window before the freeze — causing the notification to be suppressed. client.visibilityState from clients.matchAll() is updated by the browser engine when the OS signals a visibility transition, independently of the page JS thread, making it immune to this race. When matchAll() returns zero clients (an iOS Safari PWA quirk) we default to showing the notification rather than silently dropping it. Removes checkLiveVisibility(), visibilityCheckPendingMap, the visibilityCheckResult message handler, and the checkVisibility handler in ClientNonUIFeatures.
…ntVariant to useClientConfig
…chAll() visibilityState
…ndler Restores the dual-signal visibility check in the service worker (appIsVisible flag OR clients.matchAll visibilityState) and the setAppVisible message handler. Also restores the visibilitychange listener in ClientNonUIFeatures that posts visibility state to the SW. These were removed in f79b75e which broke background notification delivery, particularly on iOS Safari where clients.matchAll() can return stale results after SW suspension.
…imers 1. appEvents.ts: Replace single-callback onVisibilityChange/onVisibilityHidden slots with Set-based multi-subscriber pattern. Subscriptions return an unsubscribe function, preventing silent overwrites. 2. useAppVisibility.ts: Update to use emitVisibilityChange/emitVisibilityHidden for dispatching and onVisibilityChange() subscription for togglePusher. 3. BackgroundNotifications.tsx: Track retry setTimeout IDs in a Set and cancel them on effect cleanup, preventing orphaned background clients on unmount.
…e handler - sw.ts: add type validation in loadPersistedSession before accessing fields - sw.ts: remove access token leak from debug log - sw.ts: replace Promise.race with Promise.all+find in handleMinimalPushPayload to avoid returning undefined from first fast-failing client - sw.ts: fix misleading comment about preloadedSession/getAnyStoredSession - sw.ts: add ?? [] fallback for precacheAndRoute(self.__WB_MANIFEST) - ClientRoot: pass activeSession to useAppVisibility - index.tsx: add controllerchange listener to re-push session when SW updates via skipWaiting — fixes notifications stopping after SW replacement
iOS PWA freezes the page thread before visibilitychange fires, leaving appIsVisible stuck at true and suppressing push notifications. Replace the unreliable OR of appIsVisible / matchAll().visibilityState with a live checkVisibility round-trip: the SW posts a ping to every window client and only suppresses if a client confirms visible within 500 ms. Frozen or killed pages cannot respond, so the timeout resolves false and the OS notification fires correctly.
… on this branch Removes imports and usages of usePresenceAutoIdle, presenceAutoIdledAtom, useInitBookmarks, presenceMode setting, and presenceAutoIdleTimeoutMs config that were accidentally merged from other feature branches but don't exist on fix/sw-push-session-recovery. Restores PresenceFeature to upstream dev shape.
…atchAll The checkLiveVisibility approach (postMessage ping with 500ms timeout) was causing false-positive suppression on iOS: the push event itself can briefly wake a suspended page, allowing it to respond with visibilityState='visible' even when the user is not looking at the app. This caused background notifications to silently stop after a period of inactivity. Revert to upstream/dev's approach: OR of appIsVisible flag (set via visibilitychange listener) and clients.matchAll() visibilityState. Remove the checkLiveVisibility function, visibilityCheckPendingMap, and the client-side checkVisibility responder.
- Fix preloadedSession comment: only media fetch handlers use it, not handleMinimalPushPayload - Fix changeset frontmatter: '@sable/client': patch → default: patch
…Visibility Removes the Session dependency from useAppVisibility by deriving the userId directly from the MatrixClient instance.
retryImmediately() is a no-op on SlidingSyncSdk — it returns true without touching the polling loop. Call slidingSync.resend() on foreground/focus to abort a stale long-poll and start a fresh one. Also fixes activeSession references that should use mx methods (getHomeserverUrl/getAccessToken/getUserId).
…dling - Use getEffectiveEvent()?.type for decrypted event type in BackgroundNotifications - Fix isEncryptedRoom flag in pushNotification.ts (was hardcoded false) - Add isEncryptedRoom: true to relay payload when decryption succeeds - Wrap push handlers in try/catch with fallback notifications (prevents silent drops on iOS) - Parallelize requestDecryptionFromClient with Promise.any + shared timeout (was sequential)
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…cations Add sessionSync.phase1ForegroundResync and phase2VisibleHeartbeat to config.json so the service worker session stays fresh on iOS. Without these flags useAppVisibility disables both foreground resync (phase1) and the 10-min visible heartbeat (phase2), leaving the CacheStorage session to age out after 24 h with no refresh. When iOS kills the SW while backgrounded and the session has gone stale, push decryption fails and notifications are silently dropped. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
onPushNotification already fetches the persisted session and stores it in preloadedSession. Thread that through handleMinimalPushPayload's fallback chain so we skip the second cache.match() call on iOS restarts where the in-memory sessions Map is empty. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
When backgrounded, the service worker manages the badge from push payloads. The app's local unread state may be stale before sync catches up, causing the badge to flash on then immediately off. Guard clearAppBadge() with a visibility check so the SW badge persists. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Unregister all service worker registrations before reloading when the user clicks Clear Cache & Reload. On iOS/mobile, stale SWs can persist and serve outdated assets even after an app update; this ensures the next load starts completely fresh. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Add activeRoomIdAtom (synced from all RoomProviders) so BackgroundNotifications can detect when the user is already viewing the notification room. When the room matches and the window is focused, the background handler now returns early — no banner, no OS notification. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Increase jump timeout from 15s to 30s for slow sync catch-up - Always pass eventId on navigation (even after timeout) so the room loads historical context around the notification message instead of dumping the user at live bottom Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
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
1 participant
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.
Summary
Consolidates three push-notification reliability branches. Fixes silent notification drops on mobile (iOS/Android), improves service worker session handling, and ensures reaction notifications include correct room and user context.
Changes
fix/clear-cache-sw-unregister
if (self.__WB_MANIFEST)guard withprecacheAndRoute(self.__WB_MANIFEST ?? [])for Workbox compatibility.fix/reaction-notification-context
roomanduserIdto the reaction notification filter so it can correctly determine whether a reaction notification belongs to the current user.isEncryptedRoomflag inpushNotification.ts(was hardcodedfalse).fix/sw-push-session-recovery
install/activateso push handlers don't have to wait for a fullrequestSessionround-trip.getEffectiveEvent()?.typefor decrypted event type checks; wrap all push handlers in try/catch with a fallback generic notification to prevent silent drops on iOS.requestDecryptionFromClientwithPromise.any+ shared timeout to fan out decryption across multiple open clients simultaneously.requestSessionWithTimeoutfallback.clients.matchAll()visibilityState(live) instead of a stored flag to determine whether to suppress a push notification.appEventsmulti-subscriber: convert to a multi-subscriber pattern and cancel retry timers correctly.experimentConfig,sessionSynctypes anduseExperimentVarianttouseClientConfigfor feature-flag-gated SW sync phases.Testing Checklist
Changeset
fix:Push notification reliability (session recovery, encryption context, silent-drop prevention)fix:Reaction notification contextfeat:Unregister service workers on Clear Cache