Fix /sync missing membership in state_after (re-introduce) (#19460)

*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 <1342360+anoadragon453@users.noreply.github.com>
Co-authored-by: Andrew Ferrazzutti <andrewf@element.io>

Author: 3 people

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.

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

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

synapse/handlers/sync.py

@@ -1046,9 +1046,18 @@ async def compute_state_delta(
                     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.