docs(audit): Wave-1 bug-hunt findings (notifications/threads/calls/EC fork)
4 parallel deep-audit agents over the Tier-1 high-risk areas. Findings only (no source changes). Top 🔴: markThreadAsRead corrupts the main read marker via a thread-root receipt (a SECOND instance of the P6 read-receipt regression, likely a live cause of "unread won't clear"); favicon/title count double-counts space aggregates; deliverNotification dedupe cache never cleared on read → missed notifications/sounds. Plus 🟠 (thread "All" override defeated, phantom muted-thread dot, receipt-DELETE badge race, thread jump-to-latest, call forceState-on-reconnect clobber, AFK wrong-mic auto-mute, stale control observer) and a long 🟡 tail. Recorded in LOTUS_TODO for prioritized fix passes. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
@@ -33,6 +33,41 @@ 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.
|
||||
|
||||
### 🔴 High — data-integrity / broken core UX
|
||||
|
||||
| ID | Defect | File:line | Repro | Fix sketch |
|
||||
| :----- | :------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------ | :--------------------------------------------------------------------------------------------------- | :----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | :----------------------------------------------------------------------------------------------------------------------------------------------------- |
|
||||
| **T1** | **`markThreadAsRead` corrupts the room's MAIN read marker** — sends `sendReadReceipt(latestEvent, Read)` with `unthreaded` omitted (→false); `latestEvent` is the thread **root** when replies aren't loaded, so the SDK writes a `thread_id:"main"` receipt at the old root → `getEventReadUpTo` jumps back → room re-lights. **Same class as the P6 regression, second path.** CONFIRMED. | `features/room/thread/useThread.ts:172` (fired on mount by `ThreadPanel.tsx:145-167`) | Open a thread whose root is an older message (or any thread before replies load) → previously-read main messages resurface as unread; room dot won't clear. **Likely a live cause of the current "unread won't clear."** | Mirror `notifications.ts`: use `thread.lastReply()` and bail if null, or `if (latestEvent.getId() === thread.id) return;` — only receipt a real reply. |
|
||||
| **N1** | Favicon + tab-title unread count **double-counts space aggregates** — `putUnreadInfo` writes an entry for the leaf room AND every ancestor space; `FaviconUpdater` sums **all** map entries. CONFIRMED. | `pages/client/ClientNonUIFeatures.tsx:97-103` (FaviconUpdater) vs `state/room/roomToUnread.ts:67-74` | A room w/ 3 highlights nested in Space→Subspace → tab title reads `(9)` not `(3)`. Orphan-only users unaffected (why it hid). | Sum only leaf entries (skip aggregates: leaves have `from:null`), or compute from `mx.getRooms()`. |
|
||||
| **N2** | **Missed notifications/sounds** — the `deliverNotification` dedupe cache (`unreadCacheRef`) is written only on incoming messages, never cleared on read. `unreadInfo.total` resets to 0 on read but the cache keeps the stale value. CONFIRMED. | `pages/client/ClientNonUIFeatures.tsx:364-408` | DM cadence "msg→read→msg→read…": after the first, every later message's notification+sound is suppressed (`unreadEqual(1,1)`). | Clear/refresh `unreadCacheRef` from a `RoomEvent.Receipt` listener, or dedupe on last-notified `eventId` (as the thread path already does). |
|
||||
|
||||
### 🟠 Medium — visible wrong behavior
|
||||
|
||||
| ID | Defect | File:line | Notes |
|
||||
| :--------- | :--------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | :--------------------------------------------------------------------- | :--------------------------------------------------------------------------------------------------------- |
|
||||
| **T4** | Explicit per-thread **"All" override is defeated** by the room-count gate: `deliverNotification` returns early on `unreadInfo.total===0` and dedupes on `unreadEqual`, but a non-mention thread reply in a Mentions-only room doesn't bump the count → "notify all replies" silently drops. CONFIRMED. | `ClientNonUIFeatures.tsx:370-380` vs `:489-502` | Don't gate the thread path on room `total`; dedupe on thread event id (`lastNotifiedThreadRef`) only. |
|
||||
| **N3/T3+** | **Muted-thread-only room gets a phantom unread dot** — `getUnreadInfos` pushes a `{0,0}` entry (roomHaveNotification true from the muted thread's server count), which still lights the nav row + pollutes "unread only" filters. CONFIRMED. | `utils/room.ts:261-278` | Don't push when `getUnreadInfo`→`{0,0}` and `roomHaveUnread` is false. |
|
||||
| **N4/T3** | Reading a thread **DELETEs the whole room badge** — `handleReceipt` unconditionally `DELETE`s on any of my receipts; if the thread was already read (no `UnreadNotifications` PUT follows) the room's legit main-timeline badge vanishes until the next event. Also a PUT/DELETE ordering race. PLAUSIBLE. | `state/room/roomToUnread.ts:243-263` | For threaded receipts, recompute via `getUnreadInfo` PUT instead of blanket DELETE. |
|
||||
| **T2** | Thread **"Jump to Latest" doesn't reset the virtual window** — only bumps scroll count; when scrolled up with live replies arriving, newest replies aren't rendered so the chip lands on a mid/old event. CONFIRMED. | `features/room/thread/ThreadTimeline.tsx:462-468` | Re-anchor with `setTimeline(getInitialThreadTimeline(...))` like the main timeline's `handleJumpToLatest`. |
|
||||
| **C-H1** | `forceState()` **re-runs on every `JoinCall`** (no once-guard) → on an EC reconnect it resets live mic/video/deafen back to the join-time snapshot, clobbering the user's mid-call toggles. PLAUSIBLE (structurally confirmed). | `plugins/call/CallEmbed.ts:362-364,502-509` + `CallControl.ts:141-156` | Guard `onCallJoined` to `forceState` only on first join, or re-apply _current_ state on rejoin. |
|
||||
| **C-H2** | AFK auto-mute monitors the **browser default mic**, not EC's selected device → measures silence while the user talks on a non-default mic → **auto-mutes an active speaker**; also a 2nd OS mic indicator. PLAUSIBLE. | `hooks/useAfkAutoMute.ts:39-40` | Acquire with EC's selected `deviceId`, or tap EC's existing track. |
|
||||
| **C-H3** | Screenshare/spotlight **state observer goes stale** — `bodyMutationObserver` uses `subtree:false`, so when EC re-renders its controls subtree `observeControls()` never re-runs and the reflected screenshare/spotlight boolean freezes. PLAUSIBLE. | `plugins/call/CallControl.ts:168-200,274-287` | Observe the stable controls container with `subtree:true`, or re-query on each mutation. |
|
||||
|
||||
### 🟡 Low / minor / cosmetic / perf
|
||||
|
||||
- **Calls host:** C-M1 deafen DOM-fallback leaks late-added `<audio>` tracks; C-M2 `.click()`-by-testid toggles silently no-op if EC renames; C-M3 `setQuality` not join-gated (pends to 10s timeout) — mirror the deafen gate; C-M4 End button spins forever if EC ACKs hangup but never echoes Close (add fallback dispose timeout); C-M5 PTT while deafened silently un-deafens on key-up; C-M6 `screenshareAudioMuted` never reset when screenshare stops; C-L1 AFK mic not released if EC elides the echo; C-L2 ringtone-preview global cross-cancel; C-L3 first incoming ring after cold load can be silent (ringtones ctx not unlocked); C-L4 deafen M-key bound to window only (not iframe); C-L5 speaker-observer churn on every membership change; C-L6 setState-after-unmount (IncomingCallListener, CallSoundboard 30s timer); C-L7 all-muted DOM-fallback miscount if EC label format differs; C-L8 PiP sw/nw resize anchor jitter at min size.
|
||||
- **Notifications:** N5 `deleteUnreadInfo` `?? roomId` spreads a string into chars (latent/unreachable; should be `?? []`) — `roomToUnread.ts:85`; N6 per-message read-receipt avatars may not refresh on membership change (`RoomMemberEvent.Membership` on `Room` may not bubble) — `useRoomReadPositions.ts:57-61`.
|
||||
- **Threads:** T5 `participating` detection is server-bundle-only (`thread.hasCurrentUserParticipated`) → can under-notify a thread you just replied to; T6 room "Mentions & Keywords" not honored for participated/Default thread replies (over-notify, partly masked by T4); T7 account-data thread-mute write is a lost-update read-modify-write race.
|
||||
- **EC fork (needs a republish):** EC1 `lotusQuality` 500ms re-apply `setTimeout` never cleared → fires on torn-down rooms; EC2/EC3 `lotusQuality`+`lotusAudioInject` subscribe to the remote-gated `livekitRoomItems$` (no-op when alone) — should use `allConnections$` like denoise; EC4 `lotusDecorations` keeps module-scoped per-call state (cosmetic leak into next call — the last deafen-class sibling); EC5 `lotusDecorations` inline `subscribe` re-subscribes every render; EC6 `lotusFocus` clears the pin on a payload missing `userId` (should keep current).
|
||||
|
||||
**Verified sound (spot-check list):** `markAsRead` (no root fallback), `useRoomsListener` variable-arity fix, `unreadEqual`, `getUnreadInfo` muted-thread subtraction, own-reply suppression, single-owner timeline rule, `lotusDeafen` (closure-scoped), denoise processor lifecycle, PTT stuck-mic hardening + editable-target guard, PiP position validation, embed single-dispose. (Full per-area "looks-correct" lists in the audit run.)
|
||||
|
||||
---
|
||||
|
||||
## ✅ Done — Awaiting Verification
|
||||
|
||||
Built and gate-green; verify per [LOTUS_TESTING.md](./LOTUS_TESTING.md), then they graduate to LOTUS_FEATURES.md. (Open bugs + the verification backlog now live in this file and LOTUS_TESTING.md.)
|
||||
|
||||
Reference in New Issue
Block a user