fix(client): harden PWA push resume and SW session recovery

.changeset/fix-pwa-push-resume-recovery.md

@@ -0,0 +1,5 @@
+---
+default: patch
+---
+
+Keep web push registered across visibility changes, and let the service worker suppress OS notifications only when a controlled page proves it is actively foregrounded. This also preserves the resumed-PWA media/session recovery work so push and authenticated fetches do not silently fail after the app is suspended or restored.

src/app/features/settings/notifications/PushNotifications.test.ts

@@ -2,7 +2,11 @@ import { afterEach, describe, expect, it, vi } from 'vitest';
 import type { MatrixClient } from '$types/matrix-sdk';
 
 import type { ClientConfig } from '../../../hooks/useClientConfig';
-import { disablePushNotifications, enablePushNotifications } from './PushNotifications';
+import {
+  disablePushNotifications,
+  enablePushNotifications,
+  reconcilePushNotifications,
+} from './PushNotifications';
 
 vi.mock('@sentry/react', () => ({
   metrics: {
@@ -106,6 +110,40 @@ afterEach(() => {
 });
 
 describe('web push notifications', () => {
+  it('reconciles an enabled push registration at startup when permission is already granted', async () => {
+    const subscription = makeSubscription();
+    installWebPush(subscription);
+    const mx = makeMatrixClient();
+
+    vi.stubGlobal('Notification', { permission: 'granted' });
+
+    await expect(
+      reconcilePushNotifications(mx, clientConfig, [subscription.toJSON(), vi.fn<() => void>()])
+    ).resolves.toBe(true);
+
+    expect(mx.setPusher).toHaveBeenCalledWith(
+      expect.objectContaining({
+        kind: 'http',
+        app_id: 'moe.sable.web',
+        pushkey: 'p256dh-key',
+      })
+    );
+  });
+
+  it('skips startup reconciliation when notification permission is not granted', async () => {
+    const subscription = makeSubscription();
+    installWebPush(subscription);
+    const mx = makeMatrixClient();
+
+    vi.stubGlobal('Notification', { permission: 'default' });
+
+    await expect(
+      reconcilePushNotifications(mx, clientConfig, [subscription.toJSON(), vi.fn<() => void>()])
+    ).resolves.toBe(false);
+
+    expect(mx.setPusher).not.toHaveBeenCalled();
+  });
+
   it('updates the Matrix pusher directly and removes the legacy Sable pusher when reusing a browser subscription', async () => {
     const subscription = makeSubscription();
     const { controllerPostMessage } = installWebPush(subscription);

src/app/features/settings/notifications/PushNotifications.tsx

@@ -138,6 +138,25 @@ export async function requestBrowserNotificationPermission(): Promise<Notificati
   }
 }
 
+export async function reconcilePushNotifications(
+  mx: MatrixClient,
+  clientConfig: ClientConfig,
+  pushSubscriptionAtom: PushSubscriptionState
+): Promise<boolean> {
+  if (isTauri()) return false;
+  if (
+    !('serviceWorker' in navigator) ||
+    !('PushManager' in window) ||
+    !('Notification' in window)
+  ) {
+    return false;
+  }
+  if (window.Notification.permission !== 'granted') return false;
+
+  await enablePushNotifications(mx, clientConfig, pushSubscriptionAtom);
+  return true;
+}
+
 export async function enablePushNotifications(
   mx: MatrixClient,
   clientConfig: ClientConfig,
@@ -304,21 +323,3 @@ export async function deRegisterAllPushers(mx: MatrixClient): Promise<void> {
 
   await Promise.allSettled(deletionPromises);
 }
-
-export async function togglePusher(
-  mx: MatrixClient,
-  clientConfig: ClientConfig,
-  visible: boolean,
-  usePushNotifications: boolean,
-  pushSubscriptionAtom: PushSubscriptionState,
-  keepEnabledWhenVisible = false
-): Promise<void> {
-  if (!usePushNotifications) return;
-
-  if (visible && !keepEnabledWhenVisible) {
-    await disablePushNotifications(mx, clientConfig, pushSubscriptionAtom);
-    return;
-  }
-
-  await enablePushNotifications(mx, clientConfig, pushSubscriptionAtom);
-}

src/app/hooks/useAppVisibility.test.tsx

@@ -8,8 +8,7 @@ import { useAppVisibility } from './useAppVisibility';
 
 const mocks = vi.hoisted(() => ({
   pushSessionToSW: vi.fn<() => void>(),
-  togglePusher: vi.fn<() => Promise<void>>(() => Promise.resolve()),
-  usePushNotifications: false,
+  swPostMessage: vi.fn<() => void>(),
 }));
 
 vi.mock('@sentry/react', () => ({
@@ -28,26 +27,6 @@ vi.mock('../../sw-session', () => ({
   pushSessionToSW: mocks.pushSessionToSW,
 }));
 
-vi.mock('../features/settings/notifications/PushNotifications', () => ({
-  togglePusher: mocks.togglePusher,
-}));
-
-vi.mock('../state/hooks/settings', () => ({
-  useSetting: () => [mocks.usePushNotifications],
-}));
-
-vi.mock('../state/settings', () => ({
-  settingsAtom: {},
-}));
-
-vi.mock('../state/pushSubscription', () => ({
-  pushSubscriptionAtom: {},
-}));
-
-vi.mock('jotai', () => ({
-  useAtom: () => [undefined, vi.fn<() => void>()],
-}));
-
 vi.mock('./useClientConfig', () => ({
   useClientConfig: () => ({}),
   useExperimentVariant: () => ({
@@ -102,19 +81,31 @@ describe('useAppVisibility', () => {
     setVisibilityState('visible');
     setOnline(true);
     mocks.pushSessionToSW.mockClear();
-    mocks.togglePusher.mockClear();
-    mocks.togglePusher.mockImplementation(() => Promise.resolve());
-    mocks.usePushNotifications = false;
+    mocks.swPostMessage.mockClear();
+
+    Object.defineProperty(window.navigator, 'serviceWorker', {
+      configurable: true,
+      value: {
+        controller: null,
+        ready: Promise.resolve({
+          active: {
+            state: 'activated',
+            postMessage: mocks.swPostMessage,
+          },
+        }),
+      },
+    });
   });
 
   afterEach(() => {
     vi.useRealTimers();
+    vi.unstubAllGlobals();
   });
 
   it('does not abort a healthy sliding sync poll on focus', () => {
     const mx = makeClient(SyncState.Syncing);
 
-    renderHook(() => useAppVisibility(mx, session));
+    renderHook(() => useAppVisibility(session));
 
     act(() => {
       window.dispatchEvent(new Event('focus'));
@@ -126,7 +117,7 @@ describe('useAppVisibility', () => {
   it('does not automatically retry when sync becomes degraded', () => {
     const mx = makeClient(SyncState.Syncing);
 
-    renderHook(() => useAppVisibility(mx, session));
+    renderHook(() => useAppVisibility(session));
 
     act(() => {
       mx.emitSyncState(SyncState.Reconnecting);
@@ -144,83 +135,13 @@ describe('useAppVisibility', () => {
   it('does not retry immediately on mount when sync is still connecting', () => {
     const mx = makeClient(SyncState.Reconnecting);
 
-    renderHook(() => useAppVisibility(mx, session));
+    renderHook(() => useAppVisibility(session));
 
     expect(mx.retryImmediately).not.toHaveBeenCalled();
   });
 
-  it('dedupes pusher visibility toggles while visible state is unchanged', () => {
-    mocks.usePushNotifications = true;
-    mocks.togglePusher.mockImplementation(() => new Promise(() => undefined));
-    const mx = makeClient(SyncState.Syncing);
-
-    renderHook(() => useAppVisibility(mx, session));
-
-    expect(mocks.togglePusher).toHaveBeenCalledOnce();
-    expect(mocks.togglePusher).toHaveBeenLastCalledWith(
-      mx,
-      {},
-      true,
-      true,
-      [undefined, expect.any(Function)],
-      false
-    );
-
-    act(() => {
-      window.dispatchEvent(new Event('focus'));
-      vi.advanceTimersByTime(100);
-    });
-
-    expect(mocks.togglePusher).toHaveBeenCalledOnce();
-  });
-
-  it('keeps the latest pusher visibility when toggles settle out of order', async () => {
-    mocks.usePushNotifications = true;
-    const resolveToggles: Array<() => void> = [];
-    mocks.togglePusher.mockImplementation(
-      () =>
-        new Promise<void>((resolve) => {
-          resolveToggles.push(resolve);
-        })
-    );
-    const mx = makeClient(SyncState.Syncing);
-
-    renderHook(() => useAppVisibility(mx, session));
-
-    act(() => {
-      setVisibilityState('hidden');
-      document.dispatchEvent(new Event('visibilitychange'));
-    });
-
-    act(() => {
-      setVisibilityState('visible');
-      document.dispatchEvent(new Event('visibilitychange'));
-    });
-
-    expect(mocks.togglePusher).toHaveBeenCalledTimes(3);
-
-    await act(async () => {
-      resolveToggles[2]?.();
-      await Promise.resolve();
-    });
-
-    await act(async () => {
-      resolveToggles[1]?.();
-      resolveToggles[0]?.();
-      await Promise.resolve();
-    });
-
-    act(() => {
-      window.dispatchEvent(new Event('focus'));
-    });
-
-    expect(mocks.togglePusher).toHaveBeenCalledTimes(3);
-  });
-
   it('does not push the service worker session on focus or online events', () => {
-    const mx = makeClient(SyncState.Syncing);
-
-    renderHook(() => useAppVisibility(mx, session));
+    renderHook(() => useAppVisibility(session));
 
     act(() => {
       window.dispatchEvent(new Event('focus'));
@@ -235,7 +156,7 @@ describe('useAppVisibility', () => {
     const visibilityHandler = vi.fn<(visible: boolean) => void>();
     const unsubscribe = appEvents.onVisibilityChange(visibilityHandler);
 
-    renderHook(() => useAppVisibility(mx, session));
+    renderHook(() => useAppVisibility(session));
 
     act(() => {
       vi.advanceTimersByTime(100);
@@ -252,7 +173,7 @@ describe('useAppVisibility', () => {
     const visibilityHandler = vi.fn<(visible: boolean) => void>();
     const unsubscribe = appEvents.onVisibilityChange(visibilityHandler);
 
-    renderHook(() => useAppVisibility(mx, session));
+    renderHook(() => useAppVisibility(session));
     visibilityHandler.mockClear();
 
     act(() => {
@@ -264,4 +185,35 @@ describe('useAppVisibility', () => {
 
     unsubscribe();
   });
+
+  it('requests a service worker claim when the app becomes visible without a controller', async () => {
+    renderHook(() => useAppVisibility(session));
+
+    act(() => {
+      setVisibilityState('hidden');
+      document.dispatchEvent(new Event('visibilitychange'));
+      setVisibilityState('visible');
+      document.dispatchEvent(new Event('visibilitychange'));
+    });
+
+    await act(async () => {
+      await Promise.resolve();
+    });
+
+    expect(mocks.swPostMessage).toHaveBeenCalledWith({ type: 'CLAIM_CLIENTS' });
+  });
+
+  it('requests a service worker claim on bfcache restore without a controller', async () => {
+    renderHook(() => useAppVisibility(session));
+
+    act(() => {
+      window.dispatchEvent(new PageTransitionEvent('pageshow', { persisted: true }));
+    });
+
+    await act(async () => {
+      await Promise.resolve();
+    });
+
+    expect(mocks.swPostMessage).toHaveBeenCalledWith({ type: 'CLAIM_CLIENTS' });
+  });
 });