Conversation
This reverts commit b0b4203.
MadLittleMods
approved these changes
Feb 17, 2026
sandhose
deleted the
revert-19463-madlittlemods/19455-fix-state-after-missing-membership
branch
| @@ -1041,18 +1041,9 @@ async def compute_state_delta( | |||
| if event.sender not in first_event_by_sender_map: | |||
Contributor
There was a problem hiding this comment.
To better explain, "Why the revert?"
While #19463 still seems sane, the PR that enables the Complement tests is failing in CI, #19460 - These tests pass just fine locally and just require some time to figure out why CI isn't as happy as we are. Additionally, @dbkr even tried it out end-to-end with Element and confirmed it fixes the problem originally reported.
The RC process is underway and we don't want to block it trying to figure out the CI. Easier to revert and re-introduce again.
Additionally, the Complement tests weren't reviewed before #19463 was merged so even if the CI was passing there, merging would mean no new tests would be actually run.
In terms of what to do better next time:
- The Complement tests PR should have been reviewed/approved and ready to go before merging the fix PR
- Enabling the Complement tests in Synapse should happen in the same PR as the fix. Separating the fix from enabling the Complement tests proved to not be good enough. Our CI behaves differently than locally (for unknown reasons, edit: see Fix Complement CI not running against the code from our PRs (remote images being chosen over local) #19475) so we can't rely on the same results.
This was referenced Feb 17, 2026
sandhose
pushed a commit
that referenced
this pull request
Mar 3, 2026
*This PR was originally only to enable [MSC4222](matrix-org/matrix-spec-proposals#4222) Complement tests (`/sync` `state_after`) but after merging the [fix PR](#19463), we discovered that while the tests pass locally, [fail in CI](#19460 (comment)). To unblock the RC, we decided to revert the fix PR (see #19474 (comment) 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 (#17888). --- Fix #19455 Fix element-hq/customer-success#656 I have a feeling, this might also fix these issues (will close and see how people report back): Fix #18182 Fix #19478 ### Testing strategy Complement tests: matrix-org/complement#842 We will need #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](matrix-org/matrix-spec-proposals#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](#19414). --- In the same ballpark: - #19455 - #17050 - #17430 - #16940 - #18182 - #18793 - #19478 --- Docker builds preferring remote image over the local image we just built, #19460 (comment) `containerd` image store (storage driver, driver type) -> #19475 ### Todo - [x] Wait for #19463 to merge so the Complement tests all pass - [x] Wait for #19475 to merge ### Pull Request Checklist <!-- Please read https://element-hq.github.io/synapse/latest/development/contributing_guide.html before submitting your pull request --> * [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 <[email protected]> Co-authored-by: Andrew Ferrazzutti <[email protected]>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Reverts #19463
The complement tests haven't been reviewed and require more testing. Discussed in the internal backend team lobby room.