From 0adce52d372f6da03f77aa5078f74b7eef68d1e7 Mon Sep 17 00:00:00 2001 From: Jared Vititoe Date: Wed, 1 Jul 2026 22:53:32 -0400 Subject: [PATCH] fix(threads): review-wave fixes for per-thread notifications MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - useRoomsListener now PREPENDS the emitting Room (was appended): the SDK emits RoomEvent.UnreadNotifications with VARIABLE arity (0/1/2 args), so a trailing extra arg landed in the wrong positional slot on the most common room-count sync path — room.isSpaceRoom() threw inside the SDK emit loop and the badge PUT never ran. Both consumers updated (CONFIRMED HIGH review finding). - roomToUnread: SpaceChild RESET now passes the thread prefs so muted-thread subtraction survives space-child state changes. Reviewer also verified: badge subtraction math exact (no double-subtraction), encrypted thread replies caught by the timeline guard (m.relates_to is cleartext), fresh prefs flow to handlers, single-owner wiring load-bearing. Documented-acceptable: hasCurrentUserParticipated can lag until the server bundle refreshes after your first reply; dedupe maps grow per-session only. Co-Authored-By: Claude Opus 4.8 --- src/app/hooks/useRoomsListener.ts | 14 ++++++++------ src/app/pages/client/ClientNonUIFeatures.tsx | 10 ++++++---- src/app/state/room/roomToUnread.ts | 16 ++++++---------- 3 files changed, 20 insertions(+), 20 deletions(-) diff --git a/src/app/hooks/useRoomsListener.ts b/src/app/hooks/useRoomsListener.ts index 33bb80f7c..2f413ac80 100644 --- a/src/app/hooks/useRoomsListener.ts +++ b/src/app/hooks/useRoomsListener.ts @@ -16,16 +16,18 @@ import { * need to memoize it — changing the handler identity never re-attaches the * per-room listeners. * - * The emitting {@link Room} is appended as a FINAL extra argument after the + * The emitting {@link Room} is PREPENDED as the first argument, before the * event's own args: several room-level SDK events (e.g. * `RoomEvent.UnreadNotifications`) don't include the room in their payload, - * which callers need for per-room updates. Handlers that don't care can simply - * ignore it. + * which callers need for per-room updates. Prepending (not appending) is + * load-bearing — some SDK events emit with VARIABLE arity + * (UnreadNotifications fires with 0, 1, or 2 args), so a trailing extra arg + * would land in a different positional slot per emit. */ export function useRoomsListener( mx: MatrixClient, event: E, - handler: (...args: [...Parameters, Room]) => void, + handler: (room: Room, ...args: Parameters) => void, ): void { const handlerRef = useRef(handler); handlerRef.current = handler; @@ -39,9 +41,9 @@ export function useRoomsListener( const attach = (room: Room) => { if (attached.has(room.roomId)) return; // Per-room trampoline: forwards to the current ref value with the - // emitting room appended. + // emitting room PREPENDED (stable slot regardless of emit arity). const roomHandler = (...args: unknown[]) => - (handlerRef.current as (...a: unknown[]) => void)(...args, room); + (handlerRef.current as (...a: unknown[]) => void)(room, ...args); attached.set(room.roomId, roomHandler); // `event`/`roomHandler` are correlated through E but TS can't prove it // for the open generic, so we assert at the boundary. diff --git a/src/app/pages/client/ClientNonUIFeatures.tsx b/src/app/pages/client/ClientNonUIFeatures.tsx index a86cfe66d..750e9d20a 100644 --- a/src/app/pages/client/ClientNonUIFeatures.tsx +++ b/src/app/pages/client/ClientNonUIFeatures.tsx @@ -6,6 +6,7 @@ import { Room, RoomEvent, RoomEventHandlerMap, + Thread, ThreadEvent, } from 'matrix-js-sdk'; import { focusAssistActiveAtom } from '../../state/focusAssist'; @@ -446,8 +447,10 @@ function MessageNotifications() { }; }, [mx, notificationSelected, selectedRoomId, deliverNotification]); - const handleNewReply = useCallback( - (thread, mEvent) => { + const handleNewReply = useCallback( + // useRoomsListener prepends the emitting Room; the thread's own room lookup + // below is kept as the authority (identical object in practice). + (_room: Room, thread: Thread, mEvent: MatrixEvent) => { if (mx.getSyncState() !== 'SYNCING') return; const room = mx.getRoom(thread.roomId); if (!room || room.isSpaceRoom()) return; @@ -457,8 +460,7 @@ function MessageNotifications() { // Suppress when the user is actively looking at this thread (or the inbox). if ( document.hasFocus() && - (notificationSelected || - (selectedRoomId === thread.roomId && activeThreadId === thread.id)) + (notificationSelected || (selectedRoomId === thread.roomId && activeThreadId === thread.id)) ) { return; } diff --git a/src/app/state/room/roomToUnread.ts b/src/app/state/room/roomToUnread.ts index c7cf0edd1..4ab31f95a 100644 --- a/src/app/state/room/roomToUnread.ts +++ b/src/app/state/room/roomToUnread.ts @@ -4,7 +4,6 @@ import { IRoomTimelineData, MatrixClient, MatrixEvent, - NotificationCount, Room, RoomEvent, SyncState, @@ -282,18 +281,15 @@ export const useBindRoomToUnreadAtom = (mx: MatrixClient, unreadAtom: typeof roo // RoomEvent.UnreadNotifications is emitted room-level only (never re-emitted // client-side), so the main Timeline pathway misses thread-count changes and - // room badges lag. useRoomsListener appends the emitting Room as the final - // arg, making this a surgical per-room PUT (not a full RESET per emit) with - // muted-thread subtraction re-applied. Room-mute keeps its DELETE semantics. + // room badges lag. useRoomsListener PREPENDS the emitting Room (the SDK emits + // this event with variable arity — 0/1/2 args — so only a leading slot is + // positionally stable), making this a surgical per-room PUT with muted-thread + // subtraction re-applied. Room-mute keeps its DELETE semantics. useRoomsListener( mx, RoomEvent.UnreadNotifications, useCallback( - ( - _unreadNotifications: NotificationCount | undefined, - _threadId: string | undefined, - room: Room, - ) => { + (room: Room) => { if (room.isSpaceRoom()) return; if (getNotificationType(mx, room.roomId) === NotificationType.Mute) { setUnreadAtom({ type: 'DELETE', roomId: room.roomId }); @@ -333,7 +329,7 @@ export const useBindRoomToUnreadAtom = (mx: MatrixClient, unreadAtom: typeof roo if (mEvent.getType() === StateEvent.SpaceChild) { setUnreadAtom({ type: 'RESET', - unreadInfos: getUnreadInfos(mx), + unreadInfos: getUnreadInfos(mx, threadNotificationsRef.current), }); } },