From 3c46a10955b1af7e039e1d853fea5ec944257cd4 Mon Sep 17 00:00:00 2001 From: Paul Chobert Date: Thu, 8 Oct 2026 15:14:23 +0200 Subject: [PATCH] Stabilise MSC4222: support the stable `use_state_after` identifiers on `/sync` (#20170) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Part of #19414 (Support Matrix 1.16). ⚠️ Before merging, wait for the following PRs to be merged first: - https://github.com/element-hq/synapse/pull/20081 - https://github.com/element-hq/synapse/pull/20169 - https://github.com/element-hq/synapse/pull/20171 --- Since Matrix 1.16, the [spec](https://spec.matrix.org/v1.19/client-server-api/#get_matrixclientv3sync) defines the stable identifiers for [MSC4222](https://github.com/matrix-org/matrix-spec-proposals/pull/4222): the `use_state_after` query parameter on `GET /_matrix/client/v3/sync`, and the `state_after` room response field: > **`use_state_after`** — `boolean` — Controls whether to receive state changes between the previous sync and the start of the timeline, or between the previous sync and the end of the timeline. If this is set to `true`, servers MUST respond with the state between the previous sync and the end of the timeline in `state_after` and MUST omit `state`. […] > > **`state_after`** — […] Required if `use_state_after` was set to true in the request, even if it is empty. Added in `v1.16` > > — [Matrix Spec](https://spec.matrix.org/v1.19/client-server-api/#get_matrixclientv3sync) Synapse so far only implemented the unstable identifiers (`org.matrix.msc4222.use_state_after` / `org.matrix.msc4222.state_after`), gated behind the `msc4222_enabled` experimental flag and the per-user experimental features admin API. This PR: - accepts the stable `use_state_after` query parameter, unconditionally, and responds with the stable `state_after` field. The unstable query parameter is kept for a transition period; clients opting in with it get the unstable response field name back, so unstable-era clients see an unchanged wire format. If both parameters are set, the stable one wins. - removes the `msc4222_enabled` experimental flag and the `msc4222` per-user feature from the experimental features admin API, since the behaviour is no longer gated. ### 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: Eric Eastwood --- changelog.d/20170.feature | 1 + .../conf/workers-shared-extra.yaml.j2 | 2 - docs/admin_api/experimental_features.md | 1 - rust/src/config/mod.rs | 1 - rust/src/storage/store.rs | 2 - synapse/config/experimental.py | 3 - synapse/handlers/sync.py | 8 ++ synapse/rest/admin/experimental_features.py | 3 - synapse/rest/client/sync.py | 20 ++-- tests/rest/client/test_sync.py | 99 +++++++++++++------ 10 files changed, 94 insertions(+), 46 deletions(-) create mode 100644 changelog.d/20170.feature diff --git a/changelog.d/20170.feature b/changelog.d/20170.feature new file mode 100644 index 0000000000..aa271eb45f --- /dev/null +++ b/changelog.d/20170.feature @@ -0,0 +1 @@ +Stabilise [MSC4222](https://github.com/matrix-org/matrix-spec-proposals/pull/4222): support the stable `use_state_after` query parameter and `state_after` response field on `/sync`, as per Matrix v1.16, and remove the `msc4222_enabled` experimental config flag. diff --git a/docker/complement/conf/workers-shared-extra.yaml.j2 b/docker/complement/conf/workers-shared-extra.yaml.j2 index 251018e1d2..3cf8615b75 100644 --- a/docker/complement/conf/workers-shared-extra.yaml.j2 +++ b/docker/complement/conf/workers-shared-extra.yaml.j2 @@ -145,8 +145,6 @@ 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/docs/admin_api/experimental_features.md b/docs/admin_api/experimental_features.md index e32728e56d..ef1b58c9ba 100644 --- a/docs/admin_api/experimental_features.md +++ b/docs/admin_api/experimental_features.md @@ -5,7 +5,6 @@ basis. The currently supported features are: - [MSC3881](https://github.com/matrix-org/matrix-spec-proposals/pull/3881): enable remotely toggling push notifications for another client - [MSC3575](https://github.com/matrix-org/matrix-spec-proposals/pull/3575): enable experimental sliding sync support -- [MSC4222](https://github.com/matrix-org/matrix-spec-proposals/pull/4222): adding `state_after` to sync v2 To use it, you will need to authenticate by providing an `access_token` for a server admin: see [Admin API](../usage/administration/admin_api/). diff --git a/rust/src/config/mod.rs b/rust/src/config/mod.rs index aba29046db..c9f4ffc635 100644 --- a/rust/src/config/mod.rs +++ b/rust/src/config/mod.rs @@ -71,7 +71,6 @@ pub struct ExperimentalConfig { pub msc4306_enabled: bool, pub msc4169_enabled: bool, pub msc4354_enabled: bool, - pub msc4222_enabled: bool, pub msc4491_enabled: bool, pub msc4143_enabled: bool, pub msc4446_enabled: bool, diff --git a/rust/src/storage/store.rs b/rust/src/storage/store.rs index b339d7748c..c099d6f501 100644 --- a/rust/src/storage/store.rs +++ b/rust/src/storage/store.rs @@ -27,8 +27,6 @@ pub enum PerUserExperimentalFeature { MSC3881, #[serde(rename = "msc3575")] MSC3575, - #[serde(rename = "msc4222")] - MSC4222, } impl std::fmt::Display for PerUserExperimentalFeature { diff --git a/synapse/config/experimental.py b/synapse/config/experimental.py index ca67f1bf56..2e3782392e 100644 --- a/synapse/config/experimental.py +++ b/synapse/config/experimental.py @@ -260,9 +260,6 @@ class ExperimentalConfig(Config): # TODO: Remove the flag and the legacy rules after 2026-12-01. self.msc4210_enabled: bool = experimental.get("msc4210_enabled", True) - # MSC4222: Adding `state_after` to sync v2 - self.msc4222_enabled: bool = experimental.get("msc4222_enabled", False) - # MSC4076: Add `disable_badge_count`` to pusher configuration self.msc4076_enabled: bool = experimental.get("msc4076_enabled", False) diff --git a/synapse/handlers/sync.py b/synapse/handlers/sync.py index ef1da33a8c..ccd860cc60 100644 --- a/synapse/handlers/sync.py +++ b/synapse/handlers/sync.py @@ -232,6 +232,14 @@ class SyncConfig: is_guest: bool device_id: str | None use_state_after: bool + # Whether the client opted in to `state_after` via the unstable + # `org.matrix.msc4222.use_state_after` query parameter rather than the + # stable one, in which case the unstable response field name is used too. + # Only relevant when `use_state_after` is True. + # + # FIXME(unstable_state_after): Remove support for the unstable identifiers after 2027-09-01 + # (to allow some time for the ecosystem to adapt to the stable identifiers) + use_unstable_state_after_name: bool = False @attr.s(slots=True, frozen=True, auto_attribs=True) diff --git a/synapse/rest/admin/experimental_features.py b/synapse/rest/admin/experimental_features.py index c91c5b6a49..ae0e60ad3e 100644 --- a/synapse/rest/admin/experimental_features.py +++ b/synapse/rest/admin/experimental_features.py @@ -43,15 +43,12 @@ class ExperimentalFeature(str, Enum): MSC3881 = "msc3881" MSC3575 = "msc3575" - MSC4222 = "msc4222" def is_globally_enabled(self, config: "HomeServerConfig") -> bool: if self is ExperimentalFeature.MSC3881: return config.experimental.msc3881_enabled if self is ExperimentalFeature.MSC3575: return config.experimental.msc3575_enabled - if self is ExperimentalFeature.MSC4222: - return config.experimental.msc4222_enabled assert_never(self) diff --git a/synapse/rest/client/sync.py b/synapse/rest/client/sync.py index 83aa00d0b5..bf4b2c660a 100644 --- a/synapse/rest/client/sync.py +++ b/synapse/rest/client/sync.py @@ -175,11 +175,12 @@ class SyncRestServlet(RestServlet): filter_id = parse_string(request, "filter") full_state = parse_boolean(request, "full_state", default=False) - use_state_after = False - if await self.store.is_feature_enabled( - user.to_string(), ExperimentalFeature.MSC4222 - ): - use_state_after = parse_boolean( + use_state_after = parse_boolean(request, "use_state_after", default=False) + use_unstable_state_after_name = False + # FIXME(unstable_state_after): Remove support for the unstable identifiers after 2027-09-01 + # (to allow some time for the ecosystem to adapt to the stable identifiers) + if not use_state_after: + use_state_after = use_unstable_state_after_name = parse_boolean( request, "org.matrix.msc4222.use_state_after", default=False ) @@ -216,6 +217,7 @@ class SyncRestServlet(RestServlet): device_id, last_ignore_accdata_streampos, use_state_after, + use_unstable_state_after_name, ) if filter_id is None: @@ -253,6 +255,7 @@ class SyncRestServlet(RestServlet): is_guest=requester.is_guest, device_id=device_id, use_state_after=use_state_after, + use_unstable_state_after_name=use_unstable_state_after_name, ) since_token = None @@ -634,7 +637,12 @@ class SyncRestServlet(RestServlet): # We either include a `state` or `state_after` field depending on # whether the client has opted in to the newer `state_after` behavior. if sync_config.use_state_after: - state_key_name = "org.matrix.msc4222.state_after" + # Clients which opted in via the unstable MSC4222 query parameter + # get the unstable field name back, for the transition period. + if sync_config.use_unstable_state_after_name: + state_key_name = "org.matrix.msc4222.state_after" + else: + state_key_name = "state_after" else: state_key_name = "state" diff --git a/tests/rest/client/test_sync.py b/tests/rest/client/test_sync.py index 20fdacdc01..fe6a1739e9 100644 --- a/tests/rest/client/test_sync.py +++ b/tests/rest/client/test_sync.py @@ -34,7 +34,6 @@ from synapse.api.constants import ( RelationTypes, ) from synapse.events import EventBase -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, RoomStreamToken, StreamKeyType, StreamToken @@ -775,6 +774,70 @@ class SyncCacheTestCase(unittest.HomeserverTestCase): self.assertEqual(channel.code, 200, channel.json_body) +# FIXME(unstable_state_after): Remove this whole test case after 2027-09-01 +# and we drop support for the unstable variant of `state_after` +class SyncStateAfterTestCase(unittest.HomeserverTestCase): + """ + Tests for the `use_state_after` opt-in on `/sync` (MSC4222, stable as of + Matrix v1.16): the variant of the `state_after` field in the response should + match the stable/unstable query parameter the client opted in with. + """ + + servlets = [ + synapse.rest.admin.register_servlets, + login.register_servlets, + room.register_servlets, + sync.register_servlets, + ] + + def prepare(self, reactor: MemoryReactor, clock: Clock, hs: HomeServer) -> None: + self.user_id = self.register_user("user", "password") + self.access_token = self.login(self.user_id, "password") + self.room_id = self.helper.create_room_as(self.user_id, tok=self.access_token) + + def _sync_room_keys(self, query_string: str) -> set[str]: + """Perform a sync with the given query string and return the keys of + our room's response object.""" + channel = self.make_request( + "GET", + f"/sync{query_string}", + access_token=self.access_token, + ) + self.assertEqual(channel.code, 200, channel.json_body) + return set(channel.json_body["rooms"]["join"][self.room_id].keys()) + + def test_state_without_opt_in(self) -> None: + """By default, room state comes down under the `state` key.""" + room_keys = self._sync_room_keys("") + self.assertIn("state", room_keys) + self.assertNotIn("state_after", room_keys) + self.assertNotIn("org.matrix.msc4222.state_after", room_keys) + + def test_state_after_stable_name(self) -> None: + """Opting in with the stable query parameter gives the stable field name.""" + room_keys = self._sync_room_keys("?use_state_after=true") + self.assertIn("state_after", room_keys) + self.assertNotIn("state", room_keys) + self.assertNotIn("org.matrix.msc4222.state_after", room_keys) + + def test_state_after_unstable_name(self) -> None: + """Opting in with the unstable query parameter gives the unstable field + name, for the transition period.""" + room_keys = self._sync_room_keys("?org.matrix.msc4222.use_state_after=true") + self.assertIn("org.matrix.msc4222.state_after", room_keys) + self.assertNotIn("state", room_keys) + self.assertNotIn("state_after", room_keys) + + def test_state_after_both_names(self) -> None: + """If a client opts in with both query parameters, the stable field name wins.""" + room_keys = self._sync_room_keys( + "?use_state_after=true&org.matrix.msc4222.use_state_after=true" + ) + self.assertIn("state_after", room_keys) + self.assertNotIn("state", room_keys) + self.assertNotIn("org.matrix.msc4222.state_after", room_keys) + + class DeviceListSyncTestCase(unittest.HomeserverTestCase): """ Tests regarding device list (`device_lists`) changes. @@ -1322,12 +1385,6 @@ class SyncStateAfterTimelineStateTestCase(unittest.HomeserverTestCase): self.bob = self.register_user("bob", "password") self.bob_tok = self.login("bob", "password") - self.get_success( - self.store.set_features_for_user( - self.alice, {ExperimentalFeature.MSC4222: True} - ) - ) - # Named room, so that hero calculation doesn't inject current # membership state into the response and confuse the assertions. self.room_id = self.helper.create_room_as( @@ -1346,7 +1403,7 @@ class SyncStateAfterTimelineStateTestCase(unittest.HomeserverTestCase): sync_filter: JsonDict = {"room": {"timeline": {"limit": timeline_limit}}} if lazy_load_members: sync_filter["room"]["state"] = {"lazy_load_members": True} - return f"/sync?filter={json.dumps(sync_filter)}&org.matrix.msc4222.use_state_after=true" + return f"/sync?filter={json.dumps(sync_filter)}&use_state_after=true" def _sync(self, sync_url: str, since: str | None = None) -> JsonDict: url = sync_url if since is None else f"{sync_url}&since={since}" @@ -1363,7 +1420,7 @@ class SyncStateAfterTimelineStateTestCase(unittest.HomeserverTestCase): return [e["event_id"] for e in room["timeline"]["events"]] def _state_after_ids(self, room: JsonDict) -> list[str]: - return [e["event_id"] for e in room["org.matrix.msc4222.state_after"]["events"]] + return [e["event_id"] for e in room["state_after"]["events"]] def _persist_batch(self) -> tuple[EventBase, EventBase]: return self.get_success( @@ -1384,7 +1441,7 @@ class SyncStateAfterTimelineStateTestCase(unittest.HomeserverTestCase): self.store.get_partial_current_state_ids(self.room_id) ) current_ids = set(current_state.values()) - for event in room["org.matrix.msc4222.state_after"]["events"]: + for event in room["state_after"]["events"]: self.assertIn( event["event_id"], current_ids, @@ -1583,9 +1640,6 @@ class SyncStateAfterArchivedRoomTestCase(unittest.HomeserverTestCase): 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 @@ -1602,11 +1656,6 @@ class SyncStateAfterArchivedRoomTestCase(unittest.HomeserverTestCase): 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( @@ -1635,7 +1684,7 @@ class SyncStateAfterArchivedRoomTestCase(unittest.HomeserverTestCase): } } ) - sync_url = f"/sync?filter={sync_filter}&org.matrix.msc4222.use_state_after=true" + sync_url = f"/sync?filter={sync_filter}&use_state_after=true" # Initial sync. channel = self.make_request("GET", sync_url, access_token=alice_tok) @@ -1665,7 +1714,7 @@ class SyncStateAfterArchivedRoomTestCase(unittest.HomeserverTestCase): 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"] + state_after_events = left_room["state_after"]["events"] # Post-leave state must not appear in `state_after`. self.assertNotIn( @@ -1708,12 +1757,6 @@ class SyncLazyLoadedMembersCacheTestCase(unittest.HomeserverTestCase): self.bob = self.register_user("bob", "password") self.bob_tok = self.login("bob", "password") - self.get_success( - hs.get_datastores().main.set_features_for_user( - self.alice, {ExperimentalFeature.MSC4222: True} - ) - ) - # Named room, so that hero calculation doesn't inject current # membership state into the response and confuse the assertions. self.room_id = self.helper.create_room_as( @@ -1748,7 +1791,7 @@ class SyncLazyLoadedMembersCacheTestCase(unittest.HomeserverTestCase): } url = f"/sync?filter={json.dumps(sync_filter)}&timeout={timeout_ms}" if use_state_after: - url += "&org.matrix.msc4222.use_state_after=true" + url += "&use_state_after=true" if since is not None: url += f"&since={since}" return self.make_request( @@ -1786,7 +1829,7 @@ class SyncLazyLoadedMembersCacheTestCase(unittest.HomeserverTestCase): self, response: JsonDict, use_state_after: bool ) -> list[JsonDict]: """Extract the list of state events from the test room in a sync response.""" - key = "org.matrix.msc4222.state_after" if use_state_after else "state" + key = "state_after" if use_state_after else "state" return self._room(response)[key]["events"] def _state_ids(self, response: JsonDict, use_state_after: bool) -> list[str]: