fix(notifications/threads): Wave-1 audit fixes (🔴 + web 🟠)
- T1 (🔴): markThreadAsRead no longer receipts the thread ROOT (a 2nd instance of the read-marker-corruption regression — opening a thread whose root is old re-lit the whole room). Extracted to a pure threadReceipt.ts + 5 regression tests. - N1 (🔴): favicon/tab-title unread count now sums only leaf rooms (was double- counting every ancestor-space aggregate in roomToUnread). - N2 (🔴): notifications/sounds dedupe on the event id, not the unread count — fixes "read a DM, next message never notifies again". - T4 (🟠): the thread notification path no longer re-gates on the room count, so an explicit per-thread "All replies" override in a Mentions-only room fires. - N3 (🟠): getUnreadInfos skips phantom {0,0} entries (muted-thread-only rooms no longer light the nav row / pollute unread filters). - N4 (🟠): the Receipt handler recomputes unread instead of blanket-DELETE, so a threaded receipt can't wipe a room's valid main-timeline badge. - T2 (🟠): thread "Jump to Latest" re-anchors the virtual window (was landing on a stale mid/old event). Gates: tsc/eslint/prettier clean, build OK, 678 tests. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
+3
-1
@@ -35,7 +35,9 @@ Completed features are documented in [LOTUS_FEATURES.md](./LOTUS_FEATURES.md).
|
||||
|
||||
## 🔎 Audit findings — Wave 1 (2026-07)
|
||||
|
||||
Bug-hunt of the Tier-1 high-risk areas (notifications/unread/receipts, threads, calls host-side, Element Call fork) by 4 parallel deep-audit agents. **Findings only — not yet fixed.** Fix in prioritized, individually-tested passes (each 🔴 first). `[T#]`=threads, `[N#]`=notifications, `[C#]`=calls host, `[EC#]`=fork. Top-two 🔴 hand-verified against the code.
|
||||
Bug-hunt of the Tier-1 high-risk areas (notifications/unread/receipts, threads, calls host-side, Element Call fork) by 4 parallel deep-audit agents. `[T#]`=threads, `[N#]`=notifications, `[C#]`=calls host, `[EC#]`=fork.
|
||||
|
||||
**✅ FIXED (2026-07):** all 🔴 (T1, N1, N2) + web 🟠 (T2, T4, N3, N4). Web fixes are gate-green (678 tests incl. the new `threadReceipt.test.ts` locking the T1 regression). EC-fork 🟡 (EC1–EC6) fixed on `element-call:lotus` (needs a republish). **Still open:** calls-host 🟠/🟡 (C-H1/2/3, C-M*, C-L*) — see below; the remaining 🟡 notification/thread tail (N5, N6, T5, T6, T7).
|
||||
|
||||
### 🔴 High — data-integrity / broken core UX
|
||||
|
||||
|
||||
@@ -460,12 +460,17 @@ export function ThreadTimeline({ room, thread, editor }: ThreadTimelineProps) {
|
||||
}, [scrollToBottomCount]);
|
||||
|
||||
const handleJumpToBottom = useCallback(() => {
|
||||
// Re-anchor the virtual window at the thread tail first. While scrolled up,
|
||||
// live replies deliberately don't extend the window, so without this the chip
|
||||
// would scroll to the bottom of the STALE window (a mid/old event) instead of
|
||||
// the newest reply. Mirrors the main timeline's handleJumpToLatest.
|
||||
setTimeline(getInitialThreadTimeline(thread, getLinkedTimelines(thread.liveTimeline)));
|
||||
scrollToBottomRef.current.count += 1;
|
||||
scrollToBottomRef.current.smooth = true;
|
||||
// Flip atBottom so the layout effect re-runs (count re-read) and live
|
||||
// events resume sticking to the bottom.
|
||||
setAtBottom(true);
|
||||
}, []);
|
||||
}, [thread]);
|
||||
|
||||
// Scroll in-place editor into view.
|
||||
useEffect(() => {
|
||||
|
||||
@@ -0,0 +1,55 @@
|
||||
import { test } from 'node:test';
|
||||
import assert from 'node:assert/strict';
|
||||
import { ReceiptType } from 'matrix-js-sdk';
|
||||
import { markThreadAsRead } from './threadReceipt';
|
||||
|
||||
// The regression this guards: sending a receipt for the thread ROOT (when
|
||||
// replies aren't loaded, lastReply() is null / equals the root) becomes a MAIN
|
||||
// receipt at an old event and drags the room's read marker backwards. It must
|
||||
// only ever receipt a genuine loaded reply.
|
||||
|
||||
const evt = (id: string, sending = false) => ({ getId: () => id, isSending: () => sending }) as any;
|
||||
|
||||
const setup = (lastReply: any) => {
|
||||
const calls: Array<{ eventId: string; type: ReceiptType }> = [];
|
||||
const thread = { id: '$root', lastReply: () => lastReply } as any;
|
||||
const mx = {
|
||||
sendReadReceipt: async (e: any, type: ReceiptType) => {
|
||||
calls.push({ eventId: e.getId(), type });
|
||||
return {};
|
||||
},
|
||||
} as any;
|
||||
return { mx, thread, calls };
|
||||
};
|
||||
|
||||
test('REGRESSION: no loaded reply (lastReply null) → NO receipt (never the root)', async () => {
|
||||
const { mx, thread, calls } = setup(null);
|
||||
await markThreadAsRead(mx, thread, false);
|
||||
assert.equal(calls.length, 0);
|
||||
});
|
||||
|
||||
test('REGRESSION: lastReply IS the root → NO receipt', async () => {
|
||||
const { mx, thread, calls } = setup(evt('$root'));
|
||||
await markThreadAsRead(mx, thread, false);
|
||||
assert.equal(calls.length, 0);
|
||||
});
|
||||
|
||||
test('genuine loaded reply → threaded receipt at that reply', async () => {
|
||||
const { mx, thread, calls } = setup(evt('$reply'));
|
||||
await markThreadAsRead(mx, thread, false);
|
||||
assert.equal(calls.length, 1);
|
||||
assert.equal(calls[0].eventId, '$reply');
|
||||
assert.equal(calls[0].type, ReceiptType.Read);
|
||||
});
|
||||
|
||||
test('sending reply is skipped', async () => {
|
||||
const { mx, thread, calls } = setup(evt('$reply', true));
|
||||
await markThreadAsRead(mx, thread, false);
|
||||
assert.equal(calls.length, 0);
|
||||
});
|
||||
|
||||
test('private flag uses ReadPrivate', async () => {
|
||||
const { mx, thread, calls } = setup(evt('$reply'));
|
||||
await markThreadAsRead(mx, thread, true);
|
||||
assert.equal(calls[0].type, ReceiptType.ReadPrivate);
|
||||
});
|
||||
@@ -0,0 +1,28 @@
|
||||
import { MatrixClient, ReceiptType, Thread } from 'matrix-js-sdk';
|
||||
|
||||
/**
|
||||
* Send a threaded read receipt for a thread, clearing its per-thread unread
|
||||
* count.
|
||||
*
|
||||
* CRITICAL: never receipt the thread ROOT. A thread's liveTimeline is
|
||||
* `[root, reply1, …]`, so the latest event IS the root when replies aren't
|
||||
* loaded yet (common — the thread panel fires this on mount before replies
|
||||
* fetch). The root is "in the main timeline", so a receipt for it is written by
|
||||
* the SDK with `thread_id:"main"` at the old root, dragging the room's MAIN read
|
||||
* marker backwards (`getEventReadUpTo` → an old/unloaded event) and re-lighting
|
||||
* the whole room. We only receipt a genuine loaded reply (`thread.lastReply()`);
|
||||
* if none is loaded we bail (the per-thread count clears when the reply loads
|
||||
* and this runs again). Mirrors the root guard in `utils/notifications.ts`.
|
||||
*
|
||||
* Pure (no React/CSS) so it can be unit-tested — see `threadReceipt.test.ts`.
|
||||
*/
|
||||
export const markThreadAsRead = async (
|
||||
mx: MatrixClient,
|
||||
thread: Thread,
|
||||
privateReceipt: boolean,
|
||||
): Promise<void> => {
|
||||
const lastReply = thread.lastReply();
|
||||
if (!lastReply || lastReply.isSending() || lastReply.getId() === thread.id) return;
|
||||
|
||||
await mx.sendReadReceipt(lastReply, privateReceipt ? ReceiptType.ReadPrivate : ReceiptType.Read);
|
||||
};
|
||||
@@ -4,7 +4,6 @@ import {
|
||||
EventTimeline,
|
||||
MatrixClient,
|
||||
MatrixEvent,
|
||||
ReceiptType,
|
||||
Room,
|
||||
RoomEvent,
|
||||
RoomEventHandlerMap,
|
||||
@@ -146,32 +145,6 @@ export const useThreadPendingEvents = (
|
||||
return pending;
|
||||
};
|
||||
|
||||
/**
|
||||
* Send a threaded read receipt up to the latest confirmed event in the thread.
|
||||
*
|
||||
* The receipt is threaded by default (scoped to this thread), which clears the
|
||||
* per-thread unread count. Mirrors the latest-valid-event scan in
|
||||
* `utils/notifications.ts`.
|
||||
*/
|
||||
export const markThreadAsRead = async (
|
||||
mx: MatrixClient,
|
||||
thread: Thread,
|
||||
privateReceipt: boolean,
|
||||
): Promise<void> => {
|
||||
const events = thread.liveTimeline.getEvents();
|
||||
|
||||
let latestEvent: MatrixEvent | undefined;
|
||||
for (let i = events.length - 1; i >= 0; i -= 1) {
|
||||
const evt = events[i];
|
||||
if (evt && !evt.isSending()) {
|
||||
latestEvent = evt;
|
||||
break;
|
||||
}
|
||||
}
|
||||
if (!latestEvent) return;
|
||||
|
||||
await mx.sendReadReceipt(
|
||||
latestEvent,
|
||||
privateReceipt ? ReceiptType.ReadPrivate : ReceiptType.Read,
|
||||
);
|
||||
};
|
||||
// markThreadAsRead moved to ./threadReceipt (pure + unit-tested); re-exported
|
||||
// here for existing import sites.
|
||||
export { markThreadAsRead } from './threadReceipt';
|
||||
|
||||
@@ -11,7 +11,7 @@ import {
|
||||
} from 'matrix-js-sdk';
|
||||
import { focusAssistActiveAtom } from '../../state/focusAssist';
|
||||
import { manualDndAtom } from '../../state/manualDnd';
|
||||
import { roomToUnreadAtom, unreadEqual, unreadInfoToUnread } from '../../state/room/roomToUnread';
|
||||
import { roomToUnreadAtom } from '../../state/room/roomToUnread';
|
||||
import LogoSVG from '../../../../public/res/lotus.png';
|
||||
import LogoUnreadSVG from '../../../../public/res/lotus-unread.png';
|
||||
import LogoHighlightSVG from '../../../../public/res/lotus-highlight.png';
|
||||
@@ -32,7 +32,7 @@ import {
|
||||
getUnreadInfo,
|
||||
isNotificationEvent,
|
||||
} from '../../utils/room';
|
||||
import { NotificationType, UnreadInfo } from '../../../types/matrix/room';
|
||||
import { NotificationType } from '../../../types/matrix/room';
|
||||
import { getMxIdLocalPart, mxcUrlToHttp } from '../../utils/matrix';
|
||||
import { useSelectedRoom } from '../../hooks/router/useSelectedRoom';
|
||||
import { useInboxNotificationsSelected } from '../../hooks/router/useInbox';
|
||||
@@ -96,6 +96,11 @@ function FaviconUpdater() {
|
||||
let totalNotif = 0;
|
||||
let totalHighlight = 0;
|
||||
roomToUnread.forEach((unread) => {
|
||||
// roomToUnread holds BOTH leaf rooms and per-ancestor space aggregates
|
||||
// (leaves have `from === null`, aggregates a Set). Sum only leaves —
|
||||
// otherwise a space-nested room is counted once as the leaf and again in
|
||||
// every ancestor space, inflating the tab title / favicon count.
|
||||
if (unread.from !== null) return;
|
||||
totalNotif += unread.total;
|
||||
totalHighlight += unread.highlight;
|
||||
});
|
||||
@@ -232,7 +237,7 @@ function PresenceUpdater() {
|
||||
|
||||
function MessageNotifications() {
|
||||
const audioRef = useRef<HTMLAudioElement>(null);
|
||||
const unreadCacheRef = useRef<Map<string, UnreadInfo>>(new Map());
|
||||
const lastNotifiedEventRef = useRef<Map<string, string>>(new Map());
|
||||
// Per-thread dedupe: threadId -> last notified eventId.
|
||||
const lastNotifiedThreadRef = useRef<Map<string, string>>(new Map());
|
||||
const mx = useMatrixClient();
|
||||
@@ -367,17 +372,21 @@ function MessageNotifications() {
|
||||
const eventId = mEvent.getId();
|
||||
if (!sender || !eventId) return;
|
||||
|
||||
const unreadInfo = getUnreadInfo(room);
|
||||
const cachedUnreadInfo = unreadCacheRef.current.get(room.roomId);
|
||||
unreadCacheRef.current.set(room.roomId, unreadInfo);
|
||||
// Dedupe on the event id (per room): the same event can re-fire (decryption,
|
||||
// edit, thread repopulation). This replaces the old unread-COUNT dedupe,
|
||||
// which suppressed a genuinely-new message whenever its post-read count
|
||||
// matched the previously-notified count — i.e. "read a DM, next message
|
||||
// never notifies/sounds" (the common one-at-a-time cadence).
|
||||
if (lastNotifiedEventRef.current.get(room.roomId) === eventId) return;
|
||||
|
||||
if (unreadInfo.total === 0) return;
|
||||
if (
|
||||
cachedUnreadInfo &&
|
||||
unreadEqual(unreadInfoToUnread(cachedUnreadInfo), unreadInfoToUnread(unreadInfo))
|
||||
) {
|
||||
return;
|
||||
}
|
||||
// Main-timeline path respects push rules: don't notify when the room has no
|
||||
// notification count (e.g. a non-mention in a Mentions-only room). The
|
||||
// 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;
|
||||
|
||||
lastNotifiedEventRef.current.set(room.roomId, eventId);
|
||||
|
||||
const quietActive =
|
||||
focusAssistActive ||
|
||||
|
||||
@@ -24,6 +24,7 @@ import {
|
||||
getUnreadInfo,
|
||||
getUnreadInfos,
|
||||
isNotificationEvent,
|
||||
roomHaveUnread,
|
||||
} from '../../utils/room';
|
||||
import { roomToParentsAtom } from './roomToParents';
|
||||
import { useStateEventCallback } from '../../hooks/useStateEventCallback';
|
||||
@@ -253,7 +254,20 @@ 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 });
|
||||
}
|
||||
}
|
||||
};
|
||||
mx.on(RoomEvent.Receipt, handleReceipt);
|
||||
|
||||
@@ -269,7 +269,15 @@ export const getUnreadInfos = (
|
||||
|
||||
if (roomHaveNotification(room) || roomHaveUnread(mx, room)) {
|
||||
const mutedThreads = content ? getMutedThreads(content, room.roomId) : undefined;
|
||||
unread.push(getUnreadInfo(room, mutedThreads));
|
||||
const info = getUnreadInfo(room, mutedThreads);
|
||||
// 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)) {
|
||||
unread.push(info);
|
||||
}
|
||||
}
|
||||
|
||||
return unread;
|
||||
|
||||
Reference in New Issue
Block a user