fix(notifications): safe thread receipts on mark-read (fixes read-receipt regression)
CI / Build & Quality Checks (push) Failing after 35m56s
CI / Trigger Desktop Build (push) Has been skipped

The prior thread-receipt change (8192da5a) broke read receipts globally. Exact
cause: markAsRead used `thread.lastReply() ?? thread.rootEvent`. When a thread's
replies weren't loaded (lastReply() null — common on room open), it sent a
receipt for the thread ROOT. Since roots are "in the main timeline",
threadIdForReceipt() makes that a MAIN receipt at an old event; when the root
isn't in the loaded timeline the SDK's backward-guard falls back to timestamp
and applies it, moving the main read receipt onto an event we don't have, so
getEventReadUpTo() returns null and roomHaveUnread() reports the room unread —
re-broken on every mark-read, amplified by the bulk mark-all-orphan-rooms-read
callers.

Fix: main unthreaded receipt unchanged; the thread loop now sends a threaded
receipt ONLY for a genuine loaded thread reply (thread.lastReply()), never the
root — if replies aren't loaded, skip. New notifications.test.ts locks the
regression (null lastReply → no root receipt) + the main/threaded/no-op cases.

Gates: tsc/eslint/prettier clean, build OK, 672 tests (7 new).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
2026-07-02 17:09:28 -04:00
parent 8192da5a12
commit abd0753148
3 changed files with 146 additions and 11 deletions
+3 -2
View File
@@ -677,8 +677,9 @@ Run the axe DevTools extension (or Lighthouse → Accessibility) on a room view,
**Unread dot on federated rooms + avatar-decoration console storm (2026-07):**
- Open a room from another homeserver that has thread activity; read it → the room's unread **dot clears** (previously an unread _thread reply_ kept the dot because `markAsRead` only sent an unthreaded receipt at the main-timeline tail). Also confirm opening a thread + reading it clears its part of the badge.
- With DevTools console open on those rooms, the `io.lotus.avatar_decoration` `403`/`502` (and federated media) errors should **not** repeat on every scroll/mount — each failing user is now requested at most ~twice per session, so the storm (and its homeserver load) is gone.
- **Read receipts (regression guard — highest priority):** open several rooms and open the Home/Direct tabs (which mark all orphan rooms read on mount) → rooms **stay read**, unread dots clear and don't come back. (A prior attempt sent a receipt for the thread _root_ when a thread's replies weren't loaded, which the SDK treats as a main receipt at an old event and re-unread every room on every mark-read. Fixed + locked by `notifications.test.ts`.)
- **Thread dot:** a room with an unread reply in a thread whose replies are loaded → its dot clears on read; for a thread not yet loaded, the dot clears once you open/load the thread. (mark-as-read now sends a threaded receipt only for a genuine loaded reply, never the root.)
- With DevTools console open on federated rooms, the `io.lotus.avatar_decoration` `403`/`502` (and federated media) errors should **not** repeat on every scroll/mount — each failing user is now requested at most ~twice per session, so the storm (and its homeserver load) is gone.
**Custom Window Chrome (Beta) fix (2026-07):** on the desktop build, Settings → General → toggle **Custom Window Chrome** — it should reload and come up with the Lotus title bar and a normal, stable feed (no screen-expand / auto-scroll-into-the-past). Toggle back off → reloads to the native frame.
+126
View File
@@ -0,0 +1,126 @@
import { test } from 'node:test';
import assert from 'node:assert/strict';
import { NotificationCountType, ReceiptType } from 'matrix-js-sdk';
import { markAsRead } from './notifications';
// markAsRead sends an unthreaded read receipt at the latest main-timeline event,
// plus a THREADED receipt at each unread thread's latest loaded reply. The
// regression these tests guard against: a thread whose replies aren't loaded
// (lastReply() === null) must NOT produce a receipt for the thread root — that
// resolves to a MAIN receipt at an old event and permanently unreads the room.
type ReceiptCall = { eventId: string; receiptType: ReceiptType; unthreaded?: boolean };
const evt = (id: string, sending = false) => ({ getId: () => id, isSending: () => sending }) as any;
const thread = (id: string, lastReply: any) => ({ id, lastReply: () => lastReply }) as any;
type RoomOpts = {
timeline?: any[];
readUpTo?: string | null;
threads?: any[];
threadUnread?: Record<string, number>;
};
const setup = (opts: RoomOpts) => {
const calls: ReceiptCall[] = [];
const room = {
getLiveTimeline: () => ({ getEvents: () => opts.timeline ?? [] }),
getEventReadUpTo: () => opts.readUpTo ?? null,
getThreads: () => opts.threads ?? [],
getThreadUnreadNotificationCount: (threadId: string, _type: NotificationCountType) =>
opts.threadUnread?.[threadId] ?? 0,
};
const mx = {
getRoom: () => room,
getUserId: () => '@me:server',
sendReadReceipt: async (event: any, receiptType: ReceiptType, unthreaded?: boolean) => {
calls.push({ eventId: event.getId(), receiptType, unthreaded });
return {};
},
} as any;
return { mx, calls };
};
test('main timeline: unthreaded receipt at the latest event', async () => {
const { mx, calls } = setup({ timeline: [evt('a'), evt('b'), evt('c')], readUpTo: 'a' });
await markAsRead(mx, '!r:server', false);
assert.equal(calls.length, 1);
assert.deepEqual(calls[0], { eventId: 'c', receiptType: ReceiptType.Read, unthreaded: true });
});
test('REGRESSION: an unread thread with unloaded replies (lastReply null) sends NO root receipt', async () => {
const t = thread('$root', null); // replies not loaded
const { mx, calls } = setup({
timeline: [evt('a'), evt('b')],
readUpTo: 'a',
threads: [t],
threadUnread: { $root: 3 },
});
await markAsRead(mx, '!r:server', false);
// Only the main unthreaded receipt — never a receipt for the thread root.
assert.equal(calls.length, 1);
assert.equal(calls[0].eventId, 'b');
assert.equal(calls[0].unthreaded, true);
assert.ok(!calls.some((c) => c.eventId === '$root'));
});
test('unread thread with a loaded reply sends a threaded receipt at that reply', async () => {
const t = thread('$root', evt('$reply'));
const { mx, calls } = setup({
timeline: [evt('a'), evt('b')],
readUpTo: 'a',
threads: [t],
threadUnread: { $root: 1 },
});
await markAsRead(mx, '!r:server', false);
const main = calls.find((c) => c.eventId === 'b');
const threaded = calls.find((c) => c.eventId === '$reply');
assert.ok(main && main.unthreaded === true);
assert.ok(threaded && threaded.unthreaded === false);
assert.equal(calls.length, 2);
});
test('main already read but a thread is unread: no main receipt, threaded receipt only', async () => {
const t = thread('$root', evt('$reply'));
const { mx, calls } = setup({
timeline: [evt('a'), evt('b')],
readUpTo: 'b', // latest main event already read → getLatestValidEvent() null
threads: [t],
threadUnread: { $root: 2 },
});
await markAsRead(mx, '!r:server', false);
assert.equal(calls.length, 1);
assert.equal(calls[0].eventId, '$reply');
assert.equal(calls[0].unthreaded, false);
});
test('everything read: no receipts sent', async () => {
const t = thread('$root', evt('$reply'));
const { mx, calls } = setup({
timeline: [evt('a'), evt('b')],
readUpTo: 'b',
threads: [t],
threadUnread: { $root: 0 }, // thread read too
});
await markAsRead(mx, '!r:server', false);
assert.equal(calls.length, 0);
});
test('sending thread reply is skipped', async () => {
const t = thread('$root', evt('$reply', true)); // isSending → skip
const { mx, calls } = setup({
timeline: [evt('a'), evt('b')],
readUpTo: 'b',
threads: [t],
threadUnread: { $root: 1 },
});
await markAsRead(mx, '!r:server', false);
assert.equal(calls.length, 0);
});
test('private receipt flag uses ReadPrivate', async () => {
const { mx, calls } = setup({ timeline: [evt('a'), evt('b')], readUpTo: 'a' });
await markAsRead(mx, '!r:server', true);
assert.equal(calls[0].receiptType, ReceiptType.ReadPrivate);
});
+17 -9
View File
@@ -25,25 +25,33 @@ export async function markAsRead(mx: MatrixClient, roomId: string, privateReceip
if (latestEvent) {
// Unthreaded receipt: with client threadSupport enabled the SDK would
// otherwise scope this to the main timeline (thread_id: "main"). Unthreaded
// clears the main timeline + every event up to this one, in any thread.
// clears the main timeline + every event up to this one.
await mx.sendReadReceipt(latestEvent, receiptType, true);
}
// ...but a thread reply NEWER than the main-timeline tail is not covered by
// the receipt above (threadSupport moves thread replies out of the main
// timeline), so its per-thread notification count — and the room's unread dot,
// which sums thread counts — would linger even after "reading" the room. Send
// a threaded receipt at the latest reply of every thread that still has unread
// counts. Also runs when the main timeline is already read (latestEvent null).
// Clear per-thread notification counts too — the room's unread dot sums them,
// so an unread thread reply keeps the dot lit even after the main timeline is
// read (threadSupport moves thread replies out of the main timeline, so the
// unthreaded receipt above doesn't necessarily cover them).
//
// CRITICAL: only send for a GENUINE loaded thread reply, via thread.lastReply().
// NEVER fall back to the thread root: a root event is "in the main timeline",
// so sendReadReceipt(root, false) resolves (via threadIdForReceipt) to a MAIN
// receipt at that old root event. If the root isn't in the loaded timeline it
// moves the main read receipt onto an event we don't have -> getEventReadUpTo()
// returns null -> the room is reported unread on every mark-read call (this was
// the P6 regression, amplified by the bulk mark-all-orphan-rooms-read callers).
// If a thread's replies aren't loaded (lastReply() null), just skip it.
const threads = room.getThreads();
await Promise.all(
threads.map((thread) => {
const unread =
room.getThreadUnreadNotificationCount(thread.id, NotificationCountType.Total) ?? 0;
if (unread <= 0) return undefined;
const lastReply = thread.lastReply() ?? thread.rootEvent;
const lastReply = thread.lastReply();
if (!lastReply || lastReply.isSending()) return undefined;
// Threaded receipt (unthreaded = false → the SDK scopes it to this thread).
// Threaded receipt (unthreaded = false → the SDK scopes it to this thread
// via the reply's real threadRootId; it never touches the main marker).
return mx.sendReadReceipt(lastReply, receiptType, false).catch(() => undefined);
}),
);