diff --git a/src/app/pages/client/ClientNonUIFeatures.tsx b/src/app/pages/client/ClientNonUIFeatures.tsx index d356a8a7f..e14a1141c 100644 --- a/src/app/pages/client/ClientNonUIFeatures.tsx +++ b/src/app/pages/client/ClientNonUIFeatures.tsx @@ -531,7 +531,7 @@ function MessageNotifications() { // thread path is already gated by shouldNotifyThreadReply, so it must NOT // re-gate on the room count — otherwise an explicit per-thread "All replies" // override in a Mentions-only room is silently dropped. - if (!threadId && getUnreadInfo(room).total === 0) return; + if (!threadId && getUnreadInfo(room, undefined, mx).total === 0) return; lastNotifiedEventRef.current.set(dedupeKey, eventId); diff --git a/src/app/state/hooks/useAutoMarkVerificationRead.ts b/src/app/state/hooks/useAutoMarkVerificationRead.ts new file mode 100644 index 000000000..1b275b740 --- /dev/null +++ b/src/app/state/hooks/useAutoMarkVerificationRead.ts @@ -0,0 +1,58 @@ +import { useEffect, useRef } from 'react'; +import { MatrixClient, MatrixEvent, MatrixEventEvent, Room } from 'matrix-js-sdk'; +import { roomHaveNotification, unreadIsOnlyVerification } from '../../utils/room'; +import { markAsRead } from '../../utils/notifications'; + +/** + * A COMPLETED in-room device-verification request is a plain `m.room.message` + * that permanently keeps a DM's server/SDK notification count > 0 (it matches the + * default DM push rule and there's no recency gate), so the DM re-lights as unread + * on every fresh sync. `getUnreadInfo`'s suppression hides the dot, but the raw + * SDK count stays "dirty" (desktop badge, other consumers) and the SDK re-inflates + * it on every decrypt. The only durable, SDK-supported fix is a read receipt that + * covers the request event. + * + * This hook sends that receipt — but ONLY when a room's ENTIRE unread span is + * verification-flow events (`unreadIsOnlyVerification`), so it can never mark a + * real unread message read. It fires at most once per room per session, after the + * tail decrypts (the count is only attributable to the request post-decryption). + * `markAsRead` honours the user's private-read-receipt setting. + */ +export const useAutoMarkVerificationRead = (mx: MatrixClient): void => { + const doneRef = useRef>(new Set()); + + useEffect(() => { + const done = doneRef.current; + + const maybeMark = (room: Room) => { + const { roomId } = room; + if (done.has(roomId)) return; + if (room.getMyMembership() !== 'join') return; + // Only touch rooms the SDK actually counts as notifying... + if (!roomHaveNotification(room)) return; + // ...and only when the whole unread span is a completed verification. + if (!unreadIsOnlyVerification(room, mx.getUserId())) return; + + done.add(roomId); + markAsRead(mx, roomId, false).catch(() => { + // Let a later decrypt/sweep retry on transient failure. + done.delete(roomId); + }); + }; + + // Sweep once on mount (after initial sync some verification tails are already + // decrypted), then re-check whenever an event decrypts — the count only + // becomes attributable to the verification request once it's decrypted. + mx.getRooms().forEach(maybeMark); + + const onDecrypted = (event: MatrixEvent) => { + const roomId = event.getRoomId(); + const room = roomId ? mx.getRoom(roomId) : null; + if (room) maybeMark(room); + }; + mx.on(MatrixEventEvent.Decrypted, onDecrypted); + return () => { + mx.removeListener(MatrixEventEvent.Decrypted, onDecrypted); + }; + }, [mx]); +}; diff --git a/src/app/state/hooks/useBindAtoms.ts b/src/app/state/hooks/useBindAtoms.ts index 61ddcd2aa..52317981c 100644 --- a/src/app/state/hooks/useBindAtoms.ts +++ b/src/app/state/hooks/useBindAtoms.ts @@ -7,6 +7,7 @@ import { markedUnreadAtom, useBindMarkedUnreadAtom } from '../room/markedUnread' import { roomToParentsAtom, useBindRoomToParentsAtom } from '../room/roomToParents'; import { roomIdToTypingMembersAtom, useBindRoomIdToTypingMembersAtom } from '../typingMembers'; import { threadNotificationsAtom, useBindThreadNotificationsAtom } from '../threadNotifications'; +import { useAutoMarkVerificationRead } from './useAutoMarkVerificationRead'; export const useBindAtoms = (mx: MatrixClient) => { useBindMDirectAtom(mx, mDirectAtom); @@ -16,6 +17,7 @@ export const useBindAtoms = (mx: MatrixClient) => { useBindThreadNotificationsAtom(mx, threadNotificationsAtom); useBindRoomToUnreadAtom(mx, roomToUnreadAtom); useBindMarkedUnreadAtom(mx, markedUnreadAtom); + useAutoMarkVerificationRead(mx); useBindRoomIdToTypingMembersAtom(mx, roomIdToTypingMembersAtom); }; diff --git a/src/app/state/room/roomToUnread.ts b/src/app/state/room/roomToUnread.ts index 3ef52d5b6..535574af8 100644 --- a/src/app/state/room/roomToUnread.ts +++ b/src/app/state/room/roomToUnread.ts @@ -254,6 +254,7 @@ export const useBindRoomToUnreadAtom = (mx: MatrixClient, unreadAtom: typeof roo unreadInfo: getUnreadInfo( room, getMutedThreads(threadNotificationsRef.current, room.roomId), + mx, ), }); }; @@ -332,6 +333,7 @@ export const useBindRoomToUnreadAtom = (mx: MatrixClient, unreadAtom: typeof roo unreadInfo: getUnreadInfo( room, getMutedThreads(threadNotificationsRef.current, room.roomId), + mx, ), }); }, diff --git a/src/app/utils/room.test.ts b/src/app/utils/room.test.ts index 2e2cbcceb..b7f0f07cf 100644 --- a/src/app/utils/room.test.ts +++ b/src/app/utils/room.test.ts @@ -3,6 +3,7 @@ import assert from 'node:assert/strict'; import { EventTimeline, JoinRule, + MatrixClient, MatrixEvent, NotificationCountType, Room, @@ -24,6 +25,8 @@ import { isMutedRule, findMutedRule, isNotificationEvent, + isVerificationFlowEvent, + unreadIsOnlyVerification, roomHaveNotification, getUnreadInfo, getRoomIconSrc, @@ -410,6 +413,137 @@ test('getUnreadInfo uses highlight when it exceeds total', () => { assert.deepEqual(getUnreadInfo(room2), { roomId: '!r:y', highlight: 1, total: 7 }); }); +// --- verification-flow unread suppression -------------------------------- + +test('isVerificationFlowEvent', () => { + // the in-room request (m.room.message + verification msgtype) + assert.equal( + isVerificationFlowEvent( + mockEvent({ + getType: () => 'm.room.message', + getContent: () => ({ msgtype: 'm.key.verification.request' }), + }), + ), + true, + ); + // the handshake events (their own m.key.verification.* types) + ['ready', 'start', 'accept', 'key', 'mac', 'done', 'cancel'].forEach((phase) => { + assert.equal( + isVerificationFlowEvent(mockEvent({ getType: () => `m.key.verification.${phase}` })), + true, + ); + }); + // a normal message is not verification flow + assert.equal( + isVerificationFlowEvent( + mockEvent({ getType: () => 'm.room.message', getContent: () => ({ msgtype: 'm.text' }) }), + ), + false, + ); + // a still-encrypted event can't be classified → false (conservative) + assert.equal(isVerificationFlowEvent(mockEvent({ getType: () => 'm.room.encrypted' })), false); +}); + +const mockUnreadRoom = ( + events: MatrixEvent[], + readUpToId: string | null, + counts: { total: number; highlight: number } = { total: 0, highlight: 0 }, + threadCounts: Record = {}, +): Room => + ({ + roomId: '!r:x', + getEventReadUpTo: () => readUpToId, + getLiveTimeline: () => ({ getEvents: () => events }), + getUnreadNotificationCount: (type: NotificationCountType) => + type === NotificationCountType.Total ? counts.total : counts.highlight, + getThreads: () => Object.keys(threadCounts).map((id) => ({ id })), + getThreadUnreadNotificationCount: (threadId: string, type: NotificationCountType) => + type === NotificationCountType.Total + ? (threadCounts[threadId]?.total ?? 0) + : (threadCounts[threadId]?.highlight ?? 0), + }) as unknown as Room; + +const mx = { getUserId: () => '@me:x' } as unknown as MatrixClient; +const verifRequest = (id: string) => + mockEvent({ + getId: () => id, + getType: () => 'm.room.message', + getContent: () => ({ msgtype: 'm.key.verification.request' }), + }); +const verifPhase = (id: string, phase: string) => + mockEvent({ getId: () => id, getType: () => `m.key.verification.${phase}` }); +const textMsg = (id: string) => + mockEvent({ + getId: () => id, + getType: () => 'm.room.message', + getContent: () => ({ msgtype: 'm.text', body: 'hi' }), + }); + +test('unreadIsOnlyVerification: verification-only unread tail → true', () => { + // timeline oldest→newest: [read msg] then the verification handshake at the tail + const events = [textMsg('$read'), verifPhase('$done', 'done'), verifRequest('$req')]; + assert.equal(unreadIsOnlyVerification(mockUnreadRoom(events, '$read'), '@me:x'), true); +}); + +test('unreadIsOnlyVerification: a real unread message in the span → false', () => { + // an unread text message sits between the read marker and the verification tail + const events = [textMsg('$read'), textMsg('$new'), verifRequest('$req')]; + assert.equal(unreadIsOnlyVerification(mockUnreadRoom(events, '$read'), '@me:x'), false); +}); + +test('unreadIsOnlyVerification: read marker off-window → false (conservative)', () => { + // the read marker isn't in the loaded timeline + const events = [verifPhase('$done', 'done'), verifRequest('$req')]; + assert.equal(unreadIsOnlyVerification(mockUnreadRoom(events, '$offwindow'), '@me:x'), false); +}); + +test('unreadIsOnlyVerification: still-encrypted tail → false (conservative)', () => { + const encryptedTail = mockEvent({ getId: () => '$enc', getType: () => 'm.room.encrypted' }); + const events = [textMsg('$read'), encryptedTail]; + assert.equal(unreadIsOnlyVerification(mockUnreadRoom(events, '$read'), '@me:x'), false); +}); + +test('unreadIsOnlyVerification: no userId → false', () => { + const events = [textMsg('$read'), verifRequest('$req')]; + assert.equal(unreadIsOnlyVerification(mockUnreadRoom(events, '$read'), null), false); +}); + +test('unreadIsOnlyVerification: verification-only main tail but a real unread THREAD → false', () => { + // markAsRead clears every thread, so a verification-only main timeline must NOT + // count as "only verification" when a thread still has a genuine unread reply. + const events = [textMsg('$read'), verifRequest('$req')]; + const room = mockUnreadRoom( + events, + '$read', + { total: 2, highlight: 0 }, + { + $thread: { total: 1, highlight: 0 }, + }, + ); + assert.equal(unreadIsOnlyVerification(room, '@me:x'), false); +}); + +test('getUnreadInfo suppresses a verification-only room to {0,0} when mx is passed', () => { + const events = [textMsg('$read'), verifRequest('$req')]; + const room = mockUnreadRoom(events, '$read', { total: 1, highlight: 0 }); + // Without mx, the raw count is trusted (backward compatible). + assert.deepEqual(getUnreadInfo(room), { roomId: '!r:x', highlight: 0, total: 1 }); + // With mx, the verification-only count is suppressed. + assert.deepEqual(getUnreadInfo(room, undefined, mx), { roomId: '!r:x', highlight: 0, total: 0 }); +}); + +test('getUnreadInfo does NOT suppress a highlight (real mention) even if the tail is a verification', () => { + const events = [textMsg('$read'), verifRequest('$req')]; + const room = mockUnreadRoom(events, '$read', { total: 2, highlight: 1 }); + assert.deepEqual(getUnreadInfo(room, undefined, mx), { roomId: '!r:x', highlight: 1, total: 2 }); +}); + +test('getUnreadInfo does NOT suppress when a real message is unread alongside a verification', () => { + const events = [textMsg('$read'), textMsg('$new'), verifRequest('$req')]; + const room = mockUnreadRoom(events, '$read', { total: 1, highlight: 0 }); + assert.deepEqual(getUnreadInfo(room, undefined, mx), { roomId: '!r:x', highlight: 0, total: 1 }); +}); + const mockRoomWithThreadCounts = ( total: number, highlight: number, diff --git a/src/app/utils/room.ts b/src/app/utils/room.ts index aeb91a245..02a59df7a 100644 --- a/src/app/utils/room.ts +++ b/src/app/utils/room.ts @@ -237,6 +237,54 @@ export const isNotificationEvent = (mEvent: MatrixEvent) => { return true; }; +// In-room device verification is a small burst at the tail of a DM: the +// `m.key.verification.request` message plus the `m.key.verification.*` flow +// (ready/start/key/mac/done/cancel). A COMPLETED request keeps the server/SDK +// Total notification count > 0 forever — it's a plain `m.room.message`, so it +// matches the default DM push rule and there's no recency gate — so the DM +// re-lights as unread on every fresh sync. +export const isVerificationFlowEvent = (mEvent: MatrixEvent): boolean => { + // getType() returns the CLEAR type once decrypted; while still encrypted we + // can't tell, so this returns false and callers treat that as "not confirmed". + const eType = mEvent.getType(); + if (eType.startsWith('m.key.verification.')) return true; + if (eType === 'm.room.message') { + return mEvent.getContent().msgtype === VERIFICATION_REQUEST_MSGTYPE; + } + return false; +}; + +// True iff a room's ENTIRE unread span (tail → the user's read receipt) is +// nothing but verification-flow events — i.e. the only "unread" is a completed +// device verification, not a real message. Conservative: returns false when the +// read marker isn't in the loaded timeline (can't confirm the span) or while the +// tail is still encrypted (undecryptable → unknown), so it never suppresses or +// auto-reads a genuine unread message. +export const unreadIsOnlyVerification = (room: Room, userId: string | null): boolean => { + if (!userId) return false; + // A real unread THREAD reply also drives the room's notification count, and + // `markAsRead` clears every thread unconditionally — so a room with ANY unread + // thread is never "only verification": suppressing/auto-reading here would hide + // or wrongly ack a genuine thread reply. Reject before scanning the main timeline. + const hasUnreadThread = room + .getThreads() + .some( + (thread) => room.getThreadUnreadNotificationCount(thread.id, NotificationCountType.Total) > 0, + ); + if (hasUnreadThread) return false; + const readUpToId = room.getEventReadUpTo(userId); + const liveEvents = room.getLiveTimeline().getEvents(); + let sawVerification = false; + for (let i = liveEvents.length - 1; i >= 0; i -= 1) { + const event = liveEvents[i]; + if (!event) return false; + if (event.getId() === readUpToId) return sawVerification; + if (isNotificationEvent(event) && !isVerificationFlowEvent(event)) return false; + if (isVerificationFlowEvent(event)) sawVerification = true; + } + return false; +}; + export const roomHaveNotification = (room: Room): boolean => { const total = room.getUnreadNotificationCount(NotificationCountType.Total); const highlight = room.getUnreadNotificationCount(NotificationCountType.Highlight); @@ -263,7 +311,11 @@ export const roomHaveUnread = (mx: MatrixClient, room: Room) => { return true; }; -export const getUnreadInfo = (room: Room, mutedThreads?: Set): UnreadInfo => { +export const getUnreadInfo = ( + room: Room, + mutedThreads?: Set, + mx?: MatrixClient, +): UnreadInfo => { let total = room.getUnreadNotificationCount(NotificationCountType.Total); let highlight = room.getUnreadNotificationCount(NotificationCountType.Highlight); @@ -280,10 +332,26 @@ export const getUnreadInfo = (room: Room, mutedThreads?: Set): UnreadInf if (highlight < 0) highlight = 0; } + const resolvedTotal = highlight > total ? highlight : total; + + // Suppress a room whose entire unread span is a completed device verification: + // the SDK/server Total stays > 0 forever for the trailing verification-request + // message (default DM push rule, no recency gate), but it isn't real unread. A + // highlight is never a verification request, so only the highlight-free case is + // guarded. Requires `mx` (backward-compatible for callers/tests without it). + if ( + mx && + resolvedTotal > 0 && + highlight === 0 && + unreadIsOnlyVerification(room, mx.getUserId()) + ) { + return { roomId: room.roomId, highlight: 0, total: 0 }; + } + return { roomId: room.roomId, highlight, - total: highlight > total ? highlight : total, + total: resolvedTotal, }; }; @@ -298,13 +366,20 @@ export const getUnreadInfos = ( if (roomHaveNotification(room) || roomHaveUnread(mx, room)) { const mutedThreads = content ? getMutedThreads(content, room.roomId) : undefined; - const info = getUnreadInfo(room, mutedThreads); + const info = getUnreadInfo(room, mutedThreads, mx); // Skip a phantom {0,0} entry: a room whose ONLY unread is a muted thread has // roomHaveNotification true (the server room total includes the muted // thread's count), but getUnreadInfo subtracts it back to zero. Pushing it // would still light the nav row + pollute "unread only" filters. Keep it - // only if there's real unread (count > 0) or a genuine unread marker. - if (info.total > 0 || info.highlight > 0 || roomHaveUnread(mx, room)) { + // only if there's real unread (count > 0) or a genuine unread marker — and + // NOT when the only unread is a completed device verification (roomHaveUnread + // can fall through to `true` for a still-encrypted/off-window verification + // tail, which getUnreadInfo above already suppressed to {0,0}). + if ( + info.total > 0 || + info.highlight > 0 || + (roomHaveUnread(mx, room) && !unreadIsOnlyVerification(room, mx.getUserId())) + ) { unread.push(info); } }