From f1a673e87e1389bf98d0ebab708e6da115ddb442 Mon Sep 17 00:00:00 2001 From: J3vb <192430104+J3vb@users.noreply.github.com> Date: Sat, 22 Aug 2026 06:34:58 +0200 Subject: [PATCH] fix: 35 findings from the 2026-08-22 bug hunt (#1402) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix(voice): 1 defect(s) (OC-0277) * fix(voice): 1 defect(s) (OC-0278) * fix(client): 1 defect(s) (OC-0280) refreshDmSidebar() rebuilds the entire DM sidebar subtree on every dmStore.channels change - which includes presence flips and new messages, not just DM list changes. The "Find a conversation" filter text and input focus live only in that destroyed subtree, so they were silently wiped mid-typing. Capture and restore both across the destroy+recreate cycle. * fix(ws): 1 defect(s) (OC-0285) * fix(client): 1 defect(s) (OC-0286) * fix(client): 1 defect(s) (OC-0288) Consume the legacy unscoped mute key after migrating it onto the first host, so a brand-new host with no scoped key of its own no longer reads through to the same legacy list and inherits another server's mutes. * fix(voice): 1 defect(s) (OC-0290) * fix(db): 1 defect(s) (OC-0293) DecrementMentionCounts reversed mention_count bumps that were never applied: message_mentions stores every resolved mention id including the author's blockers, while applyMentionCounts excludes blockers before incrementing. Deleting a blocked author's message therefore wiped an unrelated, genuine mention badge on the same read_states row. Mirror the block exclusion in the decrement UPDATE. * fix(db): 1 defect(s) (OC-0294) DeleteAccount soft-deletes the departing user's messages but never reversed the read_states.mention_count bumps those messages made, leaving phantom mention badges. Reverse them inline in the existing transaction, mirroring DecrementMentionCounts' guards. * fix(client): 1 defect(s) (OC-0295) MemberList rebuilt every row on any non-presence-only membersStore change and on every roles_update, but registered each row's click/contextmenu listeners on the component-lifetime disposable.signal, which only aborts at destroy(). Discarded rows therefore stayed reachable (and their listeners live) for the component's whole lifetime. Route per-row listeners through a per-render AbortController that is aborted and replaced at the top of every render, and aborted again in destroy(). * fix(identity): 1 defect(s) (OC-0297) UpdateProfile's post-commit re-read of the user row could fail for reasons unrelated to context cancellation (SQLITE_BUSY, I/O error, pool exhaustion) and was reported as ErrInternal even though UpdateUserProfile had already committed. Callers that treat any UpdateProfile error as proof the write never landed — handleUploadAvatar deletes the file it just stored — would delete a file the committed avatar column now points at, permanently breaking the avatar with no user_update broadcast. Since UpdateUserProfile only writes username/avatar/display_name/about, merge those four onto the pre-write snapshot to reconstruct the committed row without needing the re-read to succeed, and log the read failure. * fix(ws): 2 defect(s) (OC-0298, OC-0299) - OC-0298: applyConnectStatus stamped c.user.Status even when the UpdateUserStatus write failed, so auth_ok and the presence broadcast claimed a status users.status disagreed with, and buildReady's ListMembers read never self-corrected for the session. - OC-0299: refreshUserSnapshot silently fell back to roleName "member" when the new role lookup failed, pinning the session to a fabricated role on the wire. It now fails closed like the sibling lookups in upgradeAndAuth and handleFreshConnect. * fix(client): 1 defect(s) (OC-0300) * fix(client): 1 defect(s) (OC-0301) * fix(ws): 1 defect(s) (OC-0302) * fix(api): 1 defect(s) (OC-0305) handleDiagnosticsConnectivity used clientIP(r), ignoring cfg.Server.TrustedProxies, so behind a configured trusted reverse proxy the endpoint reported the proxy hop instead of the real client address. Use clientIPWithProxies with the parsed trusted-proxy nets, matching RateLimitMiddleware on the same route. * fix(client): 2 defect(s) (OC-0306, OC-0308) * fix(client): 1 defect(s) (OC-0307) QuickSwitcher registered a per-row click listener against the overlay-lifetime AbortSignal, but renderResults() rebuilds every row on each keystroke, arrow key, and store refresh. Discarded rows kept their listeners alive until the overlay closed. Replaced with one delegated click listener on the stable results container, keyed off the data-channelid each row already carries. * fix(client): 1 defect(s) (OC-0310) * fix(server): 3 defect(s) (OC-0279, OC-0291, OC-0292) Reap a soft-deleted message's attachment files, count lapsed temporary bans as active users in the require_2fa enrollment gate, and only apply the 2FA-enrollment precondition when require_2fa itself is being enabled. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01SdkJRbjCtrG76jEnrhKbYo * test(api): sync apiTestSchema with the user_blocks migration DeleteAccount's mention-count reversal joins user_blocks; the api package's hand-rolled schema fixture predates migration 012. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01SdkJRbjCtrG76jEnrhKbYo * fix(client): 3 defect(s) (OC-0281, OC-0282, OC-0296) Decouple the E2EE identity-mismatch modal and right-click popovers from the sidebar's per-render abort signal, and let global drag listeners survive a mid-drag re-render. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01SdkJRbjCtrG76jEnrhKbYo * fix(voice): 2 defect(s) (OC-0283, OC-0287) Retire a departed peer's E2EE key unconditionally on leave, and surface a failed microphone unmute instead of reporting an unmuted state the room never saw. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01SdkJRbjCtrG76jEnrhKbYo * fix(client): 3 defect(s) (OC-0289, OC-0303, OC-0309) Guard the DM call button against redialing the channel already joined, resolve the incoming-call banner's caller through the nickname-aware display name, and keep the DM profile sidebar subscribed to live member/status updates. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01SdkJRbjCtrG76jEnrhKbYo * style(client): prettier-format the dm-store test Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01SdkJRbjCtrG76jEnrhKbYo * fix(server): 1 defect(s) (OC-0284) Make message soft-delete a compare-and-set so a repeated chat_delete cannot reverse mention counts twice. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01SdkJRbjCtrG76jEnrhKbYo * fix(server): 2 defect(s) (OC-0276, OC-0304) Re-sync a resumed connection's voice E2EE peer keys in registerNow (announce frames are unsequenced and cannot be replayed), and apply the live-connection presence rule to every DM payload DMService builds. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01SdkJRbjCtrG76jEnrhKbYo * chore(ledger): record the 2026-08-21 hunt findings as fixed Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01SdkJRbjCtrG76jEnrhKbYo * chore(ledger): independent revert-proof pass for OC-0276..OC-0310 Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01SdkJRbjCtrG76jEnrhKbYo * refactor(service): extract DeleteMessage authorization into a helper Keeps DeleteMessage under the cyclop complexity ceiling after the OC-0284 guard. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01SdkJRbjCtrG76jEnrhKbYo --------- Co-authored-by: Claude --- .superpowers/FINDINGS.md | 951 +++++++++++++++++- .superpowers/findings-ledger.json | 807 ++++++++++++++- .../src/components/ChannelSidebar.ts | 65 +- .../src/components/DmProfileSidebar.ts | 65 +- .../src/components/EmojiPicker.ts | 40 +- .../tauri-client/src/components/MemberList.ts | 29 +- .../src/components/MessageInput.ts | 34 +- .../src/components/MessageList.ts | 40 +- .../src/components/QuickSwitcher.ts | 28 +- Client/tauri-client/src/components/UserBar.ts | 35 +- .../channel-sidebar/context-menu.ts | 28 +- .../channel-sidebar/drag-reorder.ts | 15 +- .../components/channel-sidebar/volume-menu.ts | 13 +- Client/tauri-client/src/lib/channel-mutes.ts | 9 + Client/tauri-client/src/lib/dispatcher.ts | 21 +- Client/tauri-client/src/lib/livekitE2EE.ts | 38 +- Client/tauri-client/src/lib/livekitSession.ts | 31 +- .../tauri-client/src/lib/noise-suppression.ts | 14 +- Client/tauri-client/src/pages/MainPage.ts | 120 ++- .../src/pages/main-page/SidebarArea.ts | 25 + .../src/pages/main-page/VoiceCallbacks.ts | 7 + Client/tauri-client/src/stores/dm.store.ts | 12 +- .../tests/unit/channel-mute-ui.test.ts | 6 +- .../tests/unit/channel-mutes.test.ts | 18 + .../tests/unit/channel-sidebar.test.ts | 191 ++++ .../tests/unit/dispatcher.test.ts | 56 ++ .../tests/unit/dm-profile-sidebar.test.ts | 41 + .../tauri-client/tests/unit/dm-store.test.ts | 37 + .../tests/unit/drag-reorder.test.ts | 97 +- .../tests/unit/emoji-picker.test.ts | 60 ++ .../tests/unit/livekit-e2ee.test.ts | 33 +- .../tests/unit/livekit-session.test.ts | 28 + .../tauri-client/tests/unit/main-page.test.ts | 95 +- .../tests/unit/member-list.test.ts | 30 + .../tests/unit/message-input.test.ts | 9 + .../message-list-row-listener-leak.test.ts | 156 +++ .../unit/noise-suppression-restart.test.ts | 99 ++ .../tests/unit/quick-switcher.test.ts | 23 + .../tests/unit/sidebar-area.test.ts | 79 ++ .../tests/unit/status-picker-userbar.test.ts | 67 +- .../tests/unit/voice-callbacks.test.ts | 17 + Server/admin/api_test.go | 49 + Server/admin/handlers_settings.go | 10 + Server/api/diagnostics_handler.go | 3 +- Server/api/diagnostics_handler_test.go | 62 ++ Server/api/dm_handler.go | 7 +- Server/api/dm_handler_presence_test.go | 70 ++ Server/api/middleware_test.go | 8 + Server/db/account.go | 41 + Server/db/account_test.go | 76 ++ Server/db/admin_queries.go | 5 +- .../db/attachment_orphan_softdelete_test.go | 96 ++ Server/db/attachment_queries.go | 16 +- Server/db/count_users_without_totp_test.go | 59 ++ Server/db/dbgen/attachments.sql.go | 20 +- Server/db/dbgen/messages.sql.go | 9 +- Server/db/dbgen/querier.go | 20 +- Server/db/dbgen/users.sql.go | 13 +- Server/db/mention_queries.go | 21 +- Server/db/mention_queries_test.go | 50 + Server/db/message_queries.go | 13 +- Server/db/queries/sqlite/attachments.sql | 20 +- Server/db/queries/sqlite/messages.sql | 4 +- Server/db/queries/sqlite/users.sql | 13 +- Server/service/dm.go | 80 +- Server/service/dm_test.go | 96 ++ Server/service/mentions_test.go | 45 + Server/service/message_crud.go | 71 +- Server/service/user.go | 21 +- .../service/user_postcommit_readerror_test.go | 78 ++ Server/ws/export_test.go | 10 + Server/ws/handler_v2_migration_test.go | 34 + Server/ws/handlers.go | 13 + Server/ws/handlers_test.go | 68 ++ Server/ws/handlers_voice.go | 9 +- Server/ws/hub.go | 37 + Server/ws/oc_0276_voice_e2ee_resync_test.go | 95 ++ .../ws/oc_0298_apply_connect_status_test.go | 82 ++ .../ws/oc_0299_refresh_snapshot_role_test.go | 79 ++ ...oc_0302_pending_mod_flags_transfer_test.go | 45 + Server/ws/serve.go | 20 +- Server/ws/voice_e2ee.go | 41 + Server/ws/voice_join.go | 33 +- Server/ws/voice_moderation.go | 21 +- Server/ws/voice_moderation_test.go | 48 + 85 files changed, 5065 insertions(+), 215 deletions(-) create mode 100644 Client/tauri-client/tests/unit/message-list-row-listener-leak.test.ts create mode 100644 Client/tauri-client/tests/unit/noise-suppression-restart.test.ts create mode 100644 Server/api/dm_handler_presence_test.go create mode 100644 Server/db/attachment_orphan_softdelete_test.go create mode 100644 Server/db/count_users_without_totp_test.go create mode 100644 Server/service/user_postcommit_readerror_test.go create mode 100644 Server/ws/oc_0276_voice_e2ee_resync_test.go create mode 100644 Server/ws/oc_0298_apply_connect_status_test.go create mode 100644 Server/ws/oc_0299_refresh_snapshot_role_test.go create mode 100644 Server/ws/oc_0302_pending_mod_flags_transfer_test.go diff --git a/.superpowers/FINDINGS.md b/.superpowers/FINDINGS.md index 7eca4956..a0bb352b 100644 --- a/.superpowers/FINDINGS.md +++ b/.superpowers/FINDINGS.md @@ -2,7 +2,7 @@ Generated by `render-ledger.mjs`. Do not hand-edit — edit `findings-ledger.json`. -**0 open** · 0 blocked · 271 fixed · 3 declined · 0 refuted · 1 duplicate +**0 open** · 0 blocked · 306 fixed · 3 declined · 0 refuted · 1 duplicate ## Fixed @@ -7033,6 +7033,955 @@ queries/sqlite/messages.sql:51-56 — unread excludes deleted rows, mentions is **Fixed:** `ce252b0` · test `Server/service/mentions_test.go` · revert-proof self-reported +### OC-0276 — high — voice_e2ee_announce is unsequenced and un-replayable, and the only other relay of a peer's ECDH key runs at voice_join — a WS reconnect permanently loses peer keys + +`Server/ws/voice_e2ee.go:271` · found 2026-08-22 · hunt `general-2026-08-21` · lens `voice-e2ee` + +`sendToVoiceChannelExcept` publishes `voice_e2ee_announce` straight onto the VoiceTopic pub/sub (`h.pubsub.Publish`), bypassing `h.broadcast`/`deliverBroadcast` — so the frame never gets a `seq`, never enters `h.replayBuf`, and is never persisted. Neither reconnect tier can therefore redeliver it: the replay tiers only resend sequenced frames, and `buildReady`'s payload carries `voice_states` but no `identity`/ECDH key material at all (Server/ws/serve_ready.go:355-368). The server's ONLY other relay of a stored ECDH key is inside `voiceJoinComplete` (Server/ws/voice_join.go:527), which runs on a fresh `voice_join` and never on a resume — a fact the file itself asserts ("this read is the ONLY place the server ever relays an existing participant's stored ECDH public key", voice_join.go:502-506). Meanwhile `voice_state` IS sequenced and replayed, so after a WS blip the client's voice roster and its `_peerPublicKeys` map permanently disagree. The client has no compensation either: dispatcher.ts's OC-0201 resync reconciliation (dispatcher.ts:374-384) only fires `handleParticipantLeft` for peers that DEPARTED during the outage, and only on the full-`ready` tier — nothing handles peers that ARRIVED or re-announced, and the replay tiers never run it at all. + +**Repro:** Users H (uid 1, key holder), A (uid 2) and later J (uid 9) in voice channel C. +1. A's WebSocket dies (network blip). The server has not noticed yet, so A is still in `h.clients` and still subscribed to `VoiceTopic(C)`. +2. J sends voice_join for C. `voiceJoinComplete` relays H's and A's stored keys to J, then J's `voice_e2ee_announce` is published to VoiceTopic → queued into dead-socket A's `c.send`. +3. A reconnects. `registerNow` (hub.go:559) calls `old.closeSend()`, discarding the queued announce, and re-subscribes the new connection. A resumes on the buffer/DB replay tier: J's `voice_state` IS replayed (it went through deliverBroadcast and holds a seq), J's `voice_e2ee_announce` is not — it has no seq and is not in `replayBuf`. +4. A's `voiceStore.voiceUsers` now lists J; A's `E2EEManager._peerPublicKeys` does not. +5. H leaves. A is the lowest remaining uid, so `handleParticipantLeft` promotes A, `rotateRoomKey()` installs a fresh key, and `distributeRoomKey(keypair, roomKey, peersSnapshot)` iterates `_peerPublicKeys` — J is absent and gets no offer. J never re-announces (mid-call peers only re-announce on their own LiveKit reconnect), so every subsequent 5-minute rotation excludes J too. +6. J can neither decrypt nor be decrypted for the rest of the call, while every client's VoiceWidget still shows "🔒 Secured". + +Same root cause, second variant: replace step 2 with "H's LiveKit connection blips and `reannounceForReconnect()` publishes H's NEW ephemeral key". A misses it and keeps H's OLD public key; from then on every offer H wraps (ECDH(H_new_priv, A_pub)) fails GCM authentication when A unwraps it with ECDH(A_priv, H_old_pub), so A is stuck on the pre-rotation room key permanently — `handleOfferInner` just logs "failed to handle offer" on each rotation. + +**Evidence:** Server/ws/voice_e2ee.go:270-272 + func (h *Hub) sendToVoiceChannelExcept(channelID int64, excludeUserID int64, msg []byte) { + h.pubsub.Publish(VoiceTopic(channelID), msg, excludeUserID) + } +(compare hub_broadcast.go:984-1009 `deliverBroadcast`, which is the only path that allocates `seq = h.nextSeq()` and calls `h.replayBuf.Push(seq, ...)` / `h.persistEvent(...)`) + +Server/ws/voice_join.go:527-529 — the sole re-relay, reachable only from voiceJoinComplete: + if pubKey, sig := h.getClientE2EEPubKey(vs.UserID); pubKey != "" { + c.sendMsg(buildVoiceE2EEAnnounce(vs.UserID, pubKey, sig)) + } + +Server/ws/hub.go:559 — registerNow discards everything queued for the replaced socket: `old.closeSend()` + +**Suggested fix:** Factor voice_join.go:520-530's per-participant relay loop (voice_state + buildVoiceE2EEAnnounce for each other participant with a stored key) into one helper, e.g. func (h *Hub) sendVoicePeerKeys(ctx, c *Client, channelID int64), and call it from the resume path in serve.go once registerNow has restored the client's voice channel — so BOTH reconnect tiers re-sync every current peer's ECDH public key. One shared call on the resume path fixes arrivals, re-announces missed during the outage, and the LiveKit-reconnect variant at once; making announces sequenced/replayable is a much larger change and not required. + +**Fixed:** `201e2bc` · test `Server/ws/oc_0276_voice_e2ee_resync_test.go` · revert-proof pass + +### OC-0277 — high — RNNoise processor's restart() destroys the pipeline and then rebuilds it from `opts.audioContext`, which livekit-client never sends on restart — any mic restart while Enhanced Noise Suppression is on permanently publishes silence + +`Client/tauri-client/src/lib/noise-suppression.ts:264` · found 2026-08-22 · hunt `general-2026-08-21` · lens `explore-1` + +`init()` reads the AudioContext straight off `opts` (`const ctx = opts.audioContext`) and never retains it, and `restart()` forwards livekit-client's restart opts into it. livekit-client populates `audioContext` only in `LocalTrack.setProcessor()`; its one and only `processor.restart(...)` call site — `LocalTrack.setMediaStreamTrack()` — passes `{track, kind, element, localTrack}` with no `audioContext` (the `AudioProcessorOptions` d.ts declares it required, so TypeScript never catches the omission). Because `restart()` tears the live pipeline down *before* re-initialising, the failure is not a no-op: the old worklet is disconnected (its dest track goes silent but is never stopped or replaced), `pipeline` is left null, and `init()` rejects — so `restartTrack()` unwinds before it ever reaches `sender.replaceTrack(...)`, leaving the RTCRtpSender wired to the now-dead audio graph. + +**Repro:** 1. Join a voice channel. 2. Settings -> Voice & Audio -> turn on "Enhanced Noise Suppression" (`applyNoiseSuppressor()` -> `LocalAudioTrack.setProcessor(rnnoiseProcessor)`, which supplies `audioContext`, so this succeeds and the sender ends up on the audioPipeline dest fed by the worklet's output). 3. In the same tab, toggle any of "Echo Cancellation" / "Noise Suppression" / "Automatic Gain Control" (Client/tauri-client/src/lib/audioPipeline.ts:471 `await (micPub.track as LocalAudioTrack).restartTrack(captureOptions)`). Equivalent triggers: picking a different microphone (deviceManager.ts:176 `room.switchActiveDevice("audioinput", id)` -> `setDeviceId` -> `restartTrack`), unplugging the active mic (livekit `handleTrackEnded` -> `restartTrack({deviceId:'default'})`), or a post-sleep full reconnect (`republishAllTracks` -> `restartTrack`). + +What happens: `restartTrack` -> `restart()` stops the old raw mic track (esm.mjs:20597), acquires a new one, then `setMediaStreamTrack` calls our `restart(opts)` with `opts.audioContext === undefined`. We `pipeline.destroy()` (worklet disconnected -> its dest MediaStreamTrack now emits pure silence, and it is never stopped), set `pipeline = null`, then `init(opts)`: `createWorkletPipeline(track, undefined)` throws `TypeError: Cannot read properties of undefined (reading 'audioWorklet')`, is swallowed by the fallback catch, and `createScriptProcessorPipeline(track, undefined)` then throws the same TypeError at `audioContext.createMediaStreamSource(...)` — uncaught, so `init` rejects. + +Resulting state: `processor.processedTrack === undefined`; livekit's `_mediaStreamTrack` still points at the *stopped* old track; the freshly acquired track is live but referenced by nothing (mic-in-use indicator stays lit); `sender.replaceTrack` never ran, so the peer connection keeps sending the audioPipeline dest track whose upstream worklet was just disconnected. The user sees only a "Failed to update audio settings" / "Failed to switch microphone" toast, still shows as unmuted to every peer, and transmits silence. Retrying the toggle fails identically; toggling Enhanced NS off does not recover it either (`internalStopProcessor` re-wires the stopped `_mediaStreamTrack`). Only leaving and rejoining the voice channel restores audio. No test in tests/unit locks this behaviour (tests/unit/rnnoise-worklet.test.ts never touches `restart` or `audioContext`). + +**Evidence:** noise-suppression.ts:242-265 + async init(opts: AudioProcessorOptions): Promise { + ... + const ctx = opts.audioContext; // <- never cached + if (supportsAudioWorklet()) { + try { pipeline = await createWorkletPipeline(opts.track, ctx); return; } + catch (err) { log.warn("AudioWorklet failed, falling back to ScriptProcessorNode", err); } + } + pipeline = await createScriptProcessorPipeline(opts.track, ctx); + }, + async restart(opts: AudioProcessorOptions): Promise { + if (pipeline !== null) { pipeline.destroy(); pipeline = null; } // destroy first + await this.init(opts); // then rebuild from opts.audioContext + }, + +node_modules/livekit-client/dist/livekit-client.esm.mjs:20414 (the ONLY `processor.restart` call site in the bundle) + yield this.processor.restart({ + track: newTrack, + kind: this.kind, + element: this.processorElement, + localTrack: this + }); // no audioContext field + +contrast with setProcessor(), esm.mjs:20750-20755, which does pass it: + const processorOptions = { kind, track, element: processorElement, audioContext: _this3.audioContext, localTrack: _this3 }; + +esm.mjs:20597 (LocalTrack.restart) stops the old raw mic track *before* acquiring the new one, and esm.mjs:20425 (`sender.replaceTrack`) / 20429 (`this._mediaStreamTrack = newTrack`) sit *after* the `processor.restart` await, so a rejection strands both. + +**Suggested fix:** Cache the AudioContext in the processor closure so restart() can reuse it. In createRNNoiseProcessor add `let ctx: AudioContext | null = null;` alongside `pipeline`, and in init() replace `const ctx = opts.audioContext;` with `const audioCtx = opts.audioContext ?? ctx; if (audioCtx == null) throw new Error("RNNoise processor: no AudioContext available"); ctx = audioCtx;` then pass `audioCtx` to both createWorkletPipeline/createScriptProcessorPipeline. One guard in the shared init() covers restart() too. (Optionally also build the new pipeline before destroying the old one so a future failure is not destructive, but the ctx cache alone fixes the reported defect.) + +**Fixed:** `15d65e1` · test `Client/tauri-client/tests/unit/noise-suppression-restart.test.ts` · revert-proof pass + +### OC-0278 — medium — voice_mod_move stashes the server-mute before the eviction that justifies it, so a refused move leaves a phantom mute that fires on an unrelated later join + +`Server/ws/voice_moderation.go:443` · found 2026-08-22 · hunt `general-2026-08-21` · lens `ws-hub` + +stashPendingModFlags writes pendingModServerMuted/Deafened onto the target's live *Client *before* disconnectFromVoiceIn runs. When that call reports false the handler returns an error and the move never happens, but the stash is left behind. It has no expiry, no binding to the move's destination channel and no binding to the voice_states row it was copied from, so the *next* voice_join that connection makes from a not-in-voice state re-imposes a server mute that no moderator ordered — including one that was explicitly lifted in the meantime. + +**Repro:** 1. U is in voice channel A, server-muted by a moderator (voice_states.server_muted=1). +2. A moderator issues voice_mod_move(U -> C). voiceModTarget loads U's row (state.ServerMuted=true) and stashPendingModFlags sets U's client pendingModServerMuted=true (voice_moderation.go:443). +3. Concurrently, on U's own read-pump goroutine, U switches to voice channel B. DisconnectFromVoiceInChannel(U, A) now finds U.voiceChID==B, returns false, and handleVoiceModMoveV2 returns VOICE_ERROR "user is not connected" — the move is correctly refused. The stash is NOT cleared. +4. A moderator un-mutes U in B (voice_mod_mute muted=false -> voice_states.server_muted=0). +5. U leaves voice entirely (row deleted, mute state gone, per the existing 'a leave drops server_muted' semantics) and later joins any voice channel on the same WS connection. +6. voiceJoinLeaveCurrent sees currentChID==0, takePendingModFlags() returns (true,false), and voiceJoinRestoreModFlags calls SetVoiceServerMute(..., true). +Result: U is server-muted in a channel no moderator ever muted them in, the earlier un-mute is silently reverted, and the voice_state broadcast tells every client U is moderator-muted. Expected: a refused move leaves no residue — the stash should be written only after the eviction that erased the row actually succeeded (and cleared if the move errors out). + +**Evidence:** voice_moderation.go:443-450 + stashPendingModFlags(d.Mod, c.TargetID(), state.ServerMuted, state.ServerDeafened) + if !disconnectFromVoiceIn(ctx, d.Mod, c.TargetID(), state.ChannelID) { + // No live connection on this node ... or the target left the checked + // channel while this handler was deciding, in which case the move must + // not follow them. + return Result{Error: ClientError{Code: ErrCodeVoiceError, Message: "user is not connected"}} + } + +voice_moderation.go:551-557 — the false return is reachable whenever the target switched channels concurrently: + func (h *Hub) DisconnectFromVoiceInChannel(ctx context.Context, userID, channelID int64) bool { + c := h.GetClient(userID); if c == nil { return false } + return h.handleVoiceLeaveIfStillIn(ctx, c, channelID) // false when c.voiceChID != channelID + } + +The stash is unconditional and unscoped (voice_moderation.go:181-188, client.go:234-239); the only consumer is voice_join.go:218, which fires on ANY join taken with currentChID==0, in ANY channel, at any later point in the connection's life. + +**Suggested fix:** Clear the stash on the refusal branch in handleVoiceModMoveV2 - one guard in the shared handler, not in the consumer: change voice_moderation.go:444-450 to call stashPendingModFlags(d.Mod, c.TargetID(), false, false) (or a dedicated clearPendingModFlags helper, since stashPendingModFlags early-returns when both flags are false - so a small clearPendingModFlags that calls setter.SetPendingVoiceModFlags(target, false, false) unconditionally is needed) immediately before `return Result{Error: ... "user is not connected"}`. Moving the stash to after the successful disconnect is not equivalent: the target could send an unrelated voice_join in the gap between the eviction and the stash, leaving the same residue. + +**Fixed:** `66681d1` · test `Server/ws/voice_moderation_test.go` · revert-proof pass + +### OC-0279 — medium — Deleting a message strands its uploaded attachment files on disk forever — the only reaper requires message_id IS NULL + +`Server/db/message_queries.go:203` · found 2026-08-22 · hunt `general-2026-08-21` · lens `db-storage` + +A message delete is a soft delete: the `messages` row survives, so `attachments.message_id` stays set and the row never becomes an orphan. The only code path that ever removes a stored blob from disk is the maintenance sweep's `DeleteOrphanedAttachments`, whose SQL is `DELETE FROM attachments WHERE message_id IS NULL AND uploaded_at < ? ...`. Nothing else deletes attachment files (grep for `store.Delete`/`fileStorage.Delete` finds only the orphan sweep, the emoji delete, and the upload-failure rollback). The bytes are simultaneously made unreachable (`serveFileResolve` 404s an attachment whose message is deleted) and permanently retained. + +**Repro:** 1. As any user, POST /api/v1/files with a 25 MB image → file written to `data/uploads/`, `attachments` row with `message_id NULL`. +2. Send a message with that attachment id → `LinkAttachmentsToMessage` sets `message_id = `. +3. Delete the message (DELETE /channels/{id}/messages/{msgID}, or bulk purge, or the author's account deletion). `SoftDeleteMessage` runs `UPDATE messages SET deleted = 1 WHERE id = ?`; the attachments row is untouched and still has `message_id = `. +4. Wait any number of 15-minute maintenance ticks: `DeleteOrphanedAttachments` never matches the row (`message_id IS NULL` is false), so `data/uploads/` is never deleted. +5. Meanwhile GET /api/v1/files/ returns 404 (serveFileResolve's deleted-message check), so the 25 MB is unreachable AND unreclaimable. Repeat N times → N×25 MB of permanently dead disk, driven entirely by an ordinary user. + +**Evidence:** Server/db/message_queries.go:203 `if err := d.q.SoftDeleteMessage(ctx, id); err != nil {` (SQL: `UPDATE messages SET deleted = 1 WHERE id = ?` — attachments untouched) +Server/db/message_queries.go:263-266 purge: `UPDATE messages SET deleted = 1 WHERE id IN (...)` — same, no unlink +Server/db/account.go:74-76 `UPDATE messages SET deleted = 1, content = '' WHERE user_id = ?` — same, no unlink +Server/db/queries/sqlite/attachments.sql:22-28 `DELETE FROM attachments WHERE message_id IS NULL AND uploaded_at < ? ... RETURNING stored_as;` (the ONLY reaper) +Server/main.go:610-620 the only caller of `fileStorage.Delete` for attachments +Server/api/upload_handler.go:381-384 comment already states the fact: "no sweep can ever reclaim a linked row either, since the only reaper requires message_id IS NULL" +Contrast — the project treats this exact invariant as load-bearing elsewhere: Server/db/dm_queries.go:367-373 and Server/db/account.go:293-304 both explicitly `UPDATE attachments SET message_id = NULL` before a channel delete "so the sweep already reclaims them", and migration 030 was written solely to preserve it. + +**Suggested fix:** Widen the single shared reaper instead of editing every delete path: in Server/db/queries/sqlite/attachments.sql, change DeleteOrphanedAttachments' predicate to `WHERE uploaded_at < ? AND (message_id IS NULL OR EXISTS (SELECT 1 FROM messages m WHERE m.id = attachments.message_id AND m.deleted = 1)) AND NOT EXISTS (SELECT 1 FROM users u WHERE u.avatar = '/api/v1/files/' || attachments.id)` and regenerate via the db-change skill. That covers single delete, purge and account deletion at once, keeps the avatar guard, and does not reopen the serve path (unlinking message_id instead would make the file downloadable again by the uploader and by admins via serveFileAuthorize's ChannelID==nil branch, upload_handler.go:429-458). + +**Fixed:** `140633a` · test `Server/db/attachment_orphan_softdelete_test.go` · revert-proof pass + +### OC-0280 — medium — Any DM-store change destroys and recreates the whole DM sidebar, wiping the "Find a conversation" filter and stealing keyboard focus mid-typing + +`Client/tauri-client/src/pages/main-page/SidebarArea.ts:664` · found 2026-08-22 · hunt `general-2026-08-21` · lens `client-state` + +In `dms` sidebar mode, `refreshDmSidebar()` calls `activeSidebarContent.destroy()` + `clearChildren(contentSlot)` and rebuilds a brand-new `createDmSidebar(...)`, and it is subscribed to `dmStore.subscribeSelector((s) => s.channels, ...)`. `DmSidebar.mount()` creates the `.dm-search` input fresh with an empty value and only filters via a local `input` listener over the closure-captured `items` array, so the search text and the input's focus are state that lives only in the destroyed subtree. dmStore's `channels` array identity changes far more often than "the DM list changed": `updateDmParticipant` (dispatcher PRESENCE handler) rebuilds it on any DM partner's status flip, `updateDmLastMessage`/`updateDmLastMessagePreview` on any DM message including the user's own, and `clearDmUnread` unconditionally recreates the matching channel object even when both counts are already 0. + +**Repro:** 1) Open the full DM sidebar ("View all messages", sidebarMode = "dms") with at least two conversations. 2) Click into "Find a conversation" and type `ali` — the list filters and the caret is in the input. 3) Have any DM partner change presence (online -> idle) or send/receive any DM message. The dispatcher calls `updateDmParticipant` / `updateDmLastMessage`, dmStore's `channels` reference changes, `refreshDmSidebar()` destroys the subtree and mounts a new one: the filter box is empty, every conversation is shown again, and focus has moved to `document.body`, so the rest of the user's keystrokes go nowhere. On a server where DM partners' presence flips regularly the search box is effectively unusable. (Note `tests/unit/dm-sidebar.test.ts` exercises the filter on a standalone `createDmSidebar` only — nothing locks the refresh behaviour.) + +**Evidence:** SidebarArea.ts:664-678 + function refreshDmSidebar(): void { + if (activeSidebarContent !== null) { activeSidebarContent.destroy?.(); } + clearChildren(contentSlot); + const freshDm = buildDmSidebar(); + freshDm.mount(freshSlot); + ... + } +SidebarArea.ts:684-690 + const unsubDmStore = dmStore.subscribeSelector((s) => s.channels, () => { refreshDmSidebar(); }); + +DmSidebar.ts:310-314 const searchInput = createElement("input", { class: "dm-search", placeholder: "Find a conversation" }); +DmSidebar.ts:334-353 searchInput.addEventListener("input", () => { const q = searchInput.value.trim().toLowerCase(); items.forEach(...) }, { signal: ac.signal }); + +dm.store.ts:236-252 updateDmParticipant -> returns `{ channels }` (new array, patched objects) for any DM partner presence/profile change +dm.store.ts:189-195 clearDmUnread -> always maps to a new object for the matching channel, so the selector always fires + +**Suggested fix:** Preserve the search state across the rebuild in the single shared place, `refreshDmSidebar`: before `activeSidebarContent.destroy?.()`, capture `const oldInput = contentSlot.querySelector(".dm-search"); const q = oldInput?.value ?? ""; const hadFocus = oldInput !== null && document.activeElement === oldInput; const caret = oldInput?.selectionStart ?? null;` and after `freshDm.mount(freshSlot)` restore it: `const newInput = freshSlot.querySelector(".dm-search"); if (newInput !== null && q !== "") { newInput.value = q; newInput.dispatchEvent(new Event("input")); } if (newInput !== null && hadFocus) { newInput.focus(); if (caret !== null) newInput.setSelectionRange(caret, caret); }`. (A cleaner long-term variant is an `initialQuery` option on `createDmSidebar` plus a `dmQuery` variable in `createSidebarArea`, but the above needs no signature change and fixes every refresh path — dmStore, activeChannelId, and the mute-toggle `refreshDmSidebarRef?.()` at SidebarArea.ts:543 — at once.) + +**Fixed:** `4da361a` · test `Client/tauri-client/tests/unit/sidebar-area.test.ts` · revert-proof pass + +### OC-0281 — medium — Voice E2EE identity-mismatch modal is bound to the sidebar's per-render signal, so an unrelated re-render destroys the TOFU re-pin prompt (or swallows it before it opens) + +`Client/tauri-client/src/components/ChannelSidebar.ts:497` · found 2026-08-22 · hunt `general-2026-08-21` · lens `lifecycle` + +The mismatch badge passes the per-render `signal` (`currentRenderAc.signal`, handed down at line 823) into `openIdentityMismatchModal`, which registers `signal.addEventListener("abort", closeIdentityModal, { once: true })` at line 152 to "close if the owning sidebar is destroyed". That signal is not the sidebar's lifetime signal — `renderChannels()` aborts and replaces it on every render (line 795). Any store change that re-renders the sidebar therefore destroys the open security prompt, and a re-render landing during the modal's `await computeKeyFingerprint(...)` trips the `if (signal.aborted) return` guard so the click produces nothing at all. + +**Repro:** In a voice call, a peer's identity key changes so the row shows the red mismatch badge. Click it -> the re-pin modal mounts. Now any of these fire while the user is comparing the fingerprint out-of-band: a chat message arrives in any non-active channel (dispatcher -> incrementUnread -> `channels: new Map(...)` in channels.store.ts:359 -> `subscribeSelector((s) => s.channels)` -> renderChannels()), or any voice peer toggles mute/camera/screenshare (the voiceStore structural-signature subscription), or the WS flips to reconnecting. renderChannels() runs `renderAc?.abort()`, the abort listener fires `closeIdentityModal()`, and the modal disappears mid-verification with no way to tell it happened. In the pre-click window it is worse: a re-render during the `importIdentityPublicKey`/`computeKeyFingerprint` round trip makes the click a silent no-op. No test covers this — channel-sidebar.test.ts only asserts the modal closes on `sidebar.destroy()` (line ~2226), which passes because destroy() also aborts renderAc. + +**Evidence:** ChannelSidebar.ts:795-796 renderAc?.abort(); const currentRenderAc = new AbortController(); +ChannelSidebar.ts:823 currentRenderAc.signal, // -> renderCategoryGroup -> renderChannelItem -> renderVoiceChannelItem(signal) +ChannelSidebar.ts:497 void openIdentityMismatchModal(user.userId, user.username || "Unknown", signal); +ChannelSidebar.ts:126 if (signal.aborted) return; // after the async fingerprint compute +ChannelSidebar.ts:152 signal.addEventListener("abort", closeIdentityModal, { once: true }); + +**Suggested fix:** Give openIdentityMismatchModal the sidebar-lifetime signal instead of the render signal: thread `ac.signal` down beside the per-render one (a single extra parameter through renderCategoryGroup/renderChannelItem/renderVoiceChannelItem) and use it only for the :152 abort bridge; keep `{ signal }` (per-render) on the badge's click listener at :497 so OC-0229's retention fix stays intact. + +**Fixed:** `0e3435a` · test `Client/tauri-client/tests/unit/channel-sidebar.test.ts` · revert-proof pass + +### OC-0282 — medium — Sidebar right-click popovers (channel context menu, voice-user volume/moderation menu) close themselves on any unrelated sidebar re-render + +`Client/tauri-client/src/components/ChannelSidebar.ts:610` · found 2026-08-22 · hunt `general-2026-08-21` · lens `lifecycle` + +Both popovers receive the per-render signal and register their "parent destroyed" bridge on it: context-menu.ts:188 `signal.addEventListener("abort", closeMenu, { signal: menuAc.signal })` and volume-menu.ts:136 `signal.addEventListener("abort", () => { menu.remove(); dismissAc.abort(); }, ...)`. Since `renderChannels()` aborts that controller on every render, an open menu is dismissed by events that have nothing to do with it — a message in another channel, a peer toggling their mic, the connection flipping to reconnecting. The per-item click handlers are also registered with the same `{ signal }`, so they are dead the moment the render is superseded. + +**Repro:** Right-click a text channel to open the Mark as Read / Mute / Edit / Delete / Purge menu. A message lands in any other channel -> incrementUnread -> new `channels` Map -> renderChannels() -> `renderAc?.abort()` -> closeMenu() -> the menu vanishes before it can be clicked; on a busy server it is effectively unusable. Same for the voice-user right-click menu: open it on a participant to reach the volume slider or Server Mute / Server Deafen / Move / Disconnect, then have any peer in that channel toggle mute or camera — the voiceStore structural-signature subscription re-renders and the menu is torn out from under the pointer (mid-drag on the volume slider included). + +**Evidence:** ChannelSidebar.ts:610 attachChannelContextMenu(el, channel, signal, onEditChannel, onDeleteChannel, onPurgeChannel); +ChannelSidebar.ts:517 showUserVolumeMenu(user.userId, user.username || "Unknown", e.clientX, e.clientY, signal, buildVoiceModOptions(...)); +context-menu.ts:188 signal.addEventListener("abort", closeMenu, { signal: menuAc.signal }); +volume-menu.ts:136-142 signal.addEventListener("abort", () => { menu.remove(); dismissAc.abort(); }, { signal: dismissAc.signal }); +ChannelSidebar.ts:795 renderAc?.abort(); // fires both bridges on every render + +**Suggested fix:** Same single root fix: thread the sidebar-lifetime `ac.signal` down alongside the per-render signal and hand it to attachChannelContextMenu and showUserVolumeMenu as the teardown-bridge owner (context-menu.ts:188, volume-menu.ts:136) and as the owner of the context menu's item click listeners; keep the per-render signal on the row-level `contextmenu` listener (context-menu.ts:53) so the OC-0229 retention fix and its test still hold. + +**Fixed:** `0e3435a` · test `Client/tauri-client/tests/unit/channel-sidebar.test.ts` · revert-proof pass + +### OC-0283 — medium — OC-0239's `stillInRoster` parameter is true for every genuine departure, so the departed-peer key retirement (OC-0020) never runs in production + +`Client/tauri-client/src/lib/livekitE2EE.ts:1298` · found 2026-08-22 · hunt `general-2026-08-21` · lens `flow-voice` + +`handleParticipantLeft` only retires a departing peer's ephemeral key when `!stillInRoster`, but the sole production caller computes `stillInRoster` from the roster snapshot taken *before* `removeVoiceUser()` — and a peer is always present in `voiceUsers` right up until the `voice_leave` frame that removes them. The flag is therefore `true` on a genuine departure just as it is on the stale/superseded-rejoin case it was added to detect, so the guard is unconditionally short-circuited and `_retiredPeerKeys` is never populated on leave. The only other writer of `_retiredPeerKeys` is `handleAnnounceInner`'s in-session key-change branch, which cannot fire after a leave because `handleParticipantLeft` already deleted the peer from `_peerPublicKeys` (so `existingKey` is undefined and the retire branch is skipped). The OC-0020 replay defence is dead. + +**Repro:** Alice (key holder) and Bob are in voice channel 7; Bob has announced ephemeral key A, which Alice holds in `_peerPublicKeys`. Bob leaves: the server broadcasts `voice_leave{channel_id:7,user_id:bob}`. Alice's dispatcher reads `voiceUsers.get(7).has(bob)` → **true** (Bob's `voice_state` put him there on join and nothing has removed him yet), calls `removeVoiceUser`, then `handleParticipantLeft(bob, true)`. Line 1298 evaluates `departingKey && !true && ...` → false, so key A is never added to `_retiredPeerKeys`. Bob rejoins and announces fresh key B; `handleAnnounceInner` finds no `existingKey`, so it also never retires A. A malicious or compromised relay now re-emits Bob's still-validly-signed pre-leave announce for key A (the signed message carries no channel/epoch/nonce, per the F3 comment at livekitE2EE.ts:58-63): `isRetiredPeerKey` returns false, `verifyPeerAnnounce` passes against Bob's unchanged pinned identity key, and `_peerPublicKeys.set(bob, keyA)` overwrites the live key. Alice, as holder, then wraps the *current* room key under key A — decryptable by whoever holds A's private half and undecryptable by Bob, who is blackholed for the rest of the call. The existing test `[OC-0020]` passes only because it calls `mgr.handleParticipantLeft(PEER_ID)` with the default `stillInRoster = false`, which no production caller ever does (tests/unit/livekit-e2ee.test.ts:1281). + +**Evidence:** livekitE2EE.ts:1254 `async handleParticipantLeft(userId: number, stillInRoster = false)` +livekitE2EE.ts:1298 `if (departingKey && !stillInRoster && !channelUsers?.has(userId)) {` +livekitE2EE.ts:1299 ` this.retirePeerKey(userId, await exportPublicKey(departingKey));` +dispatcher.ts:1062 `const stillInRoster =` +dispatcher.ts:1063 ` voiceStore.getState().voiceUsers.get(payload.channel_id)?.has(payload.user_id) ?? false;` +dispatcher.ts:1064 `removeVoiceUser(payload);` +dispatcher.ts:1080 `void handleParticipantLeft(payload.user_id, stillInRoster);` +voice.store.ts:231 `nextUsers.set(payload.user_id, { userId: payload.user_id, ... })` // updateVoiceState keeps every in-channel peer in the roster until their voice_leave +livekitE2EE.ts:836 `if (!isDuplicate) { this._peerPublicKeys.set(userId, peerKey); ... }` // no retire on first-sight re-announce after a leave + +**Suggested fix:** Delete the `!stillInRoster` term at livekitE2EE.ts:1298 (and the now-unused parameter plus the dispatcher.ts:1062-1063 snapshot), restoring unconditional retirement on leave — it is the only replay defence and the roster cannot distinguish the two cases it was added to separate, since the pre-mutation snapshot is identical for a genuine departure and for a stale superseded leave. The OC-0213 stale-leave case needs a discriminator the roster does not carry: have the server stamp the join instance on `voice_leave` (an epoch/join id, matching what `_peerOfferEpochs` already tracks) and drop a leave whose epoch is older than the peer's current announce, rather than suppressing retirement. + +**Fixed:** `3767be1` · test `Client/tauri-client/tests/unit/dispatcher.test.ts` · revert-proof pass + +### OC-0284 — medium — chat_delete is idempotent-but-not-guarded, so a repeated delete decrements mention_count again and wipes an unrelated, genuinely-unread mention badge + +`Server/service/message_crud.go:477` · found 2026-08-22 · hunt `general-2026-08-21` · lens `flow-message` + +MessageService.DeleteMessage has no `msg.Deleted` guard (its siblings EditMessage, handleReaction and SetMessagePinned all have one), and every layer beneath it is silently idempotent: db.DeleteMessage does not check `deleted`, and `SoftDeleteMessage` is a bare `UPDATE messages SET deleted = 1 WHERE id = ?`. So a second chat_delete for the same message id succeeds and runs `DecrementMentionCounts(channelID, []int64{msgID})` a second time. That statement decrements every recipient row in `message_mentions` for that message whose `last_message_id < msgID` and `mention_count > 0` — it has no per-message idempotence, only a monotonic floor at 0 — so the second run eats one mention that a *different, still-live* message raised. + +**Repro:** In text channel #general: Alice sends M1 = "@bob a" (id 100) and M2 = "@bob b" (id 101). Bob has never focused the channel, so read_states(bob,#general) = (last_message_id 0, mention_count 2). Alice (or any MANAGE_MESSAGES holder) sends `{"type":"chat_delete","payload":{"message_id":100}}` twice — chat_delete's rate limit is 10/s, so both frames are accepted. First call: SoftDeleteMessage sets deleted=1, DecrementMentionCounts drops Bob to 1 (correct). Second call: GetMessage still returns row 100 (tombstones are returned), the Deleted flag is never inspected, the ownership/permission checks pass again, SoftDeleteMessage is a no-op UPDATE that returns nil, and DecrementMentionCounts drops Bob to 0. Bob's red mention badge for M2 disappears even though M2 is live, unread, and mentions him; nothing ever restores it (only IncrementMentionCounts raises it, and only on the original insert). The non-adversarial version is two moderators clicking Delete on the same message before either sees the chat_deleted broadcast, or the author deleting their own message while a moderator's client still shows it live. + +**Evidence:** Server/service/message_crud.go:419-422 — `msg, err := s.st.GetMessage(ctx, msgID); if err != nil || msg == nil { return ... ErrForbidden }` (no `if msg.Deleted { ... }`, unlike EditMessage:309-311 `if msg.Deleted { return ... ErrDeletedMessage }`). +Server/service/message_crud.go:463-465 — `if err := s.st.DeleteMessage(ctx, msgID, userID, isMod); err != nil { ... }` +Server/service/message_crud.go:477 — `if mcErr := s.st.DecrementMentionCounts(context.WithoutCancel(ctx), msg.ChannelID, []int64{msgID}); mcErr != nil {` +Server/db/message_queries.go:191-207 — db.DeleteMessage checks ownership only, then `d.q.SoftDeleteMessage(ctx, id)` and returns nil. +Server/db/queries/sqlite/messages.sql:23-24 — `-- name: SoftDeleteMessage :exec` / `UPDATE messages SET deleted = 1 WHERE id = ?;` (no `AND deleted = 0`, and `:exec` so rows-affected is discarded). +Server/db/queries/sqlite/messages.sql:6-9 — GetMessage has no `deleted = 0` filter, so the tombstone is returned as a normal row. +Server/db/mention_queries.go:274-281 — `UPDATE read_states SET mention_count = mention_count - 1 WHERE channel_id = ? AND mention_count > 0 AND last_message_id < ? AND user_id IN (SELECT mentioned_user_id FROM message_mentions WHERE message_id = ?)` — nothing records that this message was already reversed. + +**Suggested fix:** Make the soft delete a compare-and-set and skip the mention reversal when it did not transition, which fixes both the sequential repeat and the concurrent race in one place. In Server/db/queries/sqlite/messages.sql change SoftDeleteMessage to `-- name: SoftDeleteMessage :execresult` / `UPDATE messages SET deleted = 1 WHERE id = ? AND deleted = 0;`, regenerate via the db-change skill, then in db.DeleteMessage (Server/db/message_queries.go:189-206) check RowsAffected() == 0 and return a sentinel (ErrNotFound or a new ErrAlreadyDeleted). MessageService.DeleteMessage then returns ErrDeletedMessage on that sentinel before reaching the DecrementMentionCounts call at message_crud.go:477. Adding `if msg.Deleted { return nil, fmt.Errorf("%w: cannot delete this message", ErrDeletedMessage) }` after message_crud.go:422 is a cheap complement that saves the extra round trip, but it is not sufficient on its own. + +**Fixed:** `d08c7e0` · test `Server/service/mentions_test.go` · revert-proof pass + +### OC-0285 — medium — kickClient never stops readPump, so a banned / revoked WS principal keeps executing fully authorized commands after the server decides to cut it off + +`Server/ws/hub_sweep.go:57` · found 2026-08-22 · hunt `general-2026-08-21` · lens `flow-session` + +kickClient only deletes the hub entry, closes the send channels and unsubscribes topics — it never touches the WebSocket and never signals readPump. readPump (serve_pumps.go:203-211) loops on conn.Read and calls hub.handleMessage for every frame with no check of c.isSendClosed() or hub membership (isSendClosed is consulted only by pubsub.Subscribe, pubsub.go:94). The connection only dies once writePump finishes draining and calls conn.Close, and each drained frame is written under a 10 s writeTimeout (serve_pumps.go:15-23), a delay the remote peer controls. Everything the kicked user sends in that window is dispatched with full authority: nothing on the per-message path re-checks ban/session state except handleMessageSessionRecheck itself, which handlers.go:116 has just reset (c.msgCount = 0), so the next nine frames skip revalidation entirely. + +**Repro:** User U is connected and holds SEND_MESSAGES on #general. U's client keeps sending frames but stops reading its socket (or simply lets its TCP receive window fill). An admin bans U via PATCH /admin/api/users/{id} {banned:true} while U is offline from the hub's point of view for that instant, or U is force-logged-out via DELETE /admin/api/users/{id}/sessions. On U's next 10th frame, handleMessageSessionRecheck reads the ban/missing session, queues the BANNED error frame and calls kickClient. writePump wakes on the closed channel, picks up that still-buffered frame and blocks in conn.Write for the full 10 s writeTimeout because the peer window is shut. Throughout those 10 s readPump keeps reading U's pipelined frames and handleMessage executes each one end to end: chat_send persists the message and broadcasts it to every other client in the channel, reaction/mark_read/channel_focus mutate state, voice_join writes a voice_states row and mints a LiveKit token. U sees no replies (sendMsg no-ops on a closed channel), so the writes are silent from U's side but visible to everyone else. Only when the write deadline fires does coder/websocket close the conn and end readPump. In the benign case the same defect still lets every frame already buffered in the kernel through, and handlers.go:116's counter reset means the nine frames after any kick are never revalidated. No test pins the current behavior (Server/ws/handlers_test.go:253 and :1910 assert only that the kick happens). + +**Evidence:** hub_sweep.go:54-69 — "closes its send channel, which causes writePump to exit and the WebSocket connection to close": + func (h *Hub) kickClient(c *Client) { + h.mu.Lock(); ... delete(h.clients, c.userID); h.mu.Unlock() + c.closeSend() + h.pubsub.UnsubscribeAll(c) + } // no conn.Close, no cancel, no flag readPump reads + +serve_pumps.go:203-211 — + for { + _, msg, err := conn.Read(ctx) + if err != nil { lastReadErr = err; return } + c.touch() + hub.handleMessage(c, msg) // no isSendClosed()/registration guard + } + +handlers.go:112-143 — + c.msgCount++ + shouldCheck := c.msgCount >= SessionCheckInterval + if shouldCheck { c.msgCount = 0 } // reset BEFORE the kick below + ... + if result == nil || auth.IsSessionExpired(result.ExpiresAt) { h.kickClient(c); return true } + ... + c.sendMsg(buildErrorMsg(ErrCodeBanned, "you are banned")) + h.kickClient(c); return true + +serve_pumps.go:15-23 — every drained frame is written under writeTimeout (serve.go:22 = 10 * time.Second) before writePumpDrainAndClose reaches conn.Close. + +**Suggested fix:** Add one guard at the top of the shared dispatch function instead of at each kick site — in Server/ws/handlers.go, first statement of handleMessage: `if c.isSendClosed() { return }`, before the handleMessageSessionRecheck call. c.isSendClosed() is already the canonical 'this client has been cut off' flag (set by closeSend under c.mu, the same flag pubsub.Subscribe uses as its re-take guard), it is set synchronously by kickClient before it returns, and handleMessage is the only path a frame reaches a handler through. This drops every post-kick frame regardless of which goroutine kicked, without touching writePump's drain-before-close contract that TestWritePump_DrainsQueuedFramesAfterCloseSend locks. It also makes the msgCount reset harmless. Optionally pair it with a conn close-deadline in kickClient, but that is not required to close the hole. + +**Fixed:** `51b3144` · test `Server/ws/handlers_test.go` · revert-proof pass + +### OC-0286 — medium — MessageList registers every message row's listeners on the component-lifetime AbortSignal, so each rebuild permanently retains a full window of detached rows + +`Client/tauri-client/src/components/MessageList.ts:332` · found 2026-08-22 · hunt `general-2026-08-21` · lens `hotspot-client-tauri-client-src-components` + +`createMessageList` has exactly one `AbortController` (`ac`, line 244), aborted only in `destroy()`. `renderVirtualItem` hands `ac.signal` to `renderMessage`, and `renderWindow` does `releaseTrackedMedia(); clearChildren(contentContainer);` then re-renders the whole window (lines 563-569). `addEventListener(..., { signal })` keeps a strong abort-algorithm entry (holding the event target) on that signal until it fires, so every rebuild leaves one full set of discarded rows — and everything they reference (videos, images, embeds, reaction tooltips) — reachable for the rest of the channel visit. This is the exact defect the codebase already fixed in ChannelSidebar (`renderAc`, comment at ChannelSidebar.ts:748-757 citing OC-0229), SettingsOverlay (`renderAC` at SettingsOverlay.ts:104-139), reaction-tooltip.ts (`hoveringChips`, comment at lines 232-241) and MessageList's own scrollToMessage flash timer (ledger MessageList.ts:1021). MessageList's row rendering is the one path none of those fixes covered, and it is by far the highest-volume one. + +**Repro:** Open a channel with a few hundred messages. Each `renderMessage` registers ~8-10 listeners on `ac.signal` (react/reply/pin/edit/delete/copy-link buttons, the reply-ref click+keydown, and per reaction chip click+keydown+4 tooltip listeners via reactions.ts:36-47). Scroll up and down so the visible range leaves the ±20 overscan ~50 times, or trigger 50 `renderAll()` rebuilds (any edit, delete, reaction_update, history prepend, or membersStore roleRevision bump). Every rebuild clears `contentContainer` and re-renders ~40-60 rows against the same signal, so after 50 rebuilds ~2000-3000 detached `.message` subtrees (with their