Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

MSC4037: Thread root is not in the thread #4037

Closed
wants to merge 18 commits into from
Closed
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
129 changes: 129 additions & 0 deletions proposals/4037-thread-root-is-not-in-thread.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,129 @@
# MSC4037: Thread root is not in the thread

The current spec implies that a thread root is considered within the thread, but
we argue that this does not make sense, and a thread root is not "in" the thread
branching from it, and neither are its non-thread children (e.g. edits).

This is important for creating and interpreting read receipts.

## Motivation

The current spec, in
[11.6.2.2 Threaded read receipts](https://spec.matrix.org/v1.7/client-server-api/#threaded-read-receipts)
says:

> An event is considered to be "in a thread" if it meets any of the following
> criteria:
>
> * It has a `rel_type` of `m.thread`.
> * It has child events with a `rel_type` of `m.thread` (in which case it’d be
> the thread root).
> * Following the event relationships, it has a parent event which qualifies for
> one of the above. Implementations should not recurse infinitely, though: a
> maximum of 3 hops is recommended to cover indirect relationships.
>
> Events not in a thread but still in the room are considered to be part of the
> "main timeline", or a special thread with an ID of `main`.

This explicitly includes thread roots (and their non-thread children) in the
thread which branches off them, and implicitly _excludes_ those messages from
being in the `main` thread.

This is problematic because:

* It seems natural for messages that are displayed in the main timeline (as
thread roots are in most clients) to be considered read/unread when the user
reads them in the main timeline.

* It normally does not make sense for a threaded read receipt to point at the
thread root, since the user has not really read anything in that thread if
they have only read the thread root.

In practice, Synapse
[returns an error for any request to mark the thread root as read within the thread](https://github.com/matrix-org/synapse/blob/v1.87.0/synapse/rest/client/receipts.py#L116-L154),
and accepts requests to mark it as read in the main timeline.
When it reports thread notifications, it excludes thread roots (and e.g. edits
to thread roots) from the thread count, only showing them in the main timeline
count.

In consequence, Element Web exhibited bugs relating to unread rooms while its
underlying library used spec-compliant behaviour, many of which were fixed by
[adopting the behaviour recommended by this proposal](https://github.com/matrix-org/matrix-js-sdk/pull/3600).
andybalaam marked this conversation as resolved.
Show resolved Hide resolved

It really does not make sense to treat thread roots as outside the main
timeline: any message can become a thread root at any time, when a user creates
a new threaded message pointing at it, so suddenly switching which receipts are
allowed to apply to it would not be sensible.

Similarly, it does not make sense for reactions to the thread root (or other
related events such as edits) to be outside the main timeline, for similar
reasons: the message we are reacting to can become a thread root at any time,
making our previous receipt invalid retrospectively. (We could conceivably allow
receipts to exist both within a thread and the main timeline, but this does not
match the expected user mental model: I have either read a reaction/edit/reply,
or I have not - I don't want to have to read it twice just because it appears in
two places in the UI.)

## Proposal
andybalaam marked this conversation as resolved.
Show resolved Hide resolved

We propose that thread roots and their non-thread children are in the main
timeline, making the definition:

> An event is considered to be "in a thread" if:
>
> * It has a `rel_type` of `m.thread`, or it has an ancestor event with this
andybalaam marked this conversation as resolved.
Show resolved Hide resolved
> `rel_type`.
>
> Implementations should limit recursion to find ancestors: a maximum of 3 hops
> is recommended.
>
> Events not in a thread but still in the room are considered to be part of the
> "main timeline": a special thread with an ID of `main`.
>
> Note: thread roots (events that are referred to in a `m.thread` relationship)
> are in the main timeline. Similarly, reactions to thread roots, edits of
> thread roots, and other events with non-thread relations to a thread root are
> in the main timeline.

## How we got here

The MSC that introduced read receipts for threads is
[MSC3771](https://github.com/matrix-org/matrix-spec-proposals/pull/3771).

The relevant wording is in the
[Proposal](https://github.com/matrix-org/matrix-spec-proposals/blob/main/proposals/3771-read-receipts-for-threads.md#proposal)
section:

> notifications generated from events with a thread relation matching the
> receipt’s thread ID prior to and including that event which are MUST be marked
> as read

Notably it only mentions things "**with a thread relation**", so it appears to
match the wording of this proposal.

It comes tantalisingly close to covering these issues in the example it uses,
but unfortunately does not cover what would happen if we received a receipt for
a thread root or for e.g. an edit of a thread root.

## Potential issues

None known.

## Alternatives

We could treat thread roots as being in *both* their thread and the `main`
timeline, but it does not offer much benefit because a thread where only the
root message has been read is almost identical to one where the no messages have
been read. A thread cannot exist without at least one additional message.

## Security considerations

Unlikely to have any security impact.

## Unstable prefix

None needed.

## Dependencies

No dependencies.