Skip to content

Conversation

Half-Shot
Copy link
Contributor

@Half-Shot Half-Shot commented Oct 1, 2025

Split out from #5017 to allow for easy reviewing. Implements matrix-org/matrix-spec-proposals#4354

As a brief overview of the MSC (though please do at least skim the MSC): This allows any member in a room to mark an event as "sticky" and ensure it gets delivered to other users even in the case of a gappy sync. This is useful for things like RTC where you want call member events to always reach clients, without having to bloat the room with state events that will contribute to overall DAG bloat.

This implementation also deals with ensuring that some events will replace predecessors that share the same "sticky key", and emit the appropriate notices to users of the API.

Checklist

  • Tests written for new code (and old code if feasible).
  • New or updated public/exported symbols have accurate TSDoc documentation.
  • Linter and other CI checks pass.
  • Sign-off given on the changes (see CONTRIBUTING.md).

@Half-Shot Half-Shot requested a review from a team as a code owner October 1, 2025 10:59
@Half-Shot Half-Shot requested review from MidhunSureshR, dbkr and toger5 and removed request for MidhunSureshR and dbkr October 1, 2025 10:59
@toger5
Copy link
Contributor

toger5 commented Oct 2, 2025

This feels super close. I think the only open comment is the sync accumulator prune logic and the delay comment.
Do we have someone from the web reviewers to look at this?

@robintown robintown requested review from robintown and removed request for MidhunSureshR and dbkr October 2, 2025 14:38
Copy link
Member

@robintown robintown left a comment

Choose a reason for hiding this comment

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

I haven't yet reviewed the file for the RoomStickyEventsStore but hopefully this is a good start:

Copy link
Member

@robintown robintown left a comment

Choose a reason for hiding this comment

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

Still working through this bit by bit as I had some thoughts I wanted to leave on the MSC as well. Here are my comments so far:

Copy link
Member

@robintown robintown left a comment

Choose a reason for hiding this comment

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

Okay, got through the rest, I feel reasonably confident now that everything's doing what it's supposed to.

Copy link
Member

@robintown robintown left a comment

Choose a reason for hiding this comment

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

🎉

@robintown
Copy link
Member

robintown commented Oct 7, 2025

Oh, there is one recent change to the MSC: matrix-org/matrix-spec-proposals#4354 (comment)

If a client sends two sticky events in the same millisecond, the 2nd event may be replaced by the 1st if the event ID of the 1st event has a higher lexicographical event ID. To protect against this, clients should ensure that they wait at least 1 millisecond between sending sticky events.

I'm happy to have that wait for a follow-up PR, or simply be an outstanding task for bulletproofing the implementation.

@Half-Shot
Copy link
Contributor Author

I'm not convinced this should be up to the client but that might be a fight for the spec. Let's see where this leads and follow up with a PR if required. It's certainly fairly edge-cased to me.

Oh, there is one recent change to the MSC: matrix-org/matrix-spec-proposals#4354 (comment)

If a client sends two sticky events in the same millisecond, the 2nd event may be replaced by the 1st if the event ID of the 1st event has a higher lexicographical event ID. To protect against this, clients should ensure that they wait at least 1 millisecond between sending sticky events.

I'm happy to have that wait for a follow-up PR, or simply be an outstanding task for bulletproofing the implementation.

@Half-Shot Half-Shot added this pull request to the merge queue Oct 7, 2025
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Oct 7, 2025
@robintown robintown added this pull request to the merge queue Oct 7, 2025
Merged via the queue into develop with commit b84a73c Oct 7, 2025
32 checks passed
@robintown robintown deleted the hs/sticky-events branch October 7, 2025 17:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants