From c3af44339cdfa862515558729083b1c37f4194ea Mon Sep 17 00:00:00 2001 From: Eric Eastwood Date: Tue, 3 Mar 2026 08:13:59 -0600 Subject: [PATCH] Fix `/sync` missing membership in `state_after` (re-introduce) (#19460) *This PR was originally only to enable [MSC4222](https://github.com/matrix-org/matrix-spec-proposals/pull/4222) Complement tests (`/sync` `state_after`) but after merging the [fix PR](https://github.com/element-hq/synapse/pull/19463), we discovered that while the tests pass locally, [fail in CI](https://github.com/element-hq/synapse/pull/19460#discussion_r2818080879). To unblock the RC, we decided to revert the fix PR (see https://github.com/element-hq/synapse/pull/19474#discussion_r2818061001 for more info). To better ensure tests actually pass in CI, we're re-introducing the fix here in the same PR that we enable the tests in.* --- Fix `/sync` missing membership in `state_after`. This applies to any scenario where the first membership has a different `sender` compared to the `state_key` and then the second membership has the same `sender`/`state_key`. Like someone inviting another person and then them joining. Or someone being kicked and then they leave. This bug has been present since the MSC4222 implementation was introduced into the codebase (https://github.com/element-hq/synapse/pull/17888). --- Fix https://github.com/element-hq/synapse/issues/19455 Fix https://github.com/element-hq/customer-success/issues/656 I have a feeling, this might also fix these issues (will close and see how people report back): Fix https://github.com/element-hq/synapse/issues/18182 Fix https://github.com/element-hq/synapse/issues/19478 ### Testing strategy Complement tests: https://github.com/matrix-org/complement/pull/842 We will need https://github.com/element-hq/synapse/pull/19460 to merge in order to enable the Complement tests in Synapse but this PR should be merged first so they pass in the first place. I've tested locally that the Complement tests pass with this fix. ### Dev notes [MSC4222](https://github.com/matrix-org/matrix-spec-proposals/pull/4222) has already been merged into the spec and is already part of Matrix v1.16 but we haven't [stabilized support in Synapse yet](https://github.com/element-hq/synapse/issues/19414). --- In the same ballpark: - https://github.com/element-hq/synapse/issues/19455 - https://github.com/element-hq/synapse/issues/17050 - https://github.com/element-hq/synapse/issues/17430 - https://github.com/element-hq/synapse/issues/16940 - https://github.com/element-hq/synapse/issues/18182 - https://github.com/element-hq/synapse/issues/18793 - https://github.com/element-hq/synapse/issues/19478 --- Docker builds preferring remote image over the local image we just built, https://github.com/element-hq/synapse/pull/19460#discussion_r2818080879 `containerd` image store (storage driver, driver type) -> https://github.com/element-hq/synapse/pull/19475 ### Todo - [x] Wait for https://github.com/element-hq/synapse/pull/19463 to merge so the Complement tests all pass - [x] Wait for https://github.com/element-hq/synapse/pull/19475 to merge ### Pull Request Checklist * [x] Pull request is based on the develop branch * [x] Pull request includes a [changelog file](https://element-hq.github.io/synapse/latest/development/contributing_guide.html#changelog). The entry should: - Be a short description of your change which makes sense to users. "Fixed a bug that prevented receiving messages from other servers." instead of "Moved X method from `EventStore` to `EventWorkerStore`.". - Use markdown where necessary, mostly for `code blocks`. - End with either a period (.) or an exclamation mark (!). - Start with a capital letter. - Feel free to credit yourself, by adding a sentence "Contributed by @github_username." or "Contributed by [Your Name]." to the end of the entry. * [x] [Code style](https://element-hq.github.io/synapse/latest/code_style.html) is correct (run the [linters](https://element-hq.github.io/synapse/latest/development/contributing_guide.html#run-the-linters)) --------- Co-authored-by: Andrew Morgan <1342360+anoadragon453@users.noreply.github.com> Co-authored-by: Andrew Ferrazzutti --- changelog.d/19460.bugfix | 1 + .../complement/conf/workers-shared-extra.yaml.j2 | 2 ++ scripts-dev/complement.sh | 1 + synapse/handlers/sync.py | 15 ++++++++++++--- 4 files changed, 16 insertions(+), 3 deletions(-) create mode 100644 changelog.d/19460.bugfix diff --git a/changelog.d/19460.bugfix b/changelog.d/19460.bugfix new file mode 100644 index 0000000000..fe44c2ddf6 --- /dev/null +++ b/changelog.d/19460.bugfix @@ -0,0 +1 @@ +Fix `/sync` missing membership event in `state_after` (experimental [MSC4222](https://github.com/matrix-org/matrix-spec-proposals/pull/4222) implementation) in some scenarios. diff --git a/docker/complement/conf/workers-shared-extra.yaml.j2 b/docker/complement/conf/workers-shared-extra.yaml.j2 index 120b3b9496..9fd7fc954a 100644 --- a/docker/complement/conf/workers-shared-extra.yaml.j2 +++ b/docker/complement/conf/workers-shared-extra.yaml.j2 @@ -141,6 +141,8 @@ experimental_features: msc4306_enabled: true # Sticky Events msc4354_enabled: true + # `/sync` `state_after` + msc4222_enabled: true server_notices: system_mxid_localpart: _server diff --git a/scripts-dev/complement.sh b/scripts-dev/complement.sh index 9a18a621ee..fec005fdb1 100755 --- a/scripts-dev/complement.sh +++ b/scripts-dev/complement.sh @@ -264,6 +264,7 @@ main() { ./tests/msc4140 ./tests/msc4155 ./tests/msc4306 + ./tests/msc4222 ) # Export the list of test packages as a space-separated environment variable, so other diff --git a/synapse/handlers/sync.py b/synapse/handlers/sync.py index 2f405004de..a32748c2a9 100644 --- a/synapse/handlers/sync.py +++ b/synapse/handlers/sync.py @@ -1046,9 +1046,18 @@ class SyncHandler: if event.sender not in first_event_by_sender_map: first_event_by_sender_map[event.sender] = event - # We need the event's sender, unless their membership was in a - # previous timeline event. - if (EventTypes.Member, event.sender) not in timeline_state: + # When using `state_after`, there is no special treatment with + # regards to state also being in the `timeline`. Always fetch + # relevant membership regardless of whether the state event is in + # the `timeline`. + if sync_config.use_state_after: + members_to_fetch.add(event.sender) + # For `state`, the client is supposed to do a flawed re-construction + # of state over time by starting with the given `state` and layering + # on state from the `timeline` as you go (flawed because state + # resolution). In this case, we only need their membership in + # `state` when their membership isn't already in the `timeline`. + elif (EventTypes.Member, event.sender) not in timeline_state: members_to_fetch.add(event.sender) # FIXME: we also care about invite targets etc.