diff --git a/LOTUS_TODO.md b/LOTUS_TODO.md index 4bbb35f7e..7fa9db146 100644 --- a/LOTUS_TODO.md +++ b/LOTUS_TODO.md @@ -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 diff --git a/src/app/features/room/thread/ThreadTimeline.tsx b/src/app/features/room/thread/ThreadTimeline.tsx index 04699d875..5abf02ce8 100644 --- a/src/app/features/room/thread/ThreadTimeline.tsx +++ b/src/app/features/room/thread/ThreadTimeline.tsx @@ -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(() => { diff --git a/src/app/features/room/thread/threadReceipt.test.ts b/src/app/features/room/thread/threadReceipt.test.ts new file mode 100644 index 000000000..f9d367a31 --- /dev/null +++ b/src/app/features/room/thread/threadReceipt.test.ts @@ -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); +}); diff --git a/src/app/features/room/thread/threadReceipt.ts b/src/app/features/room/thread/threadReceipt.ts new file mode 100644 index 000000000..ae029aaef --- /dev/null +++ b/src/app/features/room/thread/threadReceipt.ts @@ -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 => { + const lastReply = thread.lastReply(); + if (!lastReply || lastReply.isSending() || lastReply.getId() === thread.id) return; + + await mx.sendReadReceipt(lastReply, privateReceipt ? ReceiptType.ReadPrivate : ReceiptType.Read); +}; diff --git a/src/app/features/room/thread/useThread.ts b/src/app/features/room/thread/useThread.ts index 0a7bffc1b..4176d497f 100644 --- a/src/app/features/room/thread/useThread.ts +++ b/src/app/features/room/thread/useThread.ts @@ -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 => { - 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'; diff --git a/src/app/pages/client/ClientNonUIFeatures.tsx b/src/app/pages/client/ClientNonUIFeatures.tsx index 0ff25ca47..4e4692fc3 100644 --- a/src/app/pages/client/ClientNonUIFeatures.tsx +++ b/src/app/pages/client/ClientNonUIFeatures.tsx @@ -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(null); - const unreadCacheRef = useRef>(new Map()); + const lastNotifiedEventRef = useRef>(new Map()); // Per-thread dedupe: threadId -> last notified eventId. const lastNotifiedThreadRef = useRef>(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 || diff --git a/src/app/state/room/roomToUnread.ts b/src/app/state/room/roomToUnread.ts index 4ab31f95a..85610e5cc 100644 --- a/src/app/state/room/roomToUnread.ts +++ b/src/app/state/room/roomToUnread.ts @@ -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) { - setUnreadAtom({ type: 'DELETE', roomId: room.roomId }); + // 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); diff --git a/src/app/utils/room.ts b/src/app/utils/room.ts index 69650cfc4..a957328da 100644 --- a/src/app/utils/room.ts +++ b/src/app/utils/room.ts @@ -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;