Stuck unread with reaction + start thread

[weeman1337]

Steps to reproduce

1. Have two users A and B in a room
2. Send a message with A
3. React to this message with B
4. Start a thread from the message with B
5. Look at thread as A

Outcome

What did you expect?

Unread count cleared

What happened instead?

Stuck with (1) unread for the room as user A

Operating system

No response

Browser information

Seems unrelated, happens on Desktop + Web

URL for webapp

No response

Application version

Element Nightly version: 2023013101 Olm version: 3.2.12

Homeserver

No response

Will you send logs?

Yes

[germain-gg]

I have investigated this issue and here are a few thoughts that I'm writing down so that this does not get lost. I have not yet managed to write a fix for this, and this will probably happen over a handful of pull requests

The fundamental issue causing this bug is that we do not insert the events in the thread timeline in the order of the DAG

---

Events related to the thread root should be ignored when sending read receipts from a thread timeline

Thread events are paginated from the /relations API. This fetches only the m.thread and the 1st degree relations to those events.
But in order to achieve the desired UX, the Element client will insert the relations to the root event (annotations, redactions, ...) in that timeline too.

This is problematic as the timeline code only inspects the last event of a thread and sends a read receipt against that event. Instead the logic should inspect the events from most recent to oldest until it find a suitable one.

replayEvents breaks the DAG ordering

This patch was added as threads can be discovered either via /sync or via /relations. Sometimes a thread will come down all at once via a /sync response, and in those cases the client will get confused when it has to apply message edits as it hasn't fully determined yet where the original message belongs (in a thread or in the main timeline).

For that reason we have added the concept of replayEvents in the thread model, that replays and applies the edits once the thread has been fully initialised and when we can know for certain whether those edits are in the correct context.

The timeline only allows to append or prepend events, but the approach described above would require having a way to slice those events into place, otherwise the DAG order is not respected and we run the risk of having stuck notifications.

[clokep]

@gsouquet and I talked a bit about this in a DM, but wanted to iterate on it here.

This is problematic as the timeline code only inspects the last event of a thread and sends a read receipt against that event. Instead the logic should inspect the events from most recent to oldest until it find a suitable one.

This shouldn't be necessary, part of MSC3771 was written so clients could send a receipt which they think is part of a thread and it should most likely work:

It is recommended that at least 3 relations are traversed when attempting to find a thread, implementations should be careful to not infinitely recurse.

Using the example from the MSC:

flowchart RL
    subgraph "Main" timeline
    B-->A
    I-->B
    end
    subgraph Thread A timeline
    C-->A
    E-->C
    G-.->|m.reaction|C
    H-.->|m.edit|E
    end
    subgraph Thread B timeline
    D-->B
    F-->D
    end

Loading

It should be possible to send receipts on events G or H and the server will treat that receipt as properly part of the thread. If the server does not think the event belongs to a thread it will throw an error. This doesn't mean there isn't a bug in Synapse, but theoretically it sounds like it is working OK.

There is some subtly though -- in the case where the reaction (or any non-m.thread related event) is to the root event then you can use that event on either the main timeline or the thread timeline; maybe we're not including the thread_id field in this case if you're looking at the thread?

[weeman1337]

@gsouquet is it correct, that this issue also covers message edits in threads?

[germain-gg]

the symptoms appear a bit more often with reactions, but also with edits.

The issue occurs when they end up out of order compared to the real order that synapse holds.

I'm working on a fix at the moment

[t3chguy]

I can no longer reproduce this so I believe it to be fixed.