fix(unread): stop DM device-verification requests re-lighting as unread
A completed in-room device-verification request is a plain m.room.message
(msgtype m.key.verification.request) that matches the default DM push rule
with no recency gate, so the server/SDK notification count stays > 0 and the
DM re-lights as unread on every fresh sync until the room is opened twice.
Two-part fix:
- Display suppression: getUnreadInfo/getUnreadInfos return {0,0} for a room
whose ENTIRE unread span (tail -> read receipt) is verification-flow events,
via new pure helpers isVerificationFlowEvent + unreadIsOnlyVerification.
Conservative: never suppresses when the read marker is off-window, the tail
is still encrypted, or a highlight is present.
- Durable auto-read: useAutoMarkVerificationRead sends a read receipt covering
the request (the only SDK-durable lever), once per room per session, gated on
the same verification-only predicate so it can never ack a real message.
unreadIsOnlyVerification also rejects any room with an unread thread, because
markAsRead clears every thread unconditionally — otherwise a verification-only
main timeline with a genuine unread thread reply would be hidden/auto-acked.
Reviewed by 5 agents; the thread-scope guard closes the one bug they found.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
@@ -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);
|
||||
|
||||
|
||||
@@ -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<Set<string>>(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]);
|
||||
};
|
||||
@@ -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);
|
||||
};
|
||||
|
||||
@@ -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,
|
||||
),
|
||||
});
|
||||
},
|
||||
|
||||
@@ -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<string, { total: number; highlight: number }> = {},
|
||||
): 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,
|
||||
|
||||
+80
-5
@@ -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<string>): UnreadInfo => {
|
||||
export const getUnreadInfo = (
|
||||
room: Room,
|
||||
mutedThreads?: Set<string>,
|
||||
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<string>): 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);
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user