Compare commits
2 Commits
| 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
|
## 🔴 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
|
### 🧨 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.
|
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 { test } from 'node:test';
|
||||||
import assert from 'node:assert/strict';
|
import assert from 'node:assert/strict';
|
||||||
import { MatrixEvent } from 'matrix-js-sdk';
|
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
|
// 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
|
// `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);
|
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 () => {
|
test('setMarkedUnread writes both the stable and unstable keys with the flag', async () => {
|
||||||
const calls: Array<{ type: string; content: unknown }> = [];
|
const calls: Array<{ type: string; content: unknown }> = [];
|
||||||
const mx = {
|
const mx = {
|
||||||
|
|||||||
@@ -9,7 +9,7 @@ import { AccountDataEvent } from '../../../types/matrix/accountData';
|
|||||||
// flag round-trips across the ecosystem.
|
// flag round-trips across the ecosystem.
|
||||||
const UNSTABLE_MARKED_UNREAD = 'com.famedly.marked_unread';
|
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;
|
const stable = room.getAccountData(AccountDataEvent.MarkedUnread)?.getContent()?.unread;
|
||||||
if (typeof stable === 'boolean') return stable;
|
if (typeof stable === 'boolean') return stable;
|
||||||
return room.getAccountData(UNSTABLE_MARKED_UNREAD)?.getContent()?.unread === true;
|
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) => {
|
export const useBindMarkedUnreadAtom = (mx: MatrixClient, anAtom: typeof markedUnreadAtom) => {
|
||||||
const setAtom = useSetAtom(anAtom);
|
const setAtom = useSetAtom(anAtom);
|
||||||
|
|
||||||
@@ -65,12 +81,15 @@ export const useBindMarkedUnreadAtom = (mx: MatrixClient, anAtom: typeof markedU
|
|||||||
const onAccountData: RoomEventHandlerMap[RoomEvent.AccountData] = (_event, room) => {
|
const onAccountData: RoomEventHandlerMap[RoomEvent.AccountData] = (_event, room) => {
|
||||||
syncRoom(room);
|
syncRoom(room);
|
||||||
};
|
};
|
||||||
// Reading a room clears its marked-unread flag (MSC2867): when our own read
|
// Reading a room clears its marked-unread flag (MSC2867): when our own
|
||||||
// receipt lands for a room that's currently marked, clear it.
|
// 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 onReceipt: RoomEventHandlerMap[RoomEvent.Receipt] = (event, room) => {
|
||||||
const myId = mx.getUserId();
|
const myId = mx.getUserId();
|
||||||
if (!myId || !readMarkedUnread(room)) return;
|
if (!myId || !readMarkedUnread(room)) return;
|
||||||
if (receiptIsMine(event, myId)) {
|
if (myMainReceiptPresent(event, myId)) {
|
||||||
setMarkedUnread(mx, room.roomId, false).catch(() => undefined);
|
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);
|
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
|
// roomToUnreadAtom: PUT with parent aggregation
|
||||||
// ---------------------------------------------------------------------------
|
// ---------------------------------------------------------------------------
|
||||||
|
|||||||
@@ -24,7 +24,6 @@ import {
|
|||||||
getUnreadInfo,
|
getUnreadInfo,
|
||||||
getUnreadInfos,
|
getUnreadInfos,
|
||||||
isNotificationEvent,
|
isNotificationEvent,
|
||||||
roomHaveUnread,
|
|
||||||
} from '../../utils/room';
|
} from '../../utils/room';
|
||||||
import { roomToParentsAtom } from './roomToParents';
|
import { roomToParentsAtom } from './roomToParents';
|
||||||
import { useStateEventCallback } from '../../hooks/useStateEventCallback';
|
import { useStateEventCallback } from '../../hooks/useStateEventCallback';
|
||||||
@@ -139,6 +138,27 @@ export const roomToUnreadAtom = atom<RoomToUnread, [RoomToUnreadAction], undefin
|
|||||||
}
|
}
|
||||||
if (action.type === 'PUT') {
|
if (action.type === 'PUT') {
|
||||||
const { unreadInfo } = action;
|
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);
|
const currentUnread = get(baseRoomToUnread).get(unreadInfo.roomId);
|
||||||
if (currentUnread && unreadEqual(currentUnread, unreadInfoToUnread(unreadInfo))) {
|
if (currentUnread && unreadEqual(currentUnread, unreadInfoToUnread(unreadInfo))) {
|
||||||
// Do not update if unread data has not changes
|
// Do not update if unread data has not changes
|
||||||
@@ -256,20 +276,16 @@ export const useBindRoomToUnreadAtom = (mx: MatrixClient, unreadAtom: typeof roo
|
|||||||
),
|
),
|
||||||
);
|
);
|
||||||
if (isMyReceipt) {
|
if (isMyReceipt) {
|
||||||
// Don't blanket-DELETE the room's unread on any receipt: a THREADED
|
// Optimistically clear on our own receipt (upstream cinny behavior).
|
||||||
// receipt (reading one thread) would wipe the room's still-valid
|
// Do NOT recompute from getUnreadInfo here: getUnreadNotificationCount is
|
||||||
// main-timeline badge, and if the room was already read no
|
// server-computed and STALE on the synchronous synthetic receipt echo
|
||||||
// UnreadNotifications PUT follows to restore it. Recompute instead —
|
// (the SDK only zeroes it immediately when the last live event is our own
|
||||||
// DELETE only when the room is genuinely fully read.
|
// message), so recomputing PUTs the stale non-zero count back → the dot
|
||||||
const info = getUnreadInfo(
|
// sticks / resurrects. The RoomEvent.UnreadNotifications listener below
|
||||||
room,
|
// re-asserts the accurate badge (incl. restoring the main badge after a
|
||||||
getMutedThreads(threadNotificationsRef.current, room.roomId),
|
// thread read) once the server acks, and a { 0, 0 } PUT collapses to a
|
||||||
);
|
// DELETE in the reducer.
|
||||||
if (info.total === 0 && info.highlight === 0 && !roomHaveUnread(mx, room)) {
|
setUnreadAtom({ type: 'DELETE', roomId: room.roomId });
|
||||||
setUnreadAtom({ type: 'DELETE', roomId: room.roomId });
|
|
||||||
} else {
|
|
||||||
setUnreadAtom({ type: 'PUT', unreadInfo: info });
|
|
||||||
}
|
|
||||||
}
|
}
|
||||||
};
|
};
|
||||||
mx.on(RoomEvent.Receipt, handleReceipt);
|
mx.on(RoomEvent.Receipt, handleReceipt);
|
||||||
|
|||||||
@@ -20,16 +20,22 @@ type RoomOpts = {
|
|||||||
readUpTo?: string | null;
|
readUpTo?: string | null;
|
||||||
threads?: any[];
|
threads?: any[];
|
||||||
threadUnread?: Record<string, number>;
|
threadUnread?: Record<string, number>;
|
||||||
|
markedUnread?: boolean;
|
||||||
};
|
};
|
||||||
|
|
||||||
const setup = (opts: RoomOpts) => {
|
const setup = (opts: RoomOpts) => {
|
||||||
const calls: ReceiptCall[] = [];
|
const calls: ReceiptCall[] = [];
|
||||||
|
const accountDataWrites: Array<{ type: string; content: any }> = [];
|
||||||
const room = {
|
const room = {
|
||||||
getLiveTimeline: () => ({ getEvents: () => opts.timeline ?? [] }),
|
getLiveTimeline: () => ({ getEvents: () => opts.timeline ?? [] }),
|
||||||
getEventReadUpTo: () => opts.readUpTo ?? null,
|
getEventReadUpTo: () => opts.readUpTo ?? null,
|
||||||
getThreads: () => opts.threads ?? [],
|
getThreads: () => opts.threads ?? [],
|
||||||
getThreadUnreadNotificationCount: (threadId: string, _type: NotificationCountType) =>
|
getThreadUnreadNotificationCount: (threadId: string, _type: NotificationCountType) =>
|
||||||
opts.threadUnread?.[threadId] ?? 0,
|
opts.threadUnread?.[threadId] ?? 0,
|
||||||
|
getAccountData: (type: string) =>
|
||||||
|
opts.markedUnread && type === 'm.marked_unread'
|
||||||
|
? { getContent: () => ({ unread: true }) }
|
||||||
|
: undefined,
|
||||||
};
|
};
|
||||||
const mx = {
|
const mx = {
|
||||||
getRoom: () => room,
|
getRoom: () => room,
|
||||||
@@ -38,8 +44,12 @@ const setup = (opts: RoomOpts) => {
|
|||||||
calls.push({ eventId: event.getId(), receiptType, unthreaded });
|
calls.push({ eventId: event.getId(), receiptType, unthreaded });
|
||||||
return {};
|
return {};
|
||||||
},
|
},
|
||||||
|
setRoomAccountData: async (_roomId: string, type: string, content: any) => {
|
||||||
|
accountDataWrites.push({ type, content });
|
||||||
|
return {};
|
||||||
|
},
|
||||||
} as any;
|
} as any;
|
||||||
return { mx, calls };
|
return { mx, calls, accountDataWrites };
|
||||||
};
|
};
|
||||||
|
|
||||||
test('main timeline: unthreaded receipt at the latest event', async () => {
|
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);
|
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 () => {
|
test('sending thread reply is skipped', async () => {
|
||||||
const t = thread('$root', evt('$reply', true)); // isSending → skip
|
const t = thread('$root', evt('$reply', true)); // isSending → skip
|
||||||
const { mx, calls } = setup({
|
const { mx, calls } = setup({
|
||||||
|
|||||||
@@ -1,11 +1,19 @@
|
|||||||
import { MatrixClient, NotificationCountType, ReceiptType } from 'matrix-js-sdk';
|
import { MatrixClient, NotificationCountType, ReceiptType } from 'matrix-js-sdk';
|
||||||
import { getSettings } from '../state/settings';
|
import { getSettings } from '../state/settings';
|
||||||
|
import { readMarkedUnread, setMarkedUnread } from '../state/room/markedUnread';
|
||||||
|
|
||||||
export async function markAsRead(mx: MatrixClient, roomId: string, privateReceipt: boolean) {
|
export async function markAsRead(mx: MatrixClient, roomId: string, privateReceipt: boolean) {
|
||||||
const { privateReadReceipts } = getSettings();
|
const { privateReadReceipts } = getSettings();
|
||||||
const room = mx.getRoom(roomId);
|
const room = mx.getRoom(roomId);
|
||||||
if (!room) return;
|
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 =
|
const receiptType =
|
||||||
privateReceipt || privateReadReceipts ? ReceiptType.ReadPrivate : ReceiptType.Read;
|
privateReceipt || privateReadReceipts ? ReceiptType.ReadPrivate : ReceiptType.Read;
|
||||||
|
|
||||||
|
|||||||
Reference in New Issue
Block a user