fix: read receipts of thread messages stuck as "sent"

Merged
RI
2mo ago
+76-0

Proposed changes (including videos or screenshots)

The problem. A thread reply only turns into a double check once the server has a read record from every member of the room, and it then clears messages up to the oldest of those records. Nothing ever wrote that record for the author of the reply, so the author stayed missing from the count and the gate never closed. Rooms don't have this problem: sending already counts as reading there, since notifyUsersOnMessage sets the sender's own ls right after saving. #27074 (e9667ed, 6.0.0) gave threads the gate, in MessageReadsService.readThread, without the equivalent write.

Why it usually looked fine. The record was written by a client side effect: the author's browser receives its own message back over the WebSocket and, while its local copy is still the unconfirmed (temp) one, treats it as new and calls chat.readThread on itself. Not a designed path — the receipt was riding on a dedup heuristic.

Why it broke. Whenever that heuristic doesn't run:

  1. No web client at all — replies posted through the API, a bot or a webhook. Nothing compensates, so it fails every time.
  2. The server's two replies arrive out of order. The browser gets its own message back twice: the POST response, which marks the local copy confirmed, and the WebSocket copy, which triggers readThread — but only while that copy is still unconfirmed, a check that keeps the client from treating its own message as one that just arrived from someone else. Response first, and the WebSocket copy is dismissed as already known, so readThread never runs. That is the normal order in a scaled deployment, where a separate process terminates the socket and the message travels farther back than the response does; bursts produce it too.
  3. The author leaves before the echo — closes the pane, switches room, reloads.

The call is skipped, not delayed: no retry, no catch-up. The only recovery was reopening the thread, which calls readThread regardless of any echo (useThreadMessagesQuery) — that writes the record late, the gate closes, and the stuck replies jump to double check at once.

The fix. processThreads now writes the record when the reply is saved, through afterReadMessages, the same hook the canonical read path uses. It runs after the metadata update, since readThread bails out while the parent's tlm is unset, and skips edits. The write happens before any broadcast, so arrival order stops mattering and the echo goes back to being just rendering.

Issue(s)

Steps to test or reproduce

Requires an EE license with Message_Read_Receipt_Enabled and Message_Read_Receipt_Store_Users on, and a channel with two members, A and B.

On a single-process dev instance the echo always wins, so the compensation always hides the bug. Emulating the extra hop of a scaled deployment makes it deterministic — in notifyOnMessageChange, apps/meteor/server/lib/notifyListener.ts:

// local only, revert afterwards
setTimeout(() => void api.broadcast('watch.messages', { message }), 300);

Restart, then:

  1. As A, post a message and open it as a thread.
  2. As B, reply once, from the UI.
  3. As A, open the thread and react to the reply, without replying.
  4. On develop the reply stays on a single check, every time. With this branch it turns into a double check.

Without touching code: have B reply through chat.sendMessage passing tmid — no browser to compensate, so it fails on a plain instance too. In the failing state, "Read receipts" lists only A. Use a fresh channel per round, and don't reopen the thread as B — that calls readThread and heals it.

Further comments

Summary by CodeRabbit

  • Bug Fixes
    • Thread replies now correctly update to “viewed” after the thread is read, instead of remaining marked as “sent.”
    • Edited replies and non-thread messages retain their existing read-status behavior.