From d47032a14f9b7503d5a97af652f44b7ef977b161 Mon Sep 17 00:00:00 2001 From: Jared Vititoe Date: Wed, 29 Jul 2026 18:42:32 -0400 Subject: [PATCH] fix(unread): clear rooms whose read receipt already covers the tail MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A room could show a permanent unread that survives every cold start even though the server considers it fully read (notification_count 0, unthreaded read receipt at the tail). matrix-js-sdk's fixNotificationCountOnDecryption only ever INCREMENTS an encrypted room's Total, and addReceipt's auto-clear fires only when the tail event is the user's own — so a count inflated in an earlier state (before a receipt covered the tail, e.g. by a since-corrupted undecryptable event) is never decremented and keeps a genuinely-read room lit. This is aggravated by mixing threaded-receipt clients (Element X) with unthreaded ones (Lotus/Cinny), which split the read marker. Add readReceiptCoversTail(room, userId): walking the live timeline newest→ oldest, if we reach the user's read-receipt event without crossing any notification-worthy event, the room is genuinely read and a lingering Total is suppressed to {0,0} in getUnreadInfo / getUnreadInfos. Safe by construction — a real unread sits AFTER the receipt and stops the walk at isNotificationEvent — and guarded against unread threads (markAsRead clears threads unconditionally) and off-window receipts (can't confirm → don't suppress). Self-correcting: a new message becomes the tail and the walk stops suppressing. Also recognize polls (m.poll.start / msc3381) as notification events so a poll-only unread is never walked past (closes a pre-existing gap in the tail scans), and factor the unread-thread guard into roomHasUnreadThread. Reviewed by 3 agents (false-suppression safety, unread-system regression, SDK behavior): no real unread is hidden for any standard content, no regression to the atom/PUT-DELETE paths, and the fix produces {0,0} for the target scenario and stays resolved. Co-Authored-By: Claude Opus 4.8 --- src/app/utils/room.test.ts | 97 ++++++++++++++++++++++++++++++++++++++ src/app/utils/room.ts | 83 +++++++++++++++++++++++--------- 2 files changed, 159 insertions(+), 21 deletions(-) diff --git a/src/app/utils/room.test.ts b/src/app/utils/room.test.ts index b7f0f07cf..4fb8ea19a 100644 --- a/src/app/utils/room.test.ts +++ b/src/app/utils/room.test.ts @@ -27,6 +27,8 @@ import { isNotificationEvent, isVerificationFlowEvent, unreadIsOnlyVerification, + readReceiptCoversTail, + roomHasUnreadThread, roomHaveNotification, getUnreadInfo, getRoomIconSrc, @@ -478,6 +480,12 @@ const textMsg = (id: string) => getType: () => 'm.room.message', getContent: () => ({ msgtype: 'm.text', body: 'hi' }), }); +const reactionEv = (id: string) => + mockEvent({ getId: () => id, getType: () => 'm.reaction', getContent: () => ({}) }); +const encryptedEv = (id: string) => + mockEvent({ getId: () => id, getType: () => 'm.room.encrypted', getContent: () => ({}) }); +const pollEv = (id: string) => + mockEvent({ getId: () => id, getType: () => 'm.poll.start', getContent: () => ({}) }); test('unreadIsOnlyVerification: verification-only unread tail → true', () => { // timeline oldest→newest: [read msg] then the verification handshake at the tail @@ -544,6 +552,95 @@ test('getUnreadInfo does NOT suppress when a real message is unread alongside a assert.deepEqual(getUnreadInfo(room, undefined, mx), { roomId: '!r:x', highlight: 0, total: 1 }); }); +// --- readReceiptCoversTail (UTD / spurious-count suppression) -------------- + +test('readReceiptCoversTail: receipt on the tail (a reaction) → true', () => { + // The Cool Kids case: a corrupt/undecryptable event sits BEFORE the read + // receipt, and the receipt itself landed on the trailing reaction. + const events = [textMsg('$read'), encryptedEv('$corrupt'), reactionEv('$tail')]; + assert.equal(readReceiptCoversTail(mockUnreadRoom(events, '$tail'), '@me:x'), true); +}); + +test('readReceiptCoversTail: only non-notifiable events after the receipt → true', () => { + const events = [textMsg('$read'), reactionEv('$r1'), verifRequest('$v')]; + assert.equal(readReceiptCoversTail(mockUnreadRoom(events, '$read'), '@me:x'), true); +}); + +test('readReceiptCoversTail: a real unread message after the receipt → false', () => { + const events = [textMsg('$read'), reactionEv('$r'), textMsg('$new')]; + assert.equal(readReceiptCoversTail(mockUnreadRoom(events, '$read'), '@me:x'), false); +}); + +test('readReceiptCoversTail: an unread poll after the receipt → false (polls are content)', () => { + const events = [textMsg('$read'), pollEv('$poll')]; + assert.equal(readReceiptCoversTail(mockUnreadRoom(events, '$read'), '@me:x'), false); +}); + +test('isNotificationEvent recognizes polls (MSC3381)', () => { + assert.equal(isNotificationEvent(pollEv('$p')), true); +}); + +test('readReceiptCoversTail: a still-encrypted message after the receipt → false (conservative)', () => { + const events = [textMsg('$read'), encryptedEv('$enc')]; + assert.equal(readReceiptCoversTail(mockUnreadRoom(events, '$read'), '@me:x'), false); +}); + +test('readReceiptCoversTail: receipt off-window → false', () => { + const events = [reactionEv('$r'), textMsg('$new')]; + assert.equal(readReceiptCoversTail(mockUnreadRoom(events, '$gone'), '@me:x'), false); +}); + +test('readReceiptCoversTail: no receipt / null user → false', () => { + const events = [textMsg('$read'), reactionEv('$tail')]; + assert.equal(readReceiptCoversTail(mockUnreadRoom(events, null), '@me:x'), false); + assert.equal(readReceiptCoversTail(mockUnreadRoom(events, '$tail'), null), false); +}); + +test('readReceiptCoversTail: a genuine unread THREAD blocks suppression → false', () => { + const events = [textMsg('$read'), reactionEv('$tail')]; + const room = mockUnreadRoom( + events, + '$tail', + { total: 2, highlight: 0 }, + { + $thread: { total: 1, highlight: 0 }, + }, + ); + assert.equal(readReceiptCoversTail(room, '@me:x'), false); +}); + +test('roomHasUnreadThread reflects per-thread counts', () => { + const events = [textMsg('$read')]; + assert.equal(roomHasUnreadThread(mockUnreadRoom(events, '$read')), false); + assert.equal( + roomHasUnreadThread( + mockUnreadRoom( + events, + '$read', + { total: 1, highlight: 0 }, + { $t: { total: 1, highlight: 0 } }, + ), + ), + true, + ); +}); + +test('getUnreadInfo suppresses a spurious count when the read receipt covers the tail', () => { + // Inflated Total=1 (undecryptable event) but the receipt is on the trailing + // reaction → the room is genuinely read; suppress to {0,0}. Without mx the raw + // count is trusted (backward compatible). + const events = [textMsg('$read'), encryptedEv('$corrupt'), reactionEv('$tail')]; + const room = mockUnreadRoom(events, '$tail', { total: 1, highlight: 0 }); + assert.deepEqual(getUnreadInfo(room), { roomId: '!r:x', highlight: 0, total: 1 }); + assert.deepEqual(getUnreadInfo(room, undefined, mx), { roomId: '!r:x', highlight: 0, total: 0 }); +}); + +test('getUnreadInfo does NOT suppress a spurious count when a real message is unread past the receipt', () => { + const events = [textMsg('$read'), textMsg('$new')]; + 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 02a59df7a..535c2e3f7 100644 --- a/src/app/utils/room.ts +++ b/src/app/utils/room.ts @@ -214,6 +214,11 @@ const NOTIFICATION_EVENT_TYPES = [ 'm.room.encrypted', 'm.room.member', 'm.sticker', + // Polls (MSC3381) are real content the server can count toward a room's total. + // Recognizing them keeps a poll-only unread from being walked past by the + // read-receipt/tail scans (roomHaveUnread, readReceiptCoversTail). + 'm.poll.start', + 'org.matrix.msc3381.poll.start', ]; // In-room device-verification requests are sent as m.room.message with this // msgtype (the rest of the flow — start/accept/key/mac/done/cancel — uses its own @@ -254,6 +259,17 @@ export const isVerificationFlowEvent = (mEvent: MatrixEvent): boolean => { return false; }; +// True iff the room has any thread carrying a real unread notification. A room's +// server/SDK Total INCLUDES its threads, and `markAsRead` clears every thread +// unconditionally, so any tail-based suppression must bail when a thread is +// genuinely unread — otherwise it would hide (or wrongly ack) a real thread reply. +export const roomHasUnreadThread = (room: Room): boolean => + room + .getThreads() + .some( + (thread) => room.getThreadUnreadNotificationCount(thread.id, NotificationCountType.Total) > 0, + ); + // 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 @@ -262,16 +278,7 @@ export const isVerificationFlowEvent = (mEvent: MatrixEvent): boolean => { // 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; + if (roomHasUnreadThread(room)) return false; const readUpToId = room.getEventReadUpTo(userId); const liveEvents = room.getLiveTimeline().getEvents(); let sawVerification = false; @@ -285,6 +292,34 @@ export const unreadIsOnlyVerification = (room: Room, userId: string | null): boo return false; }; +// True iff the user's read receipt already covers the room's entire notifiable +// tail: walking from the newest live event, we reach the receipt's event without +// crossing any notification-worthy event. In that case there is demonstrably +// nothing real left to read, so a lingering Total > 0 is a spurious SDK count. +// matrix-js-sdk's `fixNotificationCountOnDecryption` only ever INCREMENTS an +// encrypted room's Total, and `addReceipt`'s auto-clear-to-zero only fires when +// the tail event is the user's own — so a count inflated in an earlier state +// (before a receipt covered the tail, e.g. by a since-corrupted/undecryptable +// event) is never decremented and keeps a genuinely-read room lit across cold +// starts. Anchoring on the read receipt is safe: a genuine unread would sit AFTER +// the receipt and stop the walk at `isNotificationEvent`. Conservative: returns +// false when the receipt isn't in the loaded window (can't confirm) or a thread +// is genuinely unread. +export const readReceiptCoversTail = (room: Room, userId: string | null): boolean => { + if (!userId) return false; + if (roomHasUnreadThread(room)) return false; + const readUpToId = room.getEventReadUpTo(userId); + if (!readUpToId) return false; + const liveEvents = room.getLiveTimeline().getEvents(); + for (let i = liveEvents.length - 1; i >= 0; i -= 1) { + const event = liveEvents[i]; + if (!event) return false; + if (event.getId() === readUpToId) return true; + if (isNotificationEvent(event)) return false; + } + return false; +}; + export const roomHaveNotification = (room: Room): boolean => { const total = room.getUnreadNotificationCount(NotificationCountType.Total); const highlight = room.getUnreadNotificationCount(NotificationCountType.Highlight); @@ -334,16 +369,18 @@ export const getUnreadInfo = ( 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). + // Suppress a spurious Total when the room isn't really unread. Two safe cases, + // both requiring `mx` (backward-compatible for callers/tests without it) and a + // highlight-free count (a real mention must never be hidden): + // 1. the entire unread span is a completed device verification, or + // 2. the user's read receipt already covers the whole notifiable tail (the + // SDK re-inflated an encrypted-room count past a receipt that genuinely + // covers everything — e.g. a permanently-undecryptable event). if ( mx && resolvedTotal > 0 && highlight === 0 && - unreadIsOnlyVerification(room, mx.getUserId()) + (unreadIsOnlyVerification(room, mx.getUserId()) || readReceiptCoversTail(room, mx.getUserId())) ) { return { roomId: room.roomId, highlight: 0, total: 0 }; } @@ -371,14 +408,18 @@ export const getUnreadInfos = ( // 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 — 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}). + // only if there's real unread (count > 0) or a genuine unread marker. The + // unreadIsOnlyVerification/readReceiptCoversTail guards below mirror the + // getUnreadInfo suppression: roomHaveUnread returning `true` here already + // implies both are false (they only report `true` once the receipt covers + // the tail, exactly where roomHaveUnread returns `false`), so the guards are + // defensive insurance against divergence, not load-bearing. if ( info.total > 0 || info.highlight > 0 || - (roomHaveUnread(mx, room) && !unreadIsOnlyVerification(room, mx.getUserId())) + (roomHaveUnread(mx, room) && + !unreadIsOnlyVerification(room, mx.getUserId()) && + !readReceiptCoversTail(room, mx.getUserId())) ) { unread.push(info); }