From be528c8d6f9cde183c5040b7e2d91d9ed530d05a Mon Sep 17 00:00:00 2001
From: Evie Gauthier <evie@gauthier.id>
Date: Fri, 8 May 2026 08:50:07 -0400
Subject: [PATCH] fix(notifications): prevent push dropout after SW restart
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

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.
---
 src/app/hooks/useAppVisibility.ts | 11 +++++++----
 src/sw.ts                         | 17 ++++++++++++++---
 2 files changed, 21 insertions(+), 7 deletions(-)

diff --git a/src/app/hooks/useAppVisibility.ts b/src/app/hooks/useAppVisibility.ts
index b56f564ca..cc35418f1 100644
--- a/src/app/hooks/useAppVisibility.ts
+++ b/src/app/hooks/useAppVisibility.ts
@@ -7,7 +7,6 @@ import { useClientConfig } from './useClientConfig';
 import { useSetting } from '../state/hooks/settings';
 import { settingsAtom } from '../state/settings';
 import { pushSubscriptionAtom } from '../state/pushSubscription';
-import { mobileOrTablet } from '../utils/user-agent';
 import { createDebugLogger } from '../utils/debugLogger';
 
 const debugLog = createDebugLogger('AppVisibility');
@@ -16,7 +15,6 @@ export function useAppVisibility(mx: MatrixClient | undefined) {
   const clientConfig = useClientConfig();
   const [usePushNotifications] = useSetting(settingsAtom, 'usePushNotifications');
   const pushSubAtom = useAtom(pushSubscriptionAtom);
-  const isMobile = mobileOrTablet();
 
   useEffect(() => {
     const handleVisibilityChange = () => {
@@ -43,12 +41,17 @@ export function useAppVisibility(mx: MatrixClient | undefined) {
     if (!mx) return undefined;
 
     const handleVisibilityForNotifications = (isVisible: boolean) => {
-      togglePusher(mx, clientConfig, isVisible, usePushNotifications, pushSubAtom, isMobile);
+      // Always keep the pusher registered regardless of visibility — the SW's
+      // hasVisibleClient check handles OS-notification suppression when the app
+      // 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);
     };
 
     appEvents.onVisibilityChange = handleVisibilityForNotifications;
     return () => {
       appEvents.onVisibilityChange = null;
     };
-  }, [mx, clientConfig, usePushNotifications, pushSubAtom, isMobile]);
+  }, [mx, clientConfig, usePushNotifications, pushSubAtom]);
 }
diff --git a/src/sw.ts b/src/sw.ts
index 222156e72..5ec254e66 100644
--- a/src/sw.ts
+++ b/src/sw.ts
@@ -763,10 +763,21 @@ const onPushNotification = async (event: PushEvent) => {
 
   // If the app is open and visible, skip the OS push notification — the in-app
   // pill notification handles the alert instead.
-  // Combine clients.matchAll() visibility with the explicit appIsVisible flag
-  // because iOS Safari PWA often returns empty or stale results from matchAll().
+  //
+  // Require BOTH the explicit appIsVisible flag AND a visible client from
+  // matchAll() before suppressing.  appIsVisible resets to false every time the
+  // SW starts fresh; on iOS the browser kills the SW between pushes, so on the
+  // next push appIsVisible is always false — we never suppress on a cold SW
+  // restart, which prevents the "notifications stop after a while" bug where
+  // stale matchAll() data (visibilityState stuck at 'visible') would cause all
+  // subsequent notifications to be silently dropped.
+  //
+  // When matchAll() returns zero clients (iOS Safari PWA fully-suspended quirk),
+  // clients.some() returns false — do NOT suppress.  Better to show a duplicate
+  // (handled gracefully by the in-app banner) than to silently drop a
+  // notification while the app is backgrounded.
   const hasVisibleClient =
-    appIsVisible || clients.some((client) => client.visibilityState === 'visible');
+    appIsVisible && clients.some((client) => client.visibilityState === 'visible');
   console.debug(
     '[SW push] appIsVisible:',
     appIsVisible,