Compare commits
2 Commits
4ecc173554
...
f12175e76f
| Author | SHA1 | Date | |
|---|---|---|---|
| f12175e76f | |||
| b5db617bd2 |
@@ -62,6 +62,12 @@ Built and gate-green; verify per [LOTUS_TESTING.md](./LOTUS_TESTING.md), then gr
|
||||
|
||||
## 🔴 Open — Actionable
|
||||
|
||||
### ✅ Unread/read-receipt flakiness (reported 2026-07) — FIXED (pending prod QA)
|
||||
|
||||
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
|
||||
|
||||
Observed live in prod 2026-06-30 during a 2-person **Element Call** (E2EE). These span client rust-crypto (`matrix-js-sdk@41.7.0`) ↔ Synapse ↔ EC MatrixRTC E2EE and are **interrelated** — do NOT spot-fix. **Capture first:** run **Settings → Developer Tools → Crypto Diagnostics** during the next affected call + a synapse-side trace before any fix. (Full runbook was in `LOTUS_E2EE_INVESTIGATION.md`, now in git history.) None are caused by the EC fork work.
|
||||
|
||||
@@ -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 = {
|
||||
|
||||
@@ -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);
|
||||
}
|
||||
};
|
||||
|
||||
@@ -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
|
||||
// ---------------------------------------------------------------------------
|
||||
|
||||
@@ -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);
|
||||
|
||||
@@ -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({
|
||||
|
||||
@@ -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;
|
||||
|
||||
|
||||
Reference in New Issue
Block a user