Remove the legacy mention push rules by default (#20220)

Part of #19415 (Support Matrix 1.17).

Companion test PRs:
- Complement: https://github.com/matrix-org/complement/pull/920
- Sytest: https://github.com/matrix-org/sytest/pull/1430

**What this does:** `experimental_features.msc4210_enabled` default
value changed `true`
([MSC4210](https://github.com/matrix-org/matrix-spec-proposals/pull/4210)).
The rest of the changes is fixing the tests, making sure test relying en
mentions sends the proper `m.mentions` attribute.

## Migration plan

Element Web has been writing both rule sets together since September
2023
([matrix-react-sdk#11667](https://github.com/matrix-org/matrix-react-sdk/pull/11667),
[element-web@0d367a7c7e](https://github.com/element-hq/element-web/commit/0d367a7c7e)),
so for its users `.m.rule.is_user_mention` and `.m.rule.is_room_mention`
already carry the same customisations as the legacy rules.

<strike>For everyone else, #20215 adds a background update that migrates
the overrides on the legacy rules to the intentional ones.</strike>

## Opt-out

```yaml
experimental_features:
  msc4210_enabled: false
```

---

### 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))

changelog.d/20220.removal

@@ -0,0 +1 @@
+Remove the legacy mention push rules by default, as per [MSC4210](https://github.com/matrix-org/matrix-spec-proposals/pull/4210). The `experimental_features.msc4210_enabled` flag now defaults to `true`.

synapse/config/experimental.py

@@ -257,7 +257,8 @@ class ExperimentalConfig(Config):
         self.msc4169_enabled: bool = experimental.get("msc4169_enabled", False)
 
         # MSC4210: Remove legacy mentions
-        self.msc4210_enabled: bool = experimental.get("msc4210_enabled", False)
+        # 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)

tests/push/test_bulk_push_rule_evaluator.py

@@ -149,6 +149,7 @@ class TestBulkPushRuleEvaluator(HomeserverTestCase):
                     "content": {
                         "msgtype": "m.text",
                         "body": "helo @room",
+                        EventContentFields.MENTIONS: {"room": True},
                     },
                     "sender": self.alice,
                 },
@@ -241,6 +242,44 @@ class TestBulkPushRuleEvaluator(HomeserverTestCase):
         )
         return len(result) > 0
 
+    def test_legacy_mention_rules_removed(self) -> None:
+        """
+        Matrix v1.17 (MSC4210) removed the legacy mention rules from the base rule
+        set: an event which merely contains the user's name in its body, without
+        an `m.mentions` property, must no longer notify.
+        """
+        bulk_evaluator = BulkPushRuleEvaluator(self.hs)
+
+        # Note that the events are not `m.room.message`s, so that the generic
+        # underride rules do not fire and only the mention rules can notify.
+        self.assertFalse(
+            self._create_and_process(bulk_evaluator, {"body": "hello alice"}),
+            "alice should not be notified (legacy mention rules are removed)",
+        )
+        self.assertTrue(
+            self._create_and_process(
+                bulk_evaluator,
+                {
+                    "body": "hello alice",
+                    EventContentFields.MENTIONS: {"user_ids": [self.alice]},
+                },
+            ),
+            "alice should be notified (intentional mention)",
+        )
+
+    @override_config({"experimental_features": {"msc4210_enabled": False}})
+    def test_legacy_mention_rules_opt_out(self) -> None:
+        """
+        The legacy mention rules can temporarily be restored, in which case an
+        event which contains the user's name in its body notifies again.
+        """
+        bulk_evaluator = BulkPushRuleEvaluator(self.hs)
+
+        self.assertTrue(
+            self._create_and_process(bulk_evaluator, {"body": "hello alice"}),
+            "alice should be notified (legacy mention)",
+        )
+
     def test_user_mentions(self) -> None:
         """Test the behavior of an event which includes invalid user mentions."""
         bulk_evaluator = BulkPushRuleEvaluator(self.hs)
@@ -593,6 +632,7 @@ class TestBulkPushRuleEvaluator(HomeserverTestCase):
                 {
                     "msgtype": "m.text",
                     "body": "this is a message that mentions alice",
+                    EventContentFields.MENTIONS: {"user_ids": [self.alice]},
                 },
                 type="m.room.message",
             ),

tests/push/test_http.py

@@ -25,6 +25,7 @@ from parameterized import parameterized
 from twisted.internet.defer import Deferred
 from twisted.internet.testing import MemoryReactor
 
+from synapse.api.constants import EventContentFields
 from synapse.logging.context import make_deferred_yieldable
 from synapse.push import PusherConfig, PusherConfigException
 from synapse.rest import admin
@@ -422,10 +423,10 @@ class HTTPPusherTests(HomeserverTestCase):
         # check that this is low-priority
         self.assertEqual(self.push_attempts[1][2]["notification"]["prio"], "low")
 
-    def test_sends_high_priority_for_mention(self) -> None:
+    def test_sends_high_priority_for_user_mention(self) -> None:
         """
         The HTTP pusher will send pushes at high priority if they correspond
-        to a message containing the user's display name.
+        to a message that intentionally mentions the user via `m.mentions`.
         """
         # Register the user who gets notified
         user_id = self.register_user("user", "pass")
@@ -469,8 +470,18 @@ class HTTPPusherTests(HomeserverTestCase):
             )
         )
 
-        # Send a message
-        self.helper.send(room, body="Oh, user, hello!", tok=other_access_token)
+        # Send a message mentioning the user. As a client would, the body refers
+        # to the user as well, but it is `m.mentions` that makes it a mention.
+        self.helper.send_event(
+            room,
+            "m.room.message",
+            {
+                "msgtype": "m.text",
+                "body": "Oh, @user, hello!",
+                EventContentFields.MENTIONS: {"user_ids": [user_id]},
+            },
+            tok=other_access_token,
+        )
 
         # Advance time a bit, so the pusher will register something has happened
         self.pump()
@@ -547,10 +558,15 @@ class HTTPPusherTests(HomeserverTestCase):
             )
         )
 
-        # Send a message
-        self.helper.send(
+        # Send a message mentioning the room
+        self.helper.send_event(
             room,
-            body="@room eeek! There's a spider on the table!",
+            "m.room.message",
+            {
+                "msgtype": "m.text",
+                "body": "@room eeek! There's a spider on the table!",
+                EventContentFields.MENTIONS: {"room": True},
+            },
             tok=other_access_token,
         )
 
@@ -569,8 +585,15 @@ class HTTPPusherTests(HomeserverTestCase):
         self.assertEqual(self.push_attempts[0][2]["notification"]["prio"], "high")
 
         # Send another event, this time as someone without the power of @room
-        self.helper.send(
-            room, body="@room the spider is gone", tok=yet_another_access_token
+        self.helper.send_event(
+            room,
+            "m.room.message",
+            {
+                "msgtype": "m.text",
+                "body": "@room the spider is gone",
+                EventContentFields.MENTIONS: {"room": True},
+            },
+            tok=yet_another_access_token,
         )
 
         # Advance time a bit, so the pusher will register something has happened

tests/rest/client/test_push_rule_attrs.py

@@ -520,9 +520,39 @@ class PushRuleAttributesTestCase(HomeserverTestCase):
             token, "global/override/.im.nheko.msc3664.reply"
         )
 
-    def test_contains_user_name(self) -> None:
+    @parameterized.expand(
+        [
+            ("global/override/.m.rule.contains_display_name",),
+            ("global/content/.m.rule.contains_user_name",),
+            ("global/override/.m.rule.roomnotif",),
+        ]
+    )
+    def test_legacy_mention_rules_removed(self, rule_path: str) -> None:
         """
-        Tests that `contains_user_name` rule is present and have proper value in `pattern`.
+        Tests that the legacy mention rules, which Matrix v1.17 (MSC4210) removed
+        from the base rule set, are not served by default.
+        """
+        self.register_user("bob", "pass")
+        token = self.login("bob", "pass")
+
+        channel = self.make_request(
+            "GET", f"/pushrules/{rule_path}", access_token=token
+        )
+        self.assertEqual(channel.code, 404)
+        self.assertEqual(channel.json_body["errcode"], Codes.NOT_FOUND)
+
+        _, kind, rule_id = rule_path.split("/")
+        channel = self.make_request("GET", "/pushrules/", access_token=token)
+        self.assertEqual(channel.code, 200)
+        self.assertNotIn(
+            rule_id, [rule["rule_id"] for rule in channel.json_body["global"][kind]]
+        )
+
+    @override_config({"experimental_features": {"msc4210_enabled": False}})
+    def test_contains_user_name_with_legacy_mentions(self) -> None:
+        """
+        Tests that, when the legacy mention rules are restored, the
+        `contains_user_name` rule is present and has the proper value in `pattern`.
         """
         username = "bob"
         self.register_user(username, "pass")

tests/storage/test_event_push_actions.py

@@ -22,7 +22,7 @@
 
 from twisted.internet.testing import MemoryReactor
 
-from synapse.api.constants import MAIN_TIMELINE, RelationTypes
+from synapse.api.constants import MAIN_TIMELINE, EventContentFields, RelationTypes
 from synapse.rest import admin
 from synapse.rest.client import login, room
 from synapse.server import HomeServer
@@ -84,6 +84,7 @@ class EventPushActionsStoreTestCase(HomeserverTestCase):
             content={
                 "msgtype": "m.text",
                 "body": user_id,
+                EventContentFields.MENTIONS: {"user_ids": [user_id]},
                 "m.relates_to": {
                     "rel_type": RelationTypes.THREAD,
                     "event_id": first_event_id,
@@ -194,7 +195,13 @@ class EventPushActionsStoreTestCase(HomeserverTestCase):
             result = self.helper.send_event(
                 room_id,
                 type="m.room.message",
-                content={"msgtype": "m.text", "body": user_id if highlight else "msg"},
+                content={
+                    "msgtype": "m.text",
+                    "body": "msg",
+                    EventContentFields.MENTIONS: {"user_ids": [user_id]}
+                    if highlight
+                    else {},
+                },
                 tok=other_token,
             )
             nonlocal last_event_id
@@ -388,7 +395,13 @@ class EventPushActionsStoreTestCase(HomeserverTestCase):
             return self.helper.send_event(
                 room_id,
                 type="m.room.message",
-                content={"msgtype": "m.text", "body": user_id if highlight else "msg"},
+                content={
+                    "msgtype": "m.text",
+                    "body": "msg",
+                    EventContentFields.MENTIONS: {"user_ids": [user_id]}
+                    if highlight
+                    else {},
+                },
                 tok=other_token,
             )["event_id"]
 
@@ -435,7 +448,10 @@ class EventPushActionsStoreTestCase(HomeserverTestCase):
         def _send(thread_root: str | None = None, highlight: bool = False) -> str:
             content: JsonDict = {
                 "msgtype": "m.text",
-                "body": user_id if highlight else "msg",
+                "body": "msg",
+                EventContentFields.MENTIONS: {"user_ids": [user_id]}
+                if highlight
+                else {},
             }
             if thread_root is not None:
                 content["m.relates_to"] = {
@@ -609,7 +625,10 @@ class EventPushActionsStoreTestCase(HomeserverTestCase):
         def _create_event(highlight: bool = False, thread_id: str | None = None) -> str:
             content: JsonDict = {
                 "msgtype": "m.text",
-                "body": user_id if highlight else "msg",
+                "body": "msg",
+                EventContentFields.MENTIONS: {"user_ids": [user_id]}
+                if highlight
+                else {},
             }
             if thread_id:
                 content["m.relates_to"] = {
@@ -789,7 +808,10 @@ class EventPushActionsStoreTestCase(HomeserverTestCase):
         def _create_event(highlight: bool = False, thread_id: str | None = None) -> str:
             content: JsonDict = {
                 "msgtype": "m.text",
-                "body": user_id if highlight else "msg",
+                "body": "msg",
+                EventContentFields.MENTIONS: {"user_ids": [user_id]}
+                if highlight
+                else {},
             }
             if thread_id:
                 content["m.relates_to"] = {