Fix state_after for left rooms including post-leave lazy-loaded memberships

changelog.d/20169.bugfix

@@ -0,0 +1 @@
+Fix `/sync` returning membership events from after the user's leave in `state_after` for left rooms when lazy-loading room members (experimental [MSC4222](https://github.com/matrix-org/matrix-spec-proposals/pull/4222) implementation).

synapse/handlers/sync.py

@@ -1249,6 +1249,7 @@ async def compute_state_delta(
                     end_token,
                     members_to_fetch,
                     timeline_state,
+                    joined,
                 )
 
             # If we only have partial state for the room, `state_ids` may be missing the
@@ -1471,6 +1472,7 @@ async def _compute_state_delta_for_incremental_sync(
         end_token: StreamToken,
         members_to_fetch: set[str] | None,
         timeline_state: StateMap[str],
+        joined: bool,
     ) -> StateMap[str]:
         """Calculate the state events to be included in an incremental sync response.
 
@@ -1495,6 +1497,7 @@ async def _compute_state_delta_for_incremental_sync(
                 events in the timeline. Otherwise, `None`.
             timeline_state: The contribution to the room state from state events in
                 `batch`. Only contains the last event for any given state key.
+            joined: whether the user is currently joined to the room
 
         Returns:
             A map from (type, state_key) to event_id, for each event that we believe
@@ -1520,13 +1523,28 @@ async def _compute_state_delta_for_incremental_sync(
                 # events to understand the events in this timeline. So we always
                 # fish out all the member events corresponding to the timeline
                 # here. The caller will then dedupe any redundant ones.
-                member_ids = await self._state_storage_controller.get_current_state_ids(
-                    room_id=room_id,
-                    state_filter=StateFilter.from_types(
-                        (EventTypes.Member, member) for member in members_to_fetch
-                    ),
-                    await_full_state=await_full_state,
+                member_filter = StateFilter.from_types(
+                    (EventTypes.Member, member) for member in members_to_fetch
                 )
+                if joined:
+                    member_ids = (
+                        await self._state_storage_controller.get_current_state_ids(
+                            room_id=room_id,
+                            state_filter=member_filter,
+                            await_full_state=await_full_state,
+                        )
+                    )
+                else:
+                    # The user is no longer in the room, so `end_token` points
+                    # at the user's leave/etc event, and the current state may
+                    # include state from after that point. Use state groups to
+                    # get the memberships as of `end_token` instead.
+                    member_ids = await self._state_storage_controller.get_state_ids_at(
+                        room_id,
+                        stream_position=end_token,
+                        state_filter=member_filter,
+                        await_full_state=await_full_state,
+                    )
                 delta_state_ids.update(member_ids)
 
             # We don't do LL filtering for incremental syncs - see

tests/handlers/test_sync.py

@@ -2888,6 +2888,7 @@ def test_incremental_sync_multiple_deltas(self) -> None:
                 end_token=end_stream_token,
                 members_to_fetch=None,
                 timeline_state={},
+                joined=True,
             )
         )
         self.assertEqual(state[("m.test_event", "")], second_state["event_id"])
@@ -2919,6 +2920,7 @@ def test_incremental_sync_lazy_loaded_no_timeline(self) -> None:
                 end_token=end_stream_token,
                 members_to_fetch=set(),
                 timeline_state={},
+                joined=True,
             )
         )
 

tests/rest/client/test_sync.py

@@ -33,6 +33,7 @@
     ReceiptTypes,
     RelationTypes,
 )
+from synapse.rest.admin.experimental_features import ExperimentalFeature
 from synapse.rest.client import devices, knock, login, read_marker, receipts, room, sync
 from synapse.server import HomeServer
 from synapse.types import JsonDict
@@ -1272,3 +1273,117 @@ def test_incremental_sync(self) -> None:
         )
 
         self.assertEqual(200, channel.code, msg=channel.result["body"])
+
+
+class SyncStateAfterArchivedRoomTestCase(unittest.HomeserverTestCase):
+    """Tests MSC4222 `state_after` behaviour for rooms the syncing user has
+    left (i.e. rooms in the `leave` section of the sync response)."""
+
+    servlets = [
+        synapse.rest.admin.register_servlets,
+        room.register_servlets,
+        login.register_servlets,
+        sync.register_servlets,
+    ]
+
+    def prepare(self, reactor: MemoryReactor, clock: Clock, hs: HomeServer) -> None:
+        self.store = hs.get_datastores().main
+
+    def test_archived_room_state_after_not_newer_than_leave(self) -> None:
+        """`state_after` for a left room must be the state at the end of that
+        room's timeline, i.e. at the user's leave point — never state from
+        after the leave.
+
+        Scenario: with lazy-loading of members and `use_state_after` enabled,
+        Alice does an incremental sync covering the window in which Bob sent a
+        message and Alice then left. Bob changed his per-room displayname
+        *after* Alice's leave; that post-leave membership event must NOT
+        appear in Alice's `state_after` for the left room.
+        """
+        alice = self.register_user("alice", "password")
+        alice_tok = self.login("alice", "password")
+        bob = self.register_user("bob", "password")
+        bob_tok = self.login("bob", "password")
+
+        # Opt Alice in to MSC4222.
+        self.get_success(
+            self.store.set_features_for_user(alice, {ExperimentalFeature.MSC4222: True})
+        )
+
+        # Name the room to avoid heroes: those come from the *current*
+        # summary — a separate leak path from the one under test.
+        room_id = self.helper.create_room_as(
+            alice, tok=alice_tok, extra_content={"name": "Some room name"}
+        )
+        self.helper.join(room_id, bob, tok=bob_tok)
+
+        # Bob's membership as it will stand at Alice's leave point.
+        channel = self.make_request(
+            "GET",
+            f"/_matrix/client/v3/rooms/{room_id}/state/m.room.member/{bob}?format=event",
+            access_token=alice_tok,
+        )
+        self.assertEqual(channel.code, 200, channel.result)
+        bob_member_event_id_at_leave = channel.json_body["event_id"]
+
+        # Lazy-load members; `include_redundant_members` bypasses the members
+        # cache so Bob's membership appears in the incremental sync below.
+        sync_filter = json.dumps(
+            {
+                "room": {
+                    "state": {
+                        "lazy_load_members": True,
+                        "include_redundant_members": True,
+                    },
+                }
+            }
+        )
+        sync_url = f"/sync?filter={sync_filter}&org.matrix.msc4222.use_state_after=true"
+
+        # Initial sync.
+        channel = self.make_request("GET", sync_url, access_token=alice_tok)
+        self.assertEqual(channel.code, 200, channel.result)
+        since = channel.json_body["next_batch"]
+
+        # Bob becomes a timeline sender in the next sync window.
+        self.helper.send(room_id, body="hello", tok=bob_tok)
+
+        # Alice leaves the room.
+        self.helper.leave(room_id, alice, tok=alice_tok)
+
+        # Bob's membership changes AFTER Alice's leave.
+        post_leave_member_event = self.helper.send_state(
+            room_id,
+            EventTypes.Member,
+            {"membership": "join", "displayname": "bob-post-leave"},
+            tok=bob_tok,
+            state_key=bob,
+        )
+        post_leave_member_event_id = post_leave_member_event["event_id"]
+
+        # Incremental sync: the room is in the `leave` section.
+        channel = self.make_request(
+            "GET", f"{sync_url}&since={since}", access_token=alice_tok
+        )
+        self.assertEqual(channel.code, 200, channel.result)
+
+        left_room = channel.json_body["rooms"]["leave"][room_id]
+        state_after_events = left_room["org.matrix.msc4222.state_after"]["events"]
+
+        # Post-leave state must not appear in `state_after`.
+        self.assertNotIn(
+            post_leave_member_event_id,
+            [e["event_id"] for e in state_after_events],
+            f"state_after contains state from after the user's leave: "
+            f"{state_after_events}",
+        )
+
+        # Bob's membership must be the one at the leave point.
+        self.assertEqual(
+            [
+                e["event_id"]
+                for e in state_after_events
+                if e["type"] == EventTypes.Member and e["state_key"] == bob
+            ],
+            [bob_member_event_id_at_leave],
+        )