Skip to content

Conversation

zecakeh
Copy link
Contributor

@zecakeh zecakeh commented Jul 25, 2025

And that event has the same format as in join rooms, with event_id and origin_server_ts, both for invite_state and knock_state.

This is a clarification because it has always been the case in some homeserver implementations. It's a bit hard to find places in code to point to because there is no direct link between the function where the format of the event can be seen and the function where the sync response is built.

Dendrite has a test that makes the expected output format clear:

https://github.com/element-hq/dendrite/blob/8d2da78744387e55c28bb8925ae0cc70dd3e02e3/syncapi/types/types_test.go#L86

Fixes #2138.

Pull Request Checklist

…mber` event of user

And that event has the same format as in join rooms, with `event_id` and
`origin_server_ts`.

This has always been the case in homeserver implementations.

Signed-off-by: Kévin Commaille <[email protected]>
@zecakeh zecakeh requested a review from a team as a code owner July 25, 2025 08:42
Signed-off-by: Kévin Commaille <[email protected]>
@turt2live
Copy link
Member

We spent quite a bit of time around Matrix 1.0 to ensure stripped state was in fact stripped state, including noting that Synapse in particular at the time over-populated events. This feels like a case where the ecosystem has standardized around Synapse's behaviour rather than the spec itself, but may indeed be reasonable to land as-is.

I'm currently torn on whether to ask for an MSC - thoughts are welcome.

@tulir
Copy link
Member

tulir commented Jul 26, 2025

From my point of view there are 2 main factors why it shouldn't be stripped:

  • The timestamp of the invite is useful information for clients/users.
  • The server already has all the information and there's no reason to withhold it from clients. The invite event isn't sent as stripped over federation like other state is (it can't be, considering the recipient server has to sign the PDU).
    • The AS API also sends invites in the top-level events array, so they can't be stripped there.

If ~all servers have already implemented it the way this PR describes, I'm not sure there's any benefit to an MSC

Comment on lines +345 to +347
MUST also include the `m.room.member` event of the user with a membership of
`knock`, and using the same event format as joined rooms with the `event_id` and
`origin_server_ts` fields.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why are knocks included in this, for consistency? Do servers do this currently?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, Synapse does the same, Dendrite doesn't support knocking it seems.

Copy link
Contributor

@Kladki Kladki Jul 27, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just checked, Conduit sends the knock event using the sync format if it's remote, and stripped if it's a local knock, with the former being a mistake really. 🫣 as stripped state, nevermind.

@zecakeh
Copy link
Contributor Author

zecakeh commented Jul 26, 2025

I am sorry, I actually made a mistake in the PR description, Conduit and forks only send the stripped member event.

Comment on lines 311 to +316
The [stripped state events](/client-server-api/#stripped-state) that form the
invite state.
MUST also include the `m.room.member` event of the user with a membership of
`invite`, and using the same event format as joined rooms with the `event_id`
and `origin_server_ts` fields.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we need to change more words here. Something like:

A list of events comprising:
  * The [stripped state](/client-server-api/#stripped-state) of the room.
  * The user's current  `m.room.member` event (with membership of `invite`), using the normal `ClientEventWithoutRoomID` format. 

Also we'll need to change the description on the InviteState object.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that we need to be more explicit about the difference between both of those, because the m.room.member should be a MUST, while the other events in the stripped state are a SHOULD.

@richvdh
Copy link
Member

richvdh commented Jul 29, 2025

I am sorry, I actually made a mistake in the PR description, Conduit and forks only send the stripped member event.

Given the inconsistency between servers, I'm leaning to the idea that an MSC would be better, if only so that we can discuss which is the correct behaviour here.

@zecakeh
Copy link
Contributor Author

zecakeh commented Jul 31, 2025

To be clear, what would be the goal of the MSC? Just say that the m.room.member event should use the ClientEventWithoutRoomID format instead of the stripped state format?

Because currently the spec doesn't mention this event at all in the stripped state, but it should theoretically be required in the invite_state and knock_state, because it is the reason why a room appears in invite or knock in the first place.

@richvdh
Copy link
Member

richvdh commented Jul 31, 2025

To be clear, what would be the goal of the MSC? Just say that the m.room.member event should use the ClientEventWithoutRoomID format instead of the stripped state format?

I guess that's up to the author of the MSC: they need to convince readers that it should be one format rather than the other.

@zecakeh
Copy link
Contributor Author

zecakeh commented Aug 1, 2025

I opened MSC4319 for this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

A users own invite event should not be stripped in sync

5 participants