fix(unread): stop stuck/resurrecting read indicators
CI / Build & Quality Checks (push) Successful in 10m53s
CI / Trigger Desktop Build (push) Successful in 9s

handleReceipt recomputed unread from getUnreadNotificationCount, which is
server-computed and stale on the synchronous synthetic receipt echo (the SDK
only zeroes it immediately when the last event is our own message). Reading
someone else's message therefore PUT the stale non-zero count back -> dot stuck
or resurrected on the ack-sync ordering race. Restore upstream cinny's
optimistic DELETE on our own receipt; the UnreadNotifications listener re-asserts
the accurate badge on the server ack.

Also collapse a {total:0,highlight:0} PUT to a DELETE in the reducer (a present
map entry lights the dot via hasUnread=!!unread, so phantom {0,0} PUTs from the
UnreadNotifications listener left stuck dots).

Mark-as-Unread (MSC2867): clear the flag directly in markAsRead (opening an
already-read room sends no receipt, so the receipt-driven auto-clear never
fired), and gate the receipt auto-clear to main/unthreaded receipts so reading
one thread no longer wipes the whole-room flag.

Tests: 700/700 pass; typecheck + prod build clean.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
2026-07-03 22:07:21 -04:00
parent b5db617bd2
commit f12175e76f
7 changed files with 138 additions and 23 deletions
+4 -2
View File
@@ -62,9 +62,11 @@ Built and gate-green; verify per [LOTUS_TESTING.md](./LOTUS_TESTING.md), then gr
## 🔴 Open — Actionable
### 🐛 Unread/read-receipt flakiness (reported 2026-07, live) — INVESTIGATING
### Unread/read-receipt flakiness (reported 2026-07) — FIXED (pending prod QA)
Room unread indicators are **inconsistent**: reading a message sometimes clears the dot, sometimes leaves it stuck, and **sometimes the unread comes back after it was read** (resurrects). This is the same subsystem as the Wave-1 fixes (T1 `markThreadAsRead` root receipt, N1 favicon double-count, N2 dedupe cache, N4 blanket-DELETE) plus the newly-added **Mark as Unread (MSC2867)** `markedUnreadAtom` — a prime suspect for interference. Look closely at: `state/room/roomToUnread.ts` (handleReceipt recompute vs DELETE, RESET triggers), `utils/notifications.ts markAsRead`, `features/room/thread/threadReceipt.ts`, `state/room/markedUnread.ts` (its Receipt listener + the `unread || markedUnread` dot), and `threadIdForReceipt`/unthreaded-receipt handling. Repro: read a message → dot lingers or returns. Planning session.
Room unread dots were inconsistent: reading a message sometimes cleared the dot, sometimes left it stuck, sometimes it resurrected. Root cause (confirmed by tracing + diffing upstream cinny `dev`): **our own "N4" change.** `handleReceipt` recomputed via `getUnreadInfo`, which reads `room.getUnreadNotificationCount()` — server-computed and **stale on the synchronous synthetic receipt echo** (SDK only zeroes it immediately when the last event is your own message) → it PUT the stale non-zero count back → stuck/resurrecting. Compounded by `hasUnread = !!unread` lighting the dot on any present map entry, incl. phantom `{0,0}` PUTs from our `UnreadNotifications` listener. Plus a Mark-as-Unread (MSC2867) flag that never cleared on opening an already-read room (no receipt → no auto-clear).
**Fix:** `roomToUnread.ts``handleReceipt` reverts to upstream's optimistic `DELETE` on own receipt; reducer collapses `{0,0}` PUT → DELETE. `notifications.ts markAsRead` clears the marked-unread flag directly. `markedUnread.ts onReceipt` gated to main/unthreaded receipts (`myMainReceiptPresent`). Unit tests added; 700/700 pass, typecheck + build clean. Deploy + manual QA (read → dot clears & stays; thread read; mark-unread → open → clears; reconnect no resurrect).
### 🧨 Encryption / E2EE — ⚠️ EXTREME COMPLEXITY · 🧠 PLANNING SESSION REQUIRED
+23 -1
View File
@@ -1,7 +1,7 @@
import { test } from 'node:test';
import assert from 'node:assert/strict';
import { MatrixEvent } from 'matrix-js-sdk';
import { receiptIsMine, setMarkedUnread } from './markedUnread';
import { myMainReceiptPresent, receiptIsMine, setMarkedUnread } from './markedUnread';
// MSC2867 mark-as-unread: reading a room (our own receipt) clears the flag, so
// `receiptIsMine` must detect only OUR receipt and ignore others'. And a write
@@ -33,6 +33,28 @@ test('receiptIsMine: tolerates empty / malformed content', () => {
assert.equal(receiptIsMine(receiptEvent({ $x: {} }), ME), false);
});
// myMainReceiptPresent gates the auto-clear to main-timeline reads, so reading a
// single thread does not wipe the whole-room marked-unread flag.
test('myMainReceiptPresent: true for an unthreaded receipt (no thread_id)', () => {
const event = receiptEvent({ $abc: { 'm.read': { [ME]: { ts: 1 } } } });
assert.equal(myMainReceiptPresent(event, ME), true);
});
test('myMainReceiptPresent: true for a thread_id "main" receipt', () => {
const event = receiptEvent({ $abc: { 'm.read': { [ME]: { ts: 1, thread_id: 'main' } } } });
assert.equal(myMainReceiptPresent(event, ME), true);
});
test('myMainReceiptPresent: false for a thread-scoped receipt', () => {
const event = receiptEvent({ $abc: { 'm.read': { [ME]: { ts: 1, thread_id: '$root:server' } } } });
assert.equal(myMainReceiptPresent(event, ME), false);
});
test('myMainReceiptPresent: false when only another user has a main receipt', () => {
const event = receiptEvent({ $abc: { 'm.read': { [OTHER]: { ts: 1 } } } });
assert.equal(myMainReceiptPresent(event, ME), false);
});
test('setMarkedUnread writes both the stable and unstable keys with the flag', async () => {
const calls: Array<{ type: string; content: unknown }> = [];
const mx = {
+23 -4
View File
@@ -9,7 +9,7 @@ import { AccountDataEvent } from '../../../types/matrix/accountData';
// flag round-trips across the ecosystem.
const UNSTABLE_MARKED_UNREAD = 'com.famedly.marked_unread';
const readMarkedUnread = (room: Room): boolean => {
export const readMarkedUnread = (room: Room): boolean => {
const stable = room.getAccountData(AccountDataEvent.MarkedUnread)?.getContent()?.unread;
if (typeof stable === 'boolean') return stable;
return room.getAccountData(UNSTABLE_MARKED_UNREAD)?.getContent()?.unread === true;
@@ -41,6 +41,22 @@ export const receiptIsMine = (event: MatrixEvent, userId: string): boolean => {
);
};
// True only when OUR receipt in this event is for the main timeline — either
// unthreaded (no thread_id) or thread_id "main". A receipt scoped to a specific
// thread (thread_id === <threadRootId>) must NOT clear the whole-room marked
// flag, since only that one thread was read.
export const myMainReceiptPresent = (event: MatrixEvent, userId: string): boolean => {
const content = event.getContent();
return Object.keys(content).some((eventId) =>
Object.keys(content[eventId] ?? {}).some((receiptType) => {
const receipt = content[eventId][receiptType]?.[userId];
if (!receipt) return false;
const threadId = (receipt as { thread_id?: string }).thread_id;
return threadId === undefined || threadId === 'main';
}),
);
};
export const useBindMarkedUnreadAtom = (mx: MatrixClient, anAtom: typeof markedUnreadAtom) => {
const setAtom = useSetAtom(anAtom);
@@ -65,12 +81,15 @@ export const useBindMarkedUnreadAtom = (mx: MatrixClient, anAtom: typeof markedU
const onAccountData: RoomEventHandlerMap[RoomEvent.AccountData] = (_event, room) => {
syncRoom(room);
};
// Reading a room clears its marked-unread flag (MSC2867): when our own read
// receipt lands for a room that's currently marked, clear it.
// Reading a room clears its marked-unread flag (MSC2867): when our own
// MAIN-timeline read receipt lands for a room that's currently marked, clear
// it. Gated to main/unthreaded receipts so reading a single thread doesn't
// wipe the whole-room flag. (This also fires for receipts from our other
// devices; the local read path clears via markAsRead in notifications.ts.)
const onReceipt: RoomEventHandlerMap[RoomEvent.Receipt] = (event, room) => {
const myId = mx.getUserId();
if (!myId || !readMarkedUnread(room)) return;
if (receiptIsMine(event, myId)) {
if (myMainReceiptPresent(event, myId)) {
setMarkedUnread(mx, room.roomId, false).catch(() => undefined);
}
};
+17
View File
@@ -116,6 +116,23 @@ test('PUT with unchanged counts is skipped (same map reference)', () => {
assert.equal(before, after);
});
test('PUT of { total: 0, highlight: 0 } removes the room (collapses to DELETE)', () => {
const store = createStore();
store.set(roomToUnreadAtom, { type: 'PUT', unreadInfo: info('!r:s', 3, 1) });
// A phantom zero-count PUT (e.g. UnreadNotifications after the server zeroes
// counts) must clear the entry, not leave a stuck dot.
store.set(roomToUnreadAtom, { type: 'PUT', unreadInfo: info('!r:s', 0, 0) });
assert.equal(get(store).has('!r:s'), false);
});
test('PUT of { 0, 0 } on an absent room is a no-op (same map reference)', () => {
const store = createStore();
const before = get(store);
store.set(roomToUnreadAtom, { type: 'PUT', unreadInfo: info('!r:s', 0, 0) });
assert.equal(before, get(store));
assert.equal(get(store).has('!r:s'), false);
});
// ---------------------------------------------------------------------------
// roomToUnreadAtom: PUT with parent aggregation
// ---------------------------------------------------------------------------
+31 -15
View File
@@ -24,7 +24,6 @@ import {
getUnreadInfo,
getUnreadInfos,
isNotificationEvent,
roomHaveUnread,
} from '../../utils/room';
import { roomToParentsAtom } from './roomToParents';
import { useStateEventCallback } from '../../hooks/useStateEventCallback';
@@ -139,6 +138,27 @@ export const roomToUnreadAtom = atom<RoomToUnread, [RoomToUnreadAction], undefin
}
if (action.type === 'PUT') {
const { unreadInfo } = action;
// A { total: 0, highlight: 0 } entry is still a *present* map key, and the
// nav dot lights on any present entry — so a phantom zero-count PUT (e.g.
// the UnreadNotifications listener firing once the server zeroes counts)
// would leave a stuck dot. Collapse it to a DELETE so a fully-read room
// actually clears. Done before the unreadEqual short-circuit so an
// already-stuck { 0, 0 } gets removed too.
if (unreadInfo.total === 0 && unreadInfo.highlight === 0) {
if (get(baseRoomToUnread).has(unreadInfo.roomId)) {
set(
baseRoomToUnread,
produce(get(baseRoomToUnread), (draftRoomToUnread) =>
deleteUnreadInfo(
draftRoomToUnread,
getAllParents(get(roomToParentsAtom), unreadInfo.roomId),
unreadInfo.roomId,
),
),
);
}
return;
}
const currentUnread = get(baseRoomToUnread).get(unreadInfo.roomId);
if (currentUnread && unreadEqual(currentUnread, unreadInfoToUnread(unreadInfo))) {
// Do not update if unread data has not changes
@@ -256,20 +276,16 @@ export const useBindRoomToUnreadAtom = (mx: MatrixClient, unreadAtom: typeof roo
),
);
if (isMyReceipt) {
// Don't blanket-DELETE the room's unread on any receipt: a THREADED
// receipt (reading one thread) would wipe the room's still-valid
// main-timeline badge, and if the room was already read no
// UnreadNotifications PUT follows to restore it. Recompute instead —
// DELETE only when the room is genuinely fully read.
const info = getUnreadInfo(
room,
getMutedThreads(threadNotificationsRef.current, room.roomId),
);
if (info.total === 0 && info.highlight === 0 && !roomHaveUnread(mx, room)) {
setUnreadAtom({ type: 'DELETE', roomId: room.roomId });
} else {
setUnreadAtom({ type: 'PUT', unreadInfo: info });
}
// Optimistically clear on our own receipt (upstream cinny behavior).
// Do NOT recompute from getUnreadInfo here: getUnreadNotificationCount is
// server-computed and STALE on the synchronous synthetic receipt echo
// (the SDK only zeroes it immediately when the last live event is our own
// message), so recomputing PUTs the stale non-zero count back → the dot
// sticks / resurrects. The RoomEvent.UnreadNotifications listener below
// re-asserts the accurate badge (incl. restoring the main badge after a
// thread read) once the server acks, and a { 0, 0 } PUT collapses to a
// DELETE in the reducer.
setUnreadAtom({ type: 'DELETE', roomId: room.roomId });
}
};
mx.on(RoomEvent.Receipt, handleReceipt);
+32 -1
View File
@@ -20,16 +20,22 @@ type RoomOpts = {
readUpTo?: string | null;
threads?: any[];
threadUnread?: Record<string, number>;
markedUnread?: boolean;
};
const setup = (opts: RoomOpts) => {
const calls: ReceiptCall[] = [];
const accountDataWrites: Array<{ type: string; content: any }> = [];
const room = {
getLiveTimeline: () => ({ getEvents: () => opts.timeline ?? [] }),
getEventReadUpTo: () => opts.readUpTo ?? null,
getThreads: () => opts.threads ?? [],
getThreadUnreadNotificationCount: (threadId: string, _type: NotificationCountType) =>
opts.threadUnread?.[threadId] ?? 0,
getAccountData: (type: string) =>
opts.markedUnread && type === 'm.marked_unread'
? { getContent: () => ({ unread: true }) }
: undefined,
};
const mx = {
getRoom: () => room,
@@ -38,8 +44,12 @@ const setup = (opts: RoomOpts) => {
calls.push({ eventId: event.getId(), receiptType, unthreaded });
return {};
},
setRoomAccountData: async (_roomId: string, type: string, content: any) => {
accountDataWrites.push({ type, content });
return {};
},
} as any;
return { mx, calls };
return { mx, calls, accountDataWrites };
};
test('main timeline: unthreaded receipt at the latest event', async () => {
@@ -107,6 +117,27 @@ test('everything read: no receipts sent', async () => {
assert.equal(calls.length, 0);
});
test('marked-unread + already fully read: clears the flag even though no receipt is sent', async () => {
const { mx, calls, accountDataWrites } = setup({
timeline: [evt('a'), evt('b')],
readUpTo: 'b', // nothing newer → no receipt
markedUnread: true,
});
await markAsRead(mx, '!r:server', false);
assert.equal(calls.length, 0); // no receipt (the stuck-dot case)
// ...but the marked-unread flag is cleared directly (both keys, unread:false)
assert.ok(accountDataWrites.some((w) => w.type === 'm.marked_unread' && w.content.unread === false));
});
test('not marked-unread: markAsRead does not touch account data', async () => {
const { mx, accountDataWrites } = setup({
timeline: [evt('a'), evt('b')],
readUpTo: 'a',
});
await markAsRead(mx, '!r:server', false);
assert.equal(accountDataWrites.length, 0);
});
test('sending thread reply is skipped', async () => {
const t = thread('$root', evt('$reply', true)); // isSending → skip
const { mx, calls } = setup({
+8
View File
@@ -1,11 +1,19 @@
import { MatrixClient, NotificationCountType, ReceiptType } from 'matrix-js-sdk';
import { getSettings } from '../state/settings';
import { readMarkedUnread, setMarkedUnread } from '../state/room/markedUnread';
export async function markAsRead(mx: MatrixClient, roomId: string, privateReceipt: boolean) {
const { privateReadReceipts } = getSettings();
const room = mx.getRoom(roomId);
if (!room) return;
// Reading a room clears an explicit "mark as unread" (MSC2867). The binder's
// receipt-driven auto-clear does NOT fire when the room is already fully read
// (no receipt is sent below in that case), so clear it directly here.
if (readMarkedUnread(room)) {
setMarkedUnread(mx, roomId, false).catch(() => undefined);
}
const receiptType =
privateReceipt || privateReadReceipts ? ReceiptType.ReadPrivate : ReceiptType.Read;