From 0b910449efe21ebf5f5cf393f01719c6f95a7493 Mon Sep 17 00:00:00 2001 From: Benjamin Bouvier Date: Fri, 24 Apr 2026 12:18:05 +0200 Subject: [PATCH 1/9] Send a SSS response immediately if the config has changed and there are new results to sync (#19714) This fixes the bug described in #19713 (and double-checked against the SDK integration test, which now passes with this change). A sync response must be returned immediately if a room subscription configuration change caused a new non-empty response (checked with `if response` in the code) to be produced. Fixes #19713. Fixes #18844. --------- Co-authored-by: Erik Johnston --- changelog.d/19714.bugfix | 1 + synapse/handlers/sliding_sync/__init__.py | 52 ++++---- synapse/handlers/sliding_sync/room_lists.py | 12 +- .../sliding_sync/test_room_subscriptions.py | 119 ++++++++++++++++++ 4 files changed, 156 insertions(+), 28 deletions(-) create mode 100644 changelog.d/19714.bugfix diff --git a/changelog.d/19714.bugfix b/changelog.d/19714.bugfix new file mode 100644 index 00000000000..6aba7b21a61 --- /dev/null +++ b/changelog.d/19714.bugfix @@ -0,0 +1 @@ +Have SSS return a new response immediately if a room subscription have changed and produced a new response. diff --git a/synapse/handlers/sliding_sync/__init__.py b/synapse/handlers/sliding_sync/__init__.py index df1270f7f97..98b74b098d9 100644 --- a/synapse/handlers/sliding_sync/__init__.py +++ b/synapse/handlers/sliding_sync/__init__.py @@ -184,34 +184,38 @@ async def wait_for_sync_for_user( timeout_ms -= after_wait_ts - before_wait_ts timeout_ms = max(timeout_ms, 0) - # We're going to respond immediately if the timeout is 0 or if this is an - # initial sync (without a `from_token`) so we can avoid calling - # `notifier.wait_for_events()`. - if timeout_ms == 0 or from_token is None: - now_token = self.event_sources.get_current_token() - result = await self.current_sync_for_user( + # Compute a response immediately. We always need to do this before + # waiting for new data (unlike in /v3/sync), as the request config might + # have changed (e.g. new room subscriptions, etc). + now_token = self.event_sources.get_current_token() + result = await self.current_sync_for_user( + sync_config, + from_token=from_token, + to_token=now_token, + ) + + # Return immediately if we have a result, the timeout is 0, or this is + # an initial sync. + if result or timeout_ms == 0 or from_token is None: + return result, did_wait + + # Otherwise, we wait for something to happen and report it to the user. + async def current_sync_callback( + before_token: StreamToken, after_token: StreamToken + ) -> SlidingSyncResult: + return await self.current_sync_for_user( sync_config, from_token=from_token, - to_token=now_token, + to_token=after_token, ) - else: - # Otherwise, we wait for something to happen and report it to the user. - async def current_sync_callback( - before_token: StreamToken, after_token: StreamToken - ) -> SlidingSyncResult: - return await self.current_sync_for_user( - sync_config, - from_token=from_token, - to_token=after_token, - ) - result = await self.notifier.wait_for_events( - sync_config.user.to_string(), - timeout_ms, - current_sync_callback, - from_token=from_token.stream_token, - ) - did_wait = True + result = await self.notifier.wait_for_events( + sync_config.user.to_string(), + timeout_ms, + current_sync_callback, + from_token=now_token, + ) + did_wait = True return result, did_wait diff --git a/synapse/handlers/sliding_sync/room_lists.py b/synapse/handlers/sliding_sync/room_lists.py index 8969d915836..216ef3b0710 100644 --- a/synapse/handlers/sliding_sync/room_lists.py +++ b/synapse/handlers/sliding_sync/room_lists.py @@ -852,11 +852,15 @@ async def _filter_relevant_rooms_to_send( previous_connection_state.room_configs.get(room_id) ) if prev_room_sync_config is not None: - # Always include rooms whose timeline limit has increased. - # (see the "XXX: Odd behavior" described below) + # Always include rooms whose effective config has + # expanded. This covers timeline-limit increases and + # required-state additions introduced by room + # subscriptions overriding list-derived params. if ( - prev_room_sync_config.timeline_limit - < room_config.timeline_limit + prev_room_sync_config.combine_room_sync_config( + room_config + ) + != prev_room_sync_config ): rooms_should_send.add(room_id) continue diff --git a/tests/rest/client/sliding_sync/test_room_subscriptions.py b/tests/rest/client/sliding_sync/test_room_subscriptions.py index 811478f1ba7..d970af367d3 100644 --- a/tests/rest/client/sliding_sync/test_room_subscriptions.py +++ b/tests/rest/client/sliding_sync/test_room_subscriptions.py @@ -22,6 +22,7 @@ from synapse.api.constants import EventTypes, HistoryVisibility from synapse.rest.client import login, room, sync from synapse.server import HomeServer +from synapse.types import JsonDict from synapse.util.clock import Clock from tests.rest.client.sliding_sync.test_sliding_sync import SlidingSyncBase @@ -126,6 +127,124 @@ def test_room_subscriptions_with_join_membership(self) -> None: response_body["rooms"][room_id1], ) + def test_room_subscription_required_state_expansion_returns_immediately( + self, + ) -> None: + """ + Test that adding a room subscription with stronger params than the list causes an + incremental long-poll to return immediately, even without new stream activity. + """ + user1_id = self.register_user("user1", "pass") + user1_tok = self.login(user1_id, "pass") + + room_id1 = self.helper.create_room_as(user1_id, tok=user1_tok) + + sync_body: JsonDict = { + "lists": { + "foo-list": { + "ranges": [[0, 0]], + "required_state": [], + "timeline_limit": 0, + } + }, + "conn_id": "conn_id", + } + _, from_token = self.do_sync(sync_body, tok=user1_tok) + + sync_body["room_subscriptions"] = { + room_id1: { + "required_state": [ + [EventTypes.Create, ""], + ], + "timeline_limit": 0, + } + } + + channel = self.make_request( + "POST", + self.sync_endpoint + f"?timeout=10000&pos={from_token}", + content=sync_body, + access_token=user1_tok, + await_result=False, + ) + channel.await_result(timeout_ms=3000) + self.assertEqual(channel.code, 200, channel.json_body) + + state_map = self.get_success( + self.storage_controllers.state.get_current_state(room_id1) + ) + + room_response = channel.json_body["rooms"][room_id1] + self.assertNotIn("initial", room_response) + self._assertRequiredStateIncludes( + room_response["required_state"], + { + state_map[(EventTypes.Create, "")], + }, + exact=True, + ) + + def test_room_subscription_required_state_change_returns_immediately(self) -> None: + """ + Test that expanding an existing room subscription's required state causes an + incremental long-poll to return immediately, even without new stream activity. + """ + user1_id = self.register_user("user1", "pass") + user1_tok = self.login(user1_id, "pass") + + room_id1 = self.helper.create_room_as( + user1_id, tok=user1_tok, extra_content={"name": "Foo"} + ) + + sync_body: JsonDict = { + "room_subscriptions": { + room_id1: { + "required_state": [ + [EventTypes.Create, ""], + ], + "timeline_limit": 0, + } + }, + "conn_id": "conn_id", + } + response_body, from_token = self.do_sync(sync_body, tok=user1_tok) + + state_map = self.get_success( + self.storage_controllers.state.get_current_state(room_id1) + ) + self._assertRequiredStateIncludes( + response_body["rooms"][room_id1]["required_state"], + { + state_map[(EventTypes.Create, "")], + }, + exact=True, + ) + + sync_body["room_subscriptions"][room_id1]["required_state"] = [ + [EventTypes.Create, ""], + [EventTypes.Name, ""], + ] + + channel = self.make_request( + "POST", + self.sync_endpoint + f"?timeout=10000&pos={from_token}", + content=sync_body, + access_token=user1_tok, + await_result=False, + ) + channel.await_result(timeout_ms=3000) + self.assertEqual(channel.code, 200, channel.json_body) + + room_response = channel.json_body["rooms"][room_id1] + self.assertNotIn("initial", room_response) + self._assertRequiredStateIncludes( + room_response["required_state"], + { + state_map[(EventTypes.Name, "")], + }, + exact=True, + ) + def test_room_subscriptions_with_leave_membership(self) -> None: """ Test `room_subscriptions` with a leave room should give us timeline and state From a5e6dd23da57f3d430695388cb81dc045c5bd3ec Mon Sep 17 00:00:00 2001 From: Erik Johnston Date: Wed, 20 May 2026 14:09:37 +0100 Subject: [PATCH 2/9] Ensure e2ee extension doesn't always return --- synapse/types/handlers/sliding_sync.py | 11 +-- .../sliding_sync/test_extension_e2ee.py | 90 +++++++++++++++++++ 2 files changed, 94 insertions(+), 7 deletions(-) diff --git a/synapse/types/handlers/sliding_sync.py b/synapse/types/handlers/sliding_sync.py index 1a84bf1ff8f..c3b3a25cfdd 100644 --- a/synapse/types/handlers/sliding_sync.py +++ b/synapse/types/handlers/sliding_sync.py @@ -301,15 +301,12 @@ def __bool__(self) -> bool: # Also related: # https://github.com/element-hq/element-android/issues/3725 and # https://github.com/matrix-org/synapse/issues/10456 - default_otk = self.device_one_time_keys_count.get("signed_curve25519") - more_than_default_otk = len(self.device_one_time_keys_count) > 1 or ( - default_otk is not None and default_otk > 0 - ) + # + # This is why we don't incorporate `device_one_time_keys_count` into the + # `__bool__` check. return bool( - more_than_default_otk - or self.device_list_updates - or self.device_unused_fallback_key_types + self.device_list_updates or self.device_unused_fallback_key_types ) @attr.s(slots=True, frozen=True, auto_attribs=True) diff --git a/tests/rest/client/sliding_sync/test_extension_e2ee.py b/tests/rest/client/sliding_sync/test_extension_e2ee.py index 4a5e4070382..126b63797fc 100644 --- a/tests/rest/client/sliding_sync/test_extension_e2ee.py +++ b/tests/rest/client/sliding_sync/test_extension_e2ee.py @@ -290,6 +290,96 @@ def test_wait_for_new_data_timeout(self) -> None: [], ) + def test_wait_for_new_data_timeout_with_otks(self) -> None: + """ + Test that an incremental Sliding Sync with the e2ee extension enabled + does not return immediately when the user has uploaded one-time keys + (i.e. `device_one_time_keys_count` contains entries beyond the default + `signed_curve25519: 0`). + """ + test_device_id = "TESTDEVICE" + user1_id = self.register_user("user1", "pass") + user1_tok = self.login(user1_id, "pass", device_id=test_device_id) + + # Upload one-time keys for the user/device so that + # `device_one_time_keys_count` is non-default in the response. + self.get_success( + self.e2e_keys_handler.upload_keys_for_user( + user1_id, + test_device_id, + { + "one_time_keys": { + "alg1:k1": "key1", + "alg2:k2": {"key": "key2", "signatures": {"k1": "sig1"}}, + } + }, + ) + ) + + sync_body = { + "lists": {}, + "extensions": { + "e2ee": { + "enabled": True, + } + }, + } + _, from_token = self.do_sync(sync_body, tok=user1_tok) + + # Make an incremental Sliding Sync request with a timeout + channel = self.make_request( + "POST", + self.sync_endpoint + f"?timeout=10000&pos={from_token}", + content=sync_body, + access_token=user1_tok, + await_result=False, + ) + # Block for 5 seconds to make sure we are `notifier.wait_for_events(...)` + with self.assertRaises(TimedOutException): + channel.await_result(timeout_ms=5000) + + # Wake-up `notifier.wait_for_events(...)` that will cause us to test + # `SlidingSyncResult.__bool__` for new results. The non-default + # `device_one_time_keys_count` must not be considered new data. + self._bump_notifier_wait_for_events( + user1_id, wake_stream_key=StreamKeyType.ACCOUNT_DATA + ) + + # Block for a little bit more to ensure we don't see any new results. + with self.assertRaises(TimedOutException): + channel.await_result(timeout_ms=4000) + # Wait for the sync to complete (wait for the rest of the 10 second timeout, + # 5000 + 4000 + 1200 > 10000) + channel.await_result(timeout_ms=1200) + self.assertEqual(channel.code, 200, channel.json_body) + + # Device lists are present for incremental syncs but empty because no device changes + self.assertEqual( + channel.json_body["extensions"]["e2ee"] + .get("device_lists", {}) + .get("changed"), + [], + ) + self.assertEqual( + channel.json_body["extensions"]["e2ee"].get("device_lists", {}).get("left"), + [], + ) + + # The one-time key counts are present, but they should not have caused + # the sync to return early. + self.assertEqual( + channel.json_body["extensions"]["e2ee"]["device_one_time_keys_count"], + { + "alg1": 1, + "alg2": 1, + "signed_curve25519": 0, + }, + ) + self.assertEqual( + channel.json_body["extensions"]["e2ee"]["device_unused_fallback_key_types"], + [], + ) + def test_device_lists(self) -> None: """ Test that device list updates are included in the response From 4c21800317da60356568abd0fcf979e234a0abbc Mon Sep 17 00:00:00 2001 From: Erik Johnston Date: Wed, 20 May 2026 14:17:54 +0100 Subject: [PATCH 3/9] Comment on __bool__ --- synapse/types/handlers/sliding_sync.py | 28 +++++++++++++++++++++++--- 1 file changed, 25 insertions(+), 3 deletions(-) diff --git a/synapse/types/handlers/sliding_sync.py b/synapse/types/handlers/sliding_sync.py index c3b3a25cfdd..17afa5f2521 100644 --- a/synapse/types/handlers/sliding_sync.py +++ b/synapse/types/handlers/sliding_sync.py @@ -203,6 +203,9 @@ class StrippedHero: highlight_count: int def __bool__(self) -> bool: + """Are there any updates that should be returned immediately to + the client? + """ return ( # If this is the first time the client is seeing the room, we should not filter it out # under any circumstance. @@ -270,6 +273,8 @@ class ToDeviceExtension: events: Sequence[JsonMapping] def __bool__(self) -> bool: + """Are there any updates that should be returned immediately to + the client?""" return bool(self.events) @attr.s(slots=True, frozen=True, auto_attribs=True) @@ -294,6 +299,8 @@ class E2eeExtension: device_unused_fallback_key_types: Sequence[str] def __bool__(self) -> bool: + """Are there any updates that should be returned immediately to + the client?""" # Note that "signed_curve25519" is always returned in key count responses # regardless of whether we uploaded any keys for it. This is necessary until # https://github.com/matrix-org/matrix-doc/issues/3298 is fixed. @@ -324,6 +331,8 @@ class AccountDataExtension: account_data_by_room_map: Mapping[str, Mapping[str, JsonMapping]] def __bool__(self) -> bool: + """Are there any updates that should be returned immediately to + the client?""" return bool( self.global_account_data_map or self.account_data_by_room_map ) @@ -340,6 +349,8 @@ class ReceiptsExtension: room_id_to_receipt_map: Mapping[str, JsonMapping] def __bool__(self) -> bool: + """Are there any updates that should be returned immediately to + the client?""" return bool(self.room_id_to_receipt_map) @attr.s(slots=True, frozen=True, auto_attribs=True) @@ -354,6 +365,8 @@ class TypingExtension: room_id_to_typing_map: Mapping[str, JsonMapping] def __bool__(self) -> bool: + """Are there any updates that should be returned immediately to + the client?""" return bool(self.room_id_to_typing_map) @attr.s(slots=True, frozen=True, auto_attribs=True) @@ -388,6 +401,8 @@ class ThreadUnsubscription: prev_batch: ThreadSubscriptionsToken | None def __bool__(self) -> bool: + """Are there any updates that should be returned immediately to + the client?""" return ( bool(self.subscribed) or bool(self.unsubscribed) @@ -402,6 +417,8 @@ def __bool__(self) -> bool: thread_subscriptions: ThreadSubscriptionsExtension | None = None def __bool__(self) -> bool: + """Are there any updates that should be returned immediately to + the client?""" return bool( self.to_device or self.e2ee @@ -417,9 +434,14 @@ def __bool__(self) -> bool: extensions: Extensions def __bool__(self) -> bool: - """Make the result appear empty if there are no updates. This is used - to tell if the notifier needs to wait for more events when polling for - events. + """Are there any updates that should be returned immediately to + the client? + + This is used to determine if a sliding sync response should be returned + immediately or if the notifier needs to wait for further updates, and + thus MUST return false if there is no new data since the last sync. This + is subtly different than just checking if any of the fields are set, + since some fields are always included (like `bump_stamp`). """ # We don't include `self.lists` here, as a) `lists` is always non-empty even if # there are no changes, and b) since we're sorting rooms by `stream_ordering` of From 569d3d8dd6953ef3fd35c465256c42ae281800b8 Mon Sep 17 00:00:00 2001 From: Erik Johnston Date: Wed, 20 May 2026 14:22:16 +0100 Subject: [PATCH 4/9] Newsfile --- changelog.d/{19714.bugfix => 19792.bugfix} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename changelog.d/{19714.bugfix => 19792.bugfix} (100%) diff --git a/changelog.d/19714.bugfix b/changelog.d/19792.bugfix similarity index 100% rename from changelog.d/19714.bugfix rename to changelog.d/19792.bugfix From 8fbedbb34231b770bce7e49f20b7bcc6881f6edf Mon Sep 17 00:00:00 2001 From: Erik Johnston Date: Wed, 20 May 2026 14:28:14 +0100 Subject: [PATCH 5/9] Ignore device_unused_fallback_key_types as well --- synapse/types/handlers/sliding_sync.py | 7 +- .../sliding_sync/test_extension_e2ee.py | 77 +++++++++++++++++++ 2 files changed, 80 insertions(+), 4 deletions(-) diff --git a/synapse/types/handlers/sliding_sync.py b/synapse/types/handlers/sliding_sync.py index 17afa5f2521..3e969d019d4 100644 --- a/synapse/types/handlers/sliding_sync.py +++ b/synapse/types/handlers/sliding_sync.py @@ -309,12 +309,11 @@ def __bool__(self) -> bool: # https://github.com/element-hq/element-android/issues/3725 and # https://github.com/matrix-org/synapse/issues/10456 # - # This is why we don't incorporate `device_one_time_keys_count` into the + # This is why we don't incorporate `device_one_time_keys_count` + # (or `device_unused_fallback_key_types`) into the # `__bool__` check. - return bool( - self.device_list_updates or self.device_unused_fallback_key_types - ) + return bool(self.device_list_updates) @attr.s(slots=True, frozen=True, auto_attribs=True) class AccountDataExtension: diff --git a/tests/rest/client/sliding_sync/test_extension_e2ee.py b/tests/rest/client/sliding_sync/test_extension_e2ee.py index 126b63797fc..2707e4d98dc 100644 --- a/tests/rest/client/sliding_sync/test_extension_e2ee.py +++ b/tests/rest/client/sliding_sync/test_extension_e2ee.py @@ -380,6 +380,83 @@ def test_wait_for_new_data_timeout_with_otks(self) -> None: [], ) + def test_wait_for_new_data_timeout_with_fallback_keys(self) -> None: + """ + Test that an incremental Sliding Sync with the e2ee extension enabled + does not return immediately when the user has uploaded fallback keys + (i.e. `device_unused_fallback_key_types` is non-empty). + """ + test_device_id = "TESTDEVICE" + user1_id = self.register_user("user1", "pass") + user1_tok = self.login(user1_id, "pass", device_id=test_device_id) + + # Upload a fallback key for the user/device so that + # `device_unused_fallback_key_types` is non-empty in the response. + self.get_success( + self.e2e_keys_handler.upload_keys_for_user( + user1_id, + test_device_id, + {"fallback_keys": {"alg1:k1": "fallback_key1"}}, + ) + ) + + sync_body = { + "lists": {}, + "extensions": { + "e2ee": { + "enabled": True, + } + }, + } + _, from_token = self.do_sync(sync_body, tok=user1_tok) + + # Make an incremental Sliding Sync request with a timeout + channel = self.make_request( + "POST", + self.sync_endpoint + f"?timeout=10000&pos={from_token}", + content=sync_body, + access_token=user1_tok, + await_result=False, + ) + # Block for 5 seconds to make sure we are `notifier.wait_for_events(...)` + with self.assertRaises(TimedOutException): + channel.await_result(timeout_ms=5000) + + # Wake-up `notifier.wait_for_events(...)` that will cause us to test + # `SlidingSyncResult.__bool__` for new results. The non-empty + # `device_unused_fallback_key_types` must not be considered new data. + self._bump_notifier_wait_for_events( + user1_id, wake_stream_key=StreamKeyType.ACCOUNT_DATA + ) + + # Block for a little bit more to ensure we don't see any new results. + with self.assertRaises(TimedOutException): + channel.await_result(timeout_ms=4000) + + # Wait for the sync to complete (wait for the rest of the 10 second timeout, + # 5000 + 4000 + 1200 > 10000) + channel.await_result(timeout_ms=1200) + self.assertEqual(channel.code, 200, channel.json_body) + + # Device lists are present for incremental syncs but empty because no device changes + self.assertEqual( + channel.json_body["extensions"]["e2ee"] + .get("device_lists", {}) + .get("changed"), + [], + ) + self.assertEqual( + channel.json_body["extensions"]["e2ee"].get("device_lists", {}).get("left"), + [], + ) + + # The unused fallback key types are present, but they should not have + # caused the sync to return early. + self.assertEqual( + channel.json_body["extensions"]["e2ee"]["device_unused_fallback_key_types"], + ["alg1"], + ) + def test_device_lists(self) -> None: """ Test that device list updates are included in the response From cbc7f6b088f650a2da6480c9b8c5822409140332 Mon Sep 17 00:00:00 2001 From: Erik Johnston Date: Fri, 5 Jun 2026 11:25:29 +0100 Subject: [PATCH 6/9] Add clarification and test to fixed SSS behaviour (#19734) Follow on from #19714, where we should have had an extra comment and test. Co-authored-by: Eric Eastwood --- changelog.d/19734.bugfix | 1 + changelog.d/19792.bugfix | 2 +- synapse/handlers/sliding_sync/__init__.py | 7 ++ .../sliding_sync/test_rooms_required_state.py | 81 ++++++++++++++++++- .../client/sliding_sync/test_sliding_sync.py | 36 +++++++-- 5 files changed, 120 insertions(+), 7 deletions(-) create mode 100644 changelog.d/19734.bugfix diff --git a/changelog.d/19734.bugfix b/changelog.d/19734.bugfix new file mode 100644 index 00000000000..01af7d9ab83 --- /dev/null +++ b/changelog.d/19734.bugfix @@ -0,0 +1 @@ +Update Sliding Sync to return a new response immediately if a room subscription have changed and produced a new response. diff --git a/changelog.d/19792.bugfix b/changelog.d/19792.bugfix index 6aba7b21a61..01af7d9ab83 100644 --- a/changelog.d/19792.bugfix +++ b/changelog.d/19792.bugfix @@ -1 +1 @@ -Have SSS return a new response immediately if a room subscription have changed and produced a new response. +Update Sliding Sync to return a new response immediately if a room subscription have changed and produced a new response. diff --git a/synapse/handlers/sliding_sync/__init__.py b/synapse/handlers/sliding_sync/__init__.py index 98b74b098d9..d10fed2d794 100644 --- a/synapse/handlers/sliding_sync/__init__.py +++ b/synapse/handlers/sliding_sync/__init__.py @@ -213,6 +213,13 @@ async def current_sync_callback( sync_config.user.to_string(), timeout_ms, current_sync_callback, + # We *wait* from `now_token` as we have already computed the sync + # response up to `now_token` above, so as a minor optimization, we + # can wait for something new to arrive after `now_token`. + # + # We still generate the sync response using `from_token` in the + # callback above though, as to generate the correct response it + # needs to know the "real" `from_token`. from_token=now_token, ) did_wait = True diff --git a/tests/rest/client/sliding_sync/test_rooms_required_state.py b/tests/rest/client/sliding_sync/test_rooms_required_state.py index 586b127f8a1..901f22a35d1 100644 --- a/tests/rest/client/sliding_sync/test_rooms_required_state.py +++ b/tests/rest/client/sliding_sync/test_rooms_required_state.py @@ -25,8 +25,10 @@ from synapse.server import HomeServer from synapse.storage.databases.main.events import DeltaState, SlidingSyncTableChanges from synapse.util.clock import Clock +from synapse.util.duration import Duration from tests.rest.client.sliding_sync.test_sliding_sync import SlidingSyncBase +from tests.server import TimedOutException from tests.test_utils.event_injection import mark_event_as_partial_state logger = logging.getLogger(__name__) @@ -1924,7 +1926,12 @@ def test_rooms_required_state_expand(self) -> None: def test_rooms_required_state_expand_retract_expand(self) -> None: """Test that when expanding, retracting and then expanding the required - state, we get the changes that happened.""" + state, we get the changes that happened. + + Also see `test_changing_required_state_returns_immediately`, which tests + that the sync stream is woken up immediately when changing the required + state, and not just on the next change to the room. + """ user1_id = self.register_user("user1", "pass") user1_tok = self.login(user1_id, "pass") @@ -2245,3 +2252,75 @@ def test_lazy_loading_room_members_state_reset_non_limited_timeline(self) -> Non response_body["rooms"][room_id]["required_state"][0]["event_id"], first_event_id, ) + + def test_changing_required_state_returns_immediately(self) -> None: + """Test that if we change the `required_state`, then we return immediately + with the new `required_state`.""" + + user1_id = self.register_user("user1", "pass") + user1_tok = self.login(user1_id, "pass") + + room_id1 = self.helper.create_room_as(user1_id, tok=user1_tok) + + # Make an initial sync request with no required state + sync_body = { + "lists": { + "foo-list": { + "ranges": [[0, 1]], + "required_state": [], + "timeline_limit": 0, + } + } + } + response_body, from_token = self.do_sync(sync_body, tok=user1_tok) + + # We should see no required state + self.assertIsNone(response_body["rooms"][room_id1].get("required_state")) + + # Get the state_map before we change the state as this is the final state we + # expect to see when we update the required state. + state_map = self.get_success( + self.storage_controllers.state.get_current_state(room_id1) + ) + + # There is no new data, and so making another sync request will block. + channel = self.make_sync_request( + sync_body, + since=from_token, + tok=user1_tok, + timeout=Duration(seconds=10), + await_result=False, + ) + + # Request will block for 10 seconds as there no updates. + with self.assertRaises(TimedOutException): + channel.await_result(timeout_ms=9500) + + # Wait for the request to actually finish. (We do this to ensure log + # contexts don't leak between tests). + channel.await_result(timeout_ms=1000) + + # Now update the Sliding Sync requests to include a `required_state` + # event, and make another sync request. + sync_body["lists"]["foo-list"]["required_state"] = [ + [EventTypes.Create, ""], + ] + + channel = self.make_sync_request( + sync_body, + since=from_token, + tok=user1_tok, + timeout=Duration(seconds=10), + await_result=False, + ) + + # We should see the new `required_state` immediately without waiting + channel.await_result(timeout_ms=0) + response_body = channel.json_body + self._assertRequiredStateIncludes( + response_body["rooms"][room_id1]["required_state"], + { + state_map[(EventTypes.Create, "")], + }, + exact=True, + ) diff --git a/tests/rest/client/sliding_sync/test_sliding_sync.py b/tests/rest/client/sliding_sync/test_sliding_sync.py index ebf41cd87c3..fc7d6a279c3 100644 --- a/tests/rest/client/sliding_sync/test_sliding_sync.py +++ b/tests/rest/client/sliding_sync/test_sliding_sync.py @@ -12,6 +12,7 @@ # . # import logging +import urllib.parse from typing import Any, Iterable, Literal from unittest.mock import AsyncMock @@ -43,6 +44,7 @@ StreamToken, ) from synapse.util.clock import Clock +from synapse.util.duration import Duration from synapse.util.stringutils import random_string from tests import unittest @@ -82,7 +84,13 @@ def default_config(self) -> JsonDict: return config def make_sync_request( - self, sync_body: JsonDict, *, since: str | None = None, tok: str + self, + sync_body: JsonDict, + *, + since: str | None = None, + tok: str, + timeout: Duration | None = None, + await_result: bool = True, ) -> FakeChannel: """Make a sliding sync request with given body. @@ -90,25 +98,40 @@ def make_sync_request( sync_body: The full request body to use since: Optional since token tok: Access token to use - + timeout_ms: Optional timeout in milliseconds to use for the request. + await_result: Whether to block and wait for the result before returning. Returns: A tuple of the response body and the `pos` field. """ sync_path = self.sync_endpoint + + query_params: dict[str, Any] = {} if since: - sync_path += f"?pos={since}" + query_params["pos"] = since + if timeout is not None: + query_params["timeout"] = timeout.as_millis() + + if query_params: + query_str = urllib.parse.urlencode(query_params) + sync_path += f"?{query_str}" channel = self.make_request( method="POST", path=sync_path, content=sync_body, access_token=tok, + await_result=await_result, ) return channel def do_sync( - self, sync_body: JsonDict, *, since: str | None = None, tok: str + self, + sync_body: JsonDict, + *, + since: str | None = None, + tok: str, + timeout: Duration | None = None, ) -> tuple[JsonDict, str]: """Do a sliding sync request with given body. @@ -118,11 +141,14 @@ def do_sync( sync_body: The full request body to use since: Optional since token tok: Access token to use + timeout: Optional timeout to use for the request. Returns: A tuple of the response body and the `pos` field. """ - channel = self.make_sync_request(sync_body, since=since, tok=tok) + channel = self.make_sync_request( + sync_body, since=since, tok=tok, timeout=timeout + ) self.assertEqual(channel.code, 200, channel.json_body) return channel.json_body, channel.json_body["pos"] From 2edfbe27ca49899c7999c32ae7578ad5d0b9683d Mon Sep 17 00:00:00 2001 From: Erik Johnston Date: Fri, 5 Jun 2026 11:30:49 +0100 Subject: [PATCH 7/9] Note that this is not ideal --- synapse/types/handlers/sliding_sync.py | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/synapse/types/handlers/sliding_sync.py b/synapse/types/handlers/sliding_sync.py index 3e969d019d4..8f1b7da19ab 100644 --- a/synapse/types/handlers/sliding_sync.py +++ b/synapse/types/handlers/sliding_sync.py @@ -301,8 +301,9 @@ class E2eeExtension: def __bool__(self) -> bool: """Are there any updates that should be returned immediately to the client?""" - # Note that "signed_curve25519" is always returned in key count responses - # regardless of whether we uploaded any keys for it. This is necessary until + # Note that "signed_curve25519" is always returned in key count + # responses regardless of whether we uploaded any keys for it. + # This is necessary until # https://github.com/matrix-org/matrix-doc/issues/3298 is fixed. # # Also related: @@ -310,8 +311,14 @@ def __bool__(self) -> bool: # https://github.com/matrix-org/synapse/issues/10456 # # This is why we don't incorporate `device_one_time_keys_count` - # (or `device_unused_fallback_key_types`) into the - # `__bool__` check. + # (or `device_unused_fallback_key_types`) into the `__bool__` + # check. Ideally we'd detect if either of those fields have + # changed since the last sync, but we do not currently track + # such state. + # + # Note that the client will receive these fields eventually when + # we respond to the sync request, we just won't immediately + # respond (even if there are changes). return bool(self.device_list_updates) From 591f8c773660c1f773a82390098796db3dcb7b71 Mon Sep 17 00:00:00 2001 From: Erik Johnston Date: Fri, 5 Jun 2026 15:33:07 +0100 Subject: [PATCH 8/9] Fixup comment --- synapse/types/handlers/sliding_sync.py | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/synapse/types/handlers/sliding_sync.py b/synapse/types/handlers/sliding_sync.py index 8f1b7da19ab..86bf75bc50a 100644 --- a/synapse/types/handlers/sliding_sync.py +++ b/synapse/types/handlers/sliding_sync.py @@ -312,13 +312,19 @@ def __bool__(self) -> bool: # # This is why we don't incorporate `device_one_time_keys_count` # (or `device_unused_fallback_key_types`) into the `__bool__` - # check. Ideally we'd detect if either of those fields have + # check. + # + # FIXME: Ideally we'd detect if either of those fields have # changed since the last sync, but we do not currently track # such state. # # Note that the client will receive these fields eventually when - # we respond to the sync request, we just won't immediately - # respond (even if there are changes). + # we respond to the sync request (usually sync timeouts are set + # to ~30s), we just won't immediately respond (even if there are + # changes). This delay is acceptable for clients, as a) these + # fields do not trigger UI (and so don't affect user perceivable + # latency) and b) are handled in the background by the clients + # anyway. return bool(self.device_list_updates) From 2879ff3f6cec8bd23ba67f7649f8128b6637199f Mon Sep 17 00:00:00 2001 From: Erik Johnston Date: Mon, 8 Jun 2026 10:18:02 +0100 Subject: [PATCH 9/9] Update synapse/types/handlers/sliding_sync.py Co-authored-by: Eric Eastwood --- synapse/types/handlers/sliding_sync.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/synapse/types/handlers/sliding_sync.py b/synapse/types/handlers/sliding_sync.py index 86bf75bc50a..e73533d296d 100644 --- a/synapse/types/handlers/sliding_sync.py +++ b/synapse/types/handlers/sliding_sync.py @@ -324,7 +324,10 @@ def __bool__(self) -> bool: # changes). This delay is acceptable for clients, as a) these # fields do not trigger UI (and so don't affect user perceivable # latency) and b) are handled in the background by the clients - # anyway. + # anyway. The only risk being that one-time keys could be exhausted + # before the client knows about adding some more. But for example, + # if the client is syncing with a timeout of 30s, the window of + # staleness is so small for this not to matter. return bool(self.device_list_updates)