fix(unread): clear rooms whose read receipt already covers the tail
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 <noreply@anthropic.com>
This commit is contained in:
@@ -27,6 +27,8 @@ import {
|
|||||||
isNotificationEvent,
|
isNotificationEvent,
|
||||||
isVerificationFlowEvent,
|
isVerificationFlowEvent,
|
||||||
unreadIsOnlyVerification,
|
unreadIsOnlyVerification,
|
||||||
|
readReceiptCoversTail,
|
||||||
|
roomHasUnreadThread,
|
||||||
roomHaveNotification,
|
roomHaveNotification,
|
||||||
getUnreadInfo,
|
getUnreadInfo,
|
||||||
getRoomIconSrc,
|
getRoomIconSrc,
|
||||||
@@ -478,6 +480,12 @@ const textMsg = (id: string) =>
|
|||||||
getType: () => 'm.room.message',
|
getType: () => 'm.room.message',
|
||||||
getContent: () => ({ msgtype: 'm.text', body: 'hi' }),
|
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', () => {
|
test('unreadIsOnlyVerification: verification-only unread tail → true', () => {
|
||||||
// timeline oldest→newest: [read msg] then the verification handshake at the tail
|
// 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 });
|
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 = (
|
const mockRoomWithThreadCounts = (
|
||||||
total: number,
|
total: number,
|
||||||
highlight: number,
|
highlight: number,
|
||||||
|
|||||||
+62
-21
@@ -214,6 +214,11 @@ const NOTIFICATION_EVENT_TYPES = [
|
|||||||
'm.room.encrypted',
|
'm.room.encrypted',
|
||||||
'm.room.member',
|
'm.room.member',
|
||||||
'm.sticker',
|
'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
|
// 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
|
// 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;
|
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
|
// 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
|
// nothing but verification-flow events — i.e. the only "unread" is a completed
|
||||||
// device verification, not a real message. Conservative: returns false when the
|
// 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.
|
// auto-reads a genuine unread message.
|
||||||
export const unreadIsOnlyVerification = (room: Room, userId: string | null): boolean => {
|
export const unreadIsOnlyVerification = (room: Room, userId: string | null): boolean => {
|
||||||
if (!userId) return false;
|
if (!userId) return false;
|
||||||
// A real unread THREAD reply also drives the room's notification count, and
|
if (roomHasUnreadThread(room)) return false;
|
||||||
// `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 readUpToId = room.getEventReadUpTo(userId);
|
||||||
const liveEvents = room.getLiveTimeline().getEvents();
|
const liveEvents = room.getLiveTimeline().getEvents();
|
||||||
let sawVerification = false;
|
let sawVerification = false;
|
||||||
@@ -285,6 +292,34 @@ export const unreadIsOnlyVerification = (room: Room, userId: string | null): boo
|
|||||||
return false;
|
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 => {
|
export const roomHaveNotification = (room: Room): boolean => {
|
||||||
const total = room.getUnreadNotificationCount(NotificationCountType.Total);
|
const total = room.getUnreadNotificationCount(NotificationCountType.Total);
|
||||||
const highlight = room.getUnreadNotificationCount(NotificationCountType.Highlight);
|
const highlight = room.getUnreadNotificationCount(NotificationCountType.Highlight);
|
||||||
@@ -334,16 +369,18 @@ export const getUnreadInfo = (
|
|||||||
|
|
||||||
const resolvedTotal = highlight > total ? highlight : total;
|
const resolvedTotal = highlight > total ? highlight : total;
|
||||||
|
|
||||||
// Suppress a room whose entire unread span is a completed device verification:
|
// Suppress a spurious Total when the room isn't really unread. Two safe cases,
|
||||||
// the SDK/server Total stays > 0 forever for the trailing verification-request
|
// both requiring `mx` (backward-compatible for callers/tests without it) and a
|
||||||
// message (default DM push rule, no recency gate), but it isn't real unread. A
|
// highlight-free count (a real mention must never be hidden):
|
||||||
// highlight is never a verification request, so only the highlight-free case is
|
// 1. the entire unread span is a completed device verification, or
|
||||||
// guarded. Requires `mx` (backward-compatible for callers/tests without it).
|
// 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 (
|
if (
|
||||||
mx &&
|
mx &&
|
||||||
resolvedTotal > 0 &&
|
resolvedTotal > 0 &&
|
||||||
highlight === 0 &&
|
highlight === 0 &&
|
||||||
unreadIsOnlyVerification(room, mx.getUserId())
|
(unreadIsOnlyVerification(room, mx.getUserId()) || readReceiptCoversTail(room, mx.getUserId()))
|
||||||
) {
|
) {
|
||||||
return { roomId: room.roomId, highlight: 0, total: 0 };
|
return { roomId: room.roomId, highlight: 0, total: 0 };
|
||||||
}
|
}
|
||||||
@@ -371,14 +408,18 @@ export const getUnreadInfos = (
|
|||||||
// roomHaveNotification true (the server room total includes the muted
|
// roomHaveNotification true (the server room total includes the muted
|
||||||
// thread's count), but getUnreadInfo subtracts it back to zero. Pushing it
|
// thread's count), but getUnreadInfo subtracts it back to zero. Pushing it
|
||||||
// would still light the nav row + pollute "unread only" filters. Keep 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
|
// only if there's real unread (count > 0) or a genuine unread marker. The
|
||||||
// NOT when the only unread is a completed device verification (roomHaveUnread
|
// unreadIsOnlyVerification/readReceiptCoversTail guards below mirror the
|
||||||
// can fall through to `true` for a still-encrypted/off-window verification
|
// getUnreadInfo suppression: roomHaveUnread returning `true` here already
|
||||||
// tail, which getUnreadInfo above already suppressed to {0,0}).
|
// 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 (
|
if (
|
||||||
info.total > 0 ||
|
info.total > 0 ||
|
||||||
info.highlight > 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);
|
unread.push(info);
|
||||||
}
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user