fix(notifications): notification delivery and routing fixes

.changeset/fix-notification-delivery-bugs.md

@@ -0,0 +1,5 @@
+---
+'sable': patch
+---
+
+fix: notification delivery bugs

src/app/hooks/useNotificationJumper.ts

@@ -8,6 +8,7 @@ import { useSyncState } from './useSyncState';
 import { useMatrixClient } from './useMatrixClient';
 import { getCanonicalAliasOrRoomId } from '../utils/matrix';
 import { getDirectRoomPath, getHomeRoomPath, getSpaceRoomPath } from '../pages/pathUtils';
+import { getOrphanParents, guessPerfectParent } from '../utils/room';
 import { roomToParentsAtom } from '../state/room/roomToParents';
 import { createLogger } from '../utils/debug';
 
@@ -62,12 +63,16 @@ export function NotificationJumper() {
         // If the room lives inside a space, route through the space path so
         // SpaceRouteRoomProvider can resolve it — HomeRouteRoomProvider only
         // knows orphan rooms and would show JoinBeforeNavigate otherwise.
-        const parents = roomToParents.get(pending.roomId);
-        const firstParentId = parents && [...parents][0];
-        if (firstParentId) {
+        // Use getOrphanParents + guessPerfectParent (same as useRoomNavigate) so
+        // we always navigate to a root-level space, not a subspace — subspace
+        // paths are not recognised by the router and land on JoinBeforeNavigate.
+        const orphanParents = getOrphanParents(roomToParents, pending.roomId);
+        if (orphanParents.length > 0) {
+          const parentSpace =
+            guessPerfectParent(mx, pending.roomId, orphanParents) ?? orphanParents[0];
           navigate(
             getSpaceRoomPath(
-              getCanonicalAliasOrRoomId(mx, firstParentId),
+              getCanonicalAliasOrRoomId(mx, parentSpace),
               roomIdOrAlias,
               pending.eventId
             )

src/app/pages/client/ClientNonUIFeatures.tsx

@@ -40,6 +40,19 @@ import { mobileOrTablet } from '$utils/user-agent';
 import { getInboxInvitesPath } from '../pathUtils';
 import { BackgroundNotifications } from './BackgroundNotifications';
 
+function clearMediaSessionQuickly(): void {
+  if (!('mediaSession' in navigator)) return;
+  // iOS registers the lock screen media player as a side-effect of
+  // HTMLAudioElement.play(). We delay slightly so iOS has finished updating
+  // the media session before we clear it — clearing too early is a no-op.
+  // We only clear if no real in-app media (video/audio in a room) has since
+  // registered meaningful metadata; if it has, leave it alone.
+  setTimeout(() => {
+    if (navigator.mediaSession.metadata !== null) return;
+    navigator.mediaSession.playbackState = 'none';
+  }, 500);
+}
+
 function SystemEmojiFeature() {
   const [twitterEmoji] = useSetting(settingsAtom, 'twitterEmoji');
 
@@ -157,6 +170,7 @@ function InviteNotifications() {
   const playSound = useCallback(() => {
     const audioElement = audioRef.current;
     audioElement?.play();
+    clearMediaSessionQuickly();
   }, []);
 
   useEffect(() => {
@@ -229,6 +243,7 @@ function MessageNotifications() {
   const playSound = useCallback(() => {
     const audioElement = audioRef.current;
     audioElement?.play();
+    clearMediaSessionQuickly();
   }, []);
 
   useEffect(() => {
@@ -291,8 +306,13 @@ function MessageNotifications() {
       // If neither a loud nor a highlight rule matches, and it's not a DM, nothing to show.
       if (!isHighlightByRule && !loudByRule && !isDM) return;
 
-      // Page hidden: SW (push) handles the OS notification. Nothing to do in-app.
-      if (document.visibilityState !== 'visible') return;
+      // With sliding sync we only load m.room.member/$ME in required_state, so
+      // PushProcessor cannot evaluate the room_member_count == 2 condition on
+      // .m.rule.room_one_to_one.  That rule therefore fails to match, and DM
+      // messages fall through to .m.rule.message which carries no sound tweak —
+      // leaving loudByRule=false.  Treat known DMs as inherently loud so that
+      // the OS notification and badge are consistent with the DM context.
+      const isLoud = loudByRule || isDM;
 
       // Record as notified to prevent duplicate banners (e.g. re-emitted decrypted events).
       notifiedEventsRef.current.add(eventId);
@@ -301,11 +321,55 @@ function MessageNotifications() {
         if (first) notifiedEventsRef.current.delete(first);
       }
 
-      // Page is visible — show the themed in-app notification banner for any
-      // highlighted message (mention / keyword) or loud push rule.
-      // okay fast patch because that showNotifications setting atom is not getting set correctly or something
-      if (mobileOrTablet() && showNotifications && (isHighlightByRule || loudByRule || isDM)) {
-        const isEncryptedRoom = !!getStateEvent(room, StateEvent.RoomEncryption);
+      // On desktop: fire an OS notification whenever system notifications are
+      // enabled and permission is granted — regardless of whether the window is
+      // focused. When the window is also visible the in-app banner fires too,
+      // mirroring the behaviour of apps like Discord.
+      // The whole block is wrapped in try/catch: window.Notification() can throw
+      // in sandboxed environments, browsers with DnD active, or Electron — and
+      // an uncaught exception here would abort the handler before setInAppBanner
+      // is reached, causing in-app notifications to silently vanish too.
+      if (!mobileOrTablet() && showSystemNotifications && notificationPermission('granted')) {
+        try {
+          const isEncryptedRoom = !!getStateEvent(room, StateEvent.RoomEncryption);
+          const avatarMxc =
+            room.getAvatarFallbackMember()?.getMxcAvatarUrl() ?? room.getMxcAvatarUrl();
+          const osPayload = buildRoomMessageNotification({
+            roomName: room.name ?? 'Unknown',
+            roomAvatar: avatarMxc
+              ? (mxcUrlToHttp(mx, avatarMxc, useAuthentication, 96, 96, 'crop') ?? undefined)
+              : undefined,
+            username:
+              getMemberDisplayName(room, sender, nicknamesRef.current) ??
+              getMxIdLocalPart(sender) ??
+              sender,
+            previewText: resolveNotificationPreviewText({
+              content: mEvent.getContent(),
+              eventType: mEvent.getType(),
+              isEncryptedRoom,
+              showMessageContent,
+              showEncryptedMessageContent,
+            }),
+            silent: !notificationSound || !isLoud,
+            eventId,
+          });
+          const noti = new window.Notification(osPayload.title, osPayload.options);
+          const { roomId } = room;
+          noti.onclick = () => {
+            window.focus();
+            setPending({ roomId, eventId, targetSessionId: mx.getUserId() ?? undefined });
+            noti.close();
+          };
+        } catch {
+          // window.Notification unavailable or blocked (sandboxed context, DnD, etc.)
+        }
+      }
+
+      // Everything below requires the page to be visible (in-app UI + audio).
+      if (document.visibilityState !== 'visible') return;
+
+      // Page is visible — show the themed in-app notification banner.
+      if (showNotifications && (isHighlightByRule || loudByRule || isDM)) {
         const avatarMxc =
           room.getAvatarFallbackMember()?.getMxcAvatarUrl() ?? room.getMxcAvatarUrl();
         const roomAvatar = avatarMxc
@@ -316,21 +380,22 @@ function MessageNotifications() {
           getMxIdLocalPart(sender) ??
           sender;
         const content = mEvent.getContent();
+        // Events reaching here are already decrypted (m.room.encrypted is skipped
+        // above). Pass isEncryptedRoom:false so the preview always shows the actual
+        // message body when showMessageContent is enabled.
         const previewText = resolveNotificationPreviewText({
           content: mEvent.getContent(),
           eventType: mEvent.getType(),
-          isEncryptedRoom,
+          isEncryptedRoom: false,
           showMessageContent,
           showEncryptedMessageContent,
         });
 
         // Build a rich ReactNode body using the same HTML parser as the room
-        // timeline — this gives full mxc image transforms, mention pills,
-        // linkify, spoilers, code blocks, etc.
+        // timeline — mxc images, mention pills, linkify, spoilers, code blocks.
         let bodyNode: ReactNode;
         if (
           showMessageContent &&
-          (!isEncryptedRoom || showEncryptedMessageContent) &&
           content.format === 'org.matrix.custom.html' &&
           content.formatted_body
         ) {
@@ -347,7 +412,7 @@ function MessageNotifications() {
           roomAvatar,
           username: resolvedSenderName,
           previewText,
-          silent: !notificationSound,
+          silent: !notificationSound || !isLoud,
           eventId,
         });
         const { roomId } = room;
@@ -371,45 +436,8 @@ function MessageNotifications() {
         });
       }
 
-      // On desktop: also fire an OS notification so the user is alerted even
-      // if the browser window is minimised (respects System Notifications toggle).
-      if (!mobileOrTablet() && showSystemNotifications && notificationPermission('granted')) {
-        const isEncryptedRoom = !!getStateEvent(room, StateEvent.RoomEncryption);
-        const avatarMxc =
-          room.getAvatarFallbackMember()?.getMxcAvatarUrl() ?? room.getMxcAvatarUrl();
-        const osPayload = buildRoomMessageNotification({
-          roomName: room.name ?? 'Unknown',
-          roomAvatar: avatarMxc
-            ? (mxcUrlToHttp(mx, avatarMxc, useAuthentication, 96, 96, 'crop') ?? undefined)
-            : undefined,
-          username:
-            getMemberDisplayName(room, sender, nicknamesRef.current) ??
-            getMxIdLocalPart(sender) ??
-            sender,
-          previewText: resolveNotificationPreviewText({
-            content: mEvent.getContent(),
-            eventType: mEvent.getType(),
-            isEncryptedRoom,
-            showMessageContent,
-            showEncryptedMessageContent,
-          }),
-          // Play sound only if the push rule requests it and the user has sounds enabled.
-          silent: !notificationSound || !loudByRule,
-          eventId,
-        });
-        const noti = new window.Notification(osPayload.title, osPayload.options);
-        const { roomId } = room;
-        noti.onclick = () => {
-          window.focus();
-          setPending({ roomId, eventId, targetSessionId: mx.getUserId() ?? undefined });
-          noti.close();
-        };
-      }
-
       // In-app audio: play whenever notification sounds are enabled.
-      // Not gated on loudByRule — that only controls the OS-level silent flag.
-      // Audio API requires a visible, focused document; skip when hidden.
-      if (document.visibilityState === 'visible' && notificationSound) {
+      if (notificationSound) {
         playSound();
       }
     };
@@ -497,8 +525,6 @@ export function HandleNotificationClick() {
 }
 
 function SyncNotificationSettingsWithServiceWorker() {
-  const [notificationSound] = useSetting(settingsAtom, 'isNotificationSounds');
-  const [usePushNotifications] = useSetting(settingsAtom, 'usePushNotifications');
   const [showMessageContent] = useSetting(settingsAtom, 'showMessageContentInNotifications');
   const [showEncryptedMessageContent] = useSetting(
     settingsAtom,
@@ -524,9 +550,11 @@ function SyncNotificationSettingsWithServiceWorker() {
 
   useEffect(() => {
     if (!('serviceWorker' in navigator)) return;
+    // notificationSoundEnabled is intentionally excluded: push notification sound
+    // is governed by the push rule's tweakSound alone (OS/Sygnal handles it).
+    // The in-app sound setting only controls the in-page <audio> playback above.
     const payload = {
       type: 'setNotificationSettings' as const,
-      notificationSoundEnabled: notificationSound,
       showMessageContent,
       showEncryptedMessageContent,
       clearNotificationsOnRead,
@@ -536,13 +564,7 @@ function SyncNotificationSettingsWithServiceWorker() {
     navigator.serviceWorker.ready.then((registration) => {
       registration.active?.postMessage(payload);
     });
-  }, [
-    notificationSound,
-    usePushNotifications,
-    showMessageContent,
-    showEncryptedMessageContent,
-    clearNotificationsOnRead,
-  ]);
+  }, [showMessageContent, showEncryptedMessageContent, clearNotificationsOnRead]);
 
   return null;
 }

src/sw.ts

@@ -16,7 +16,6 @@
 let showEncryptedMessageContent = false;
 let clearNotificationsOnRead = false;
 const { handlePushNotificationPushData } = createPushNotifications(self, () => ({
-  notificationSoundEnabled,
   showMessageContent,
   showEncryptedMessageContent,
 }));
@@ -297,20 +296,20 @@
   // because iOS Safari PWA often returns empty or stale results from matchAll().
   const hasVisibleClient =
     appIsVisible || clients.some((client) => client.visibilityState === 'visible');
   console.debug(
     '[SW push] appIsVisible:',
     appIsVisible,
     '| clients:',
     clients.map((c) => ({ url: c.url, visibility: c.visibilityState }))
   );
   console.debug('[SW push] hasVisibleClient:', hasVisibleClient);
   if (hasVisibleClient) {
     console.debug('[SW push] suppressing OS notification — app is visible');
     return;
   }
 
   const pushData = event.data.json();
   console.debug('[SW push] raw payload:', JSON.stringify(pushData, null, 2));
 
   try {
     if (typeof pushData?.unread === 'number') {
@@ -350,8 +349,8 @@
   const pushEventId: string | undefined = data?.event_id ?? undefined;
   const isInvite = data?.content?.membership === 'invite';
 
   console.debug('[SW notificationclick] notification data:', JSON.stringify(data, null, 2));
   console.debug('[SW notificationclick] resolved fields:', {
     pushUserId,
     pushRoomId,
     pushEventId,
@@ -383,7 +382,7 @@
     targetUrl = new URL('inbox/notifications/', scope).href;
   }
 
   console.debug('[SW notificationclick] targetUrl:', targetUrl);
 
   event.waitUntil(
     (async () => {

src/sw/pushNotification.ts

@@ -7,7 +7,6 @@ import {
 } from '../app/utils/notificationStyle';
 
 type NotificationSettings = {
-  notificationSoundEnabled: boolean;
   showMessageContent: boolean;
   showEncryptedMessageContent: boolean;
 };
@@ -16,13 +15,10 @@ export const createPushNotifications = (
   self: ServiceWorkerGlobalScope,
   getNotificationSettings: () => NotificationSettings
 ) => {
-  const resolveSilent = (silent: unknown, tweakSound?: unknown): boolean => {
-    if (typeof silent === 'boolean') return silent;
-    // If the push rule doesn't request a sound tweak, the notification should be silent
-    // (no sound), regardless of the user's global sound preference.
-    if (!tweakSound) return true;
-    return !getNotificationSettings().notificationSoundEnabled;
-  };
+  // Push notification sound is always controlled by the OS/device settings.
+  // We never explicitly silence push notifications — the user's device notification
+  // preferences (volume, Do Not Disturb, per-app settings) handle that instead.
+  const resolveSilent = (): boolean => false;
 
   const showNotificationWithData = async (
     title: string,
@@ -73,7 +69,7 @@ export const createPushNotifications = (
         showMessageContent: getNotificationSettings().showMessageContent,
         showEncryptedMessageContent: getNotificationSettings().showEncryptedMessageContent,
       }),
-      silent: resolveSilent(pushData?.silent, pushData?.tweaks?.sound),
+      silent: resolveSilent(),
       eventId: pushData?.event_id,
       recipientId: typeof pushData?.user_id === 'string' ? pushData.user_id : undefined,
       data,
@@ -108,7 +104,7 @@ export const createPushNotifications = (
         showMessageContent: getNotificationSettings().showMessageContent,
         showEncryptedMessageContent: getNotificationSettings().showEncryptedMessageContent,
       }),
-      silent: resolveSilent(pushData?.silent, pushData?.tweaks?.sound),
+      silent: resolveSilent(),
       eventId: pushData?.event_id,
       recipientId: typeof pushData?.user_id === 'string' ? pushData.user_id : undefined,
       data,
@@ -141,7 +137,7 @@ export const createPushNotifications = (
       ...pushData.data,
     };
 
-    await showNotificationWithData('New Invitation', body, data, resolveSilent(pushData?.silent));
+    await showNotificationWithData('New Invitation', body, data, resolveSilent());
   };
 
   const handlePushNotificationPushData = async (pushData: any) => {