From 947d9127d86ce0496d879f390662d33a4de0bebd Mon Sep 17 00:00:00 2001 From: Olivier 'reivilibre Date: Mon, 18 Aug 2025 18:33:00 +0100 Subject: [PATCH 1/4] Move the MSC4306 push rules to a new kind `postcontent` --- rust/src/push/base_rules.rs | 14 +++++++++----- rust/src/push/mod.rs | 1 + synapse/push/clientformat.py | 2 +- synapse/push/rulekinds.py | 4 ++++ 4 files changed, 15 insertions(+), 6 deletions(-) diff --git a/rust/src/push/base_rules.rs b/rust/src/push/base_rules.rs index ec027ca251e..47d5289006b 100644 --- a/rust/src/push/base_rules.rs +++ b/rust/src/push/base_rules.rs @@ -289,10 +289,10 @@ pub const BASE_APPEND_CONTENT_RULES: &[PushRule] = &[PushRule { default_enabled: true, }]; -pub const BASE_APPEND_UNDERRIDE_RULES: &[PushRule] = &[ +pub const BASE_APPEND_POSTCONTENT_RULES: &[PushRule] = &[ PushRule { - rule_id: Cow::Borrowed("global/content/.io.element.msc4306.rule.unsubscribed_thread"), - priority_class: 1, + rule_id: Cow::Borrowed("global/postcontent/.io.element.msc4306.rule.unsubscribed_thread"), + priority_class: 6, conditions: Cow::Borrowed(&[Condition::Known( KnownCondition::Msc4306ThreadSubscription { subscribed: false }, )]), @@ -301,8 +301,8 @@ pub const BASE_APPEND_UNDERRIDE_RULES: &[PushRule] = &[ default_enabled: true, }, PushRule { - rule_id: Cow::Borrowed("global/content/.io.element.msc4306.rule.subscribed_thread"), - priority_class: 1, + rule_id: Cow::Borrowed("global/postcontent/.io.element.msc4306.rule.subscribed_thread"), + priority_class: 6, conditions: Cow::Borrowed(&[Condition::Known( KnownCondition::Msc4306ThreadSubscription { subscribed: true }, )]), @@ -310,6 +310,9 @@ pub const BASE_APPEND_UNDERRIDE_RULES: &[PushRule] = &[ default: true, default_enabled: true, }, +]; + +pub const BASE_APPEND_UNDERRIDE_RULES: &[PushRule] = &[ PushRule { rule_id: Cow::Borrowed("global/underride/.m.rule.call"), priority_class: 1, @@ -726,6 +729,7 @@ lazy_static! { .iter() .chain(BASE_APPEND_OVERRIDE_RULES.iter()) .chain(BASE_APPEND_CONTENT_RULES.iter()) + .chain(BASE_APPEND_POSTCONTENT_RULES.iter()) .chain(BASE_APPEND_UNDERRIDE_RULES.iter()) .map(|rule| { (&*rule.rule_id, rule) }) .collect(); diff --git a/rust/src/push/mod.rs b/rust/src/push/mod.rs index b07a12e5ccd..b0cedd758cd 100644 --- a/rust/src/push/mod.rs +++ b/rust/src/push/mod.rs @@ -527,6 +527,7 @@ impl PushRules { .chain(base_rules::BASE_APPEND_OVERRIDE_RULES.iter()) .chain(self.content.iter()) .chain(base_rules::BASE_APPEND_CONTENT_RULES.iter()) + .chain(base_rules::BASE_APPEND_POSTCONTENT_RULES.iter()) .chain(self.room.iter()) .chain(self.sender.iter()) .chain(self.underride.iter()) diff --git a/synapse/push/clientformat.py b/synapse/push/clientformat.py index b4afcfd85b9..4f647491f1a 100644 --- a/synapse/push/clientformat.py +++ b/synapse/push/clientformat.py @@ -91,7 +91,7 @@ def _rule_to_template(rule: PushRule) -> Optional[Dict[str, Any]]: unscoped_rule_id = _rule_id_from_namespaced(rule.rule_id) template_name = _priority_class_to_template_name(rule.priority_class) - if template_name in ["override", "underride"]: + if template_name in ["override", "underride", "postcontent"]: templaterule = {"conditions": rule.conditions, "actions": rule.actions} elif template_name in ["sender", "room"]: templaterule = {"actions": rule.actions} diff --git a/synapse/push/rulekinds.py b/synapse/push/rulekinds.py index 781ecc7faec..2eff626f927 100644 --- a/synapse/push/rulekinds.py +++ b/synapse/push/rulekinds.py @@ -19,10 +19,14 @@ # # +# Integer literals for push rule `kind`s +# This is used to store them in the database. PRIORITY_CLASS_MAP = { "underride": 1, "sender": 2, "room": 3, + # MSC4306 + "postcontent": 6, "content": 4, "override": 5, } From c7c398eef8910d388880e898ec1e8192aae1ae29 Mon Sep 17 00:00:00 2001 From: Olivier 'reivilibre Date: Tue, 19 Aug 2025 11:43:06 +0100 Subject: [PATCH 2/4] Prevent users from creating user-defined `postcontent` rules --- synapse/rest/client/push_rule.py | 11 +++++++++++ tests/rest/client/test_push_rule_attrs.py | 22 ++++++++++++++++++++++ 2 files changed, 33 insertions(+) diff --git a/synapse/rest/client/push_rule.py b/synapse/rest/client/push_rule.py index af042504c9b..c20de89bf71 100644 --- a/synapse/rest/client/push_rule.py +++ b/synapse/rest/client/push_rule.py @@ -19,9 +19,11 @@ # # +from http import HTTPStatus from typing import TYPE_CHECKING, List, Tuple, Union from synapse.api.errors import ( + Codes, NotFoundError, StoreError, SynapseError, @@ -239,6 +241,15 @@ def _rule_spec_from_path(path: List[str]) -> RuleSpec: def _rule_tuple_from_request_object( rule_template: str, rule_id: str, req_obj: JsonDict ) -> Tuple[List[JsonDict], List[Union[str, JsonDict]]]: + if rule_template == "postcontent": + # postcontent is from MSC4306, which says that clients + # cannot create their own postcontent rules right now. + raise SynapseError( + HTTPStatus.BAD_REQUEST, + "user-defined rules using `postcontent` are not accepted", + errcode=Codes.INVALID_PARAM, + ) + if rule_template in ["override", "underride"]: if "conditions" not in req_obj: raise InvalidRuleException("Missing 'conditions'") diff --git a/tests/rest/client/test_push_rule_attrs.py b/tests/rest/client/test_push_rule_attrs.py index 9da0e7982f8..53c36b7a9c6 100644 --- a/tests/rest/client/test_push_rule_attrs.py +++ b/tests/rest/client/test_push_rule_attrs.py @@ -18,6 +18,8 @@ # [This file includes modifications made by New Vector Limited] # # +from http import HTTPStatus + import synapse from synapse.api.errors import Codes from synapse.rest.client import login, push_rule, room @@ -486,3 +488,23 @@ def test_is_user_mention(self) -> None: }, channel.json_body, ) + + def test_no_user_defined_postcontent_rules(self) -> None: + """ + Tests that clients are not permitted to create MSC4306 `postcontent` rules. + """ + self.register_user("bob", "pass") + token = self.login("bob", "pass") + + channel = self.make_request( + "PUT", + "/pushrules/global/postcontent/some.user.rule", + {}, + access_token=token, + ) + + self.assertEqual(channel.code, HTTPStatus.BAD_REQUEST) + self.assertEqual( + Codes.INVALID_PARAM, + channel.json_body["errcode"], + ) From 3de8c2146d804bb286d79a52a3e5f41aab760fbd Mon Sep 17 00:00:00 2001 From: Olivier 'reivilibre Date: Tue, 19 Aug 2025 11:44:59 +0100 Subject: [PATCH 3/4] Newsfile Signed-off-by: Olivier 'reivilibre --- changelog.d/18846.feature | 1 + 1 file changed, 1 insertion(+) create mode 100644 changelog.d/18846.feature diff --git a/changelog.d/18846.feature b/changelog.d/18846.feature new file mode 100644 index 00000000000..4a873d44468 --- /dev/null +++ b/changelog.d/18846.feature @@ -0,0 +1 @@ +Update push rules for experimental [MSC4306: Thread Subscriptions](https://github.com/matrix-org/matrix-doc/issues/4306) to follow newer draft. \ No newline at end of file From 2ca3ac468ce00fa26ffcddaf6ddf43b60aab1262 Mon Sep 17 00:00:00 2001 From: Olivier 'reivilibre Date: Thu, 4 Sep 2025 08:19:35 +0100 Subject: [PATCH 4/4] Add tests that justify the `postcontent` section --- tests/push/test_bulk_push_rule_evaluator.py | 133 +++++++++++++++++++- 1 file changed, 132 insertions(+), 1 deletion(-) diff --git a/tests/push/test_bulk_push_rule_evaluator.py b/tests/push/test_bulk_push_rule_evaluator.py index fad5c7affb2..7342a72dff1 100644 --- a/tests/push/test_bulk_push_rule_evaluator.py +++ b/tests/push/test_bulk_push_rule_evaluator.py @@ -19,6 +19,7 @@ # # +from http import HTTPStatus from typing import Any, Optional from unittest.mock import AsyncMock, patch @@ -30,7 +31,7 @@ from synapse.api.room_versions import RoomVersions from synapse.push.bulk_push_rule_evaluator import BulkPushRuleEvaluator from synapse.rest import admin -from synapse.rest.client import login, register, room +from synapse.rest.client import login, push_rule, register, room from synapse.server import HomeServer from synapse.types import JsonDict, create_requester from synapse.util import Clock @@ -44,6 +45,7 @@ class TestBulkPushRuleEvaluator(HomeserverTestCase): room.register_servlets, login.register_servlets, register.register_servlets, + push_rule.register_servlets, ] def prepare( @@ -494,6 +496,135 @@ def test_thread_subscriptions(self) -> None: ) ) + @override_config({"experimental_features": {"msc4306_enabled": True}}) + def test_thread_subscriptions_suppression_after_keyword_mention_overrides( + self, + ) -> None: + """ + Tests one of the purposes of the `postcontent` push rule section: + When a keyword mention is configured (in the `content` section), + it does not get suppressed by the thread being unsubscribed. + """ + # add a keyword mention to alice's push rules + channel = self.make_request( + "PUT", + "/_matrix/client/v3/pushrules/global/content/biscuits", + {"pattern": "biscuits", "actions": ["notify"]}, + access_token=self.token, + ) + self.assertEqual(channel.code, HTTPStatus.OK) + + bulk_evaluator = BulkPushRuleEvaluator(self.hs) + (thread_root_id,) = self.helper.send_messages(self.room_id, 1, tok=self.token) + + self.assertFalse( + self._create_and_process( + bulk_evaluator, + { + "msgtype": "m.text", + "body": "do you want some cookies?", + "m.relates_to": { + "rel_type": RelationTypes.THREAD, + "event_id": thread_root_id, + }, + }, + type="m.room.message", + ), + "alice is not subscribed to thread and does not have a mention on 'cookies' so should not be notified", + ) + + self.assertTrue( + self._create_and_process( + bulk_evaluator, + { + "msgtype": "m.text", + "body": "biscuits are available in the kitchen", + "m.relates_to": { + "rel_type": RelationTypes.THREAD, + "event_id": thread_root_id, + }, + }, + type="m.room.message", + ), + "alice is not subscribed to thread but DOES have a mention on 'biscuits' so should be notified", + ) + + @override_config({"experimental_features": {"msc4306_enabled": True}}) + def test_thread_subscriptions_notification_before_keywords_and_mentions( + self, + ) -> None: + """ + Tests one of the purposes of the `postcontent` push rule section: + When a room is set to (what is commonly known as) 'keywords & mentions', we still receive notifications + for messages in threads that we are subscribed to. + Effectively making this 'keywords, mentions & subscriptions' + """ + # add a 'keywords & mentions' setting to the room alice's push rules + # In case this rule isn't clear: by adding a rule in the `room` section that does nothing, + # it stops execution of the push rules before we fall through to the `underride` section, + # where intuitively many kinds of messages will ambiently generate notifications. + # Mentions and keywords are triggered before the `room` block, so this doesn't suppress those. + channel = self.make_request( + "PUT", + f"/_matrix/client/v3/pushrules/global/room/{self.room_id}", + {"actions": []}, + access_token=self.token, + ) + self.assertEqual(channel.code, HTTPStatus.OK) + + bulk_evaluator = BulkPushRuleEvaluator(self.hs) + (thread_root_id,) = self.helper.send_messages(self.room_id, 1, tok=self.token) + + # sanity check that our mentions still work + self.assertFalse( + self._create_and_process( + bulk_evaluator, + { + "msgtype": "m.text", + "body": "this is a plain message with no mention", + }, + type="m.room.message", + ), + "alice should not be notified (mentions & keywords room setting)", + ) + self.assertTrue( + self._create_and_process( + bulk_evaluator, + { + "msgtype": "m.text", + "body": "this is a message that mentions alice", + }, + type="m.room.message", + ), + "alice should be notified (mentioned)", + ) + + # let's have alice subscribe to the thread + self.get_success( + self.hs.get_datastores().main.subscribe_user_to_thread( + self.alice, + self.room_id, + thread_root_id, + automatic_event_orderings=None, + ) + ) + + self.assertTrue( + self._create_and_process( + bulk_evaluator, + { + "msgtype": "m.text", + "body": "some message in the thread", + "m.relates_to": { + "rel_type": RelationTypes.THREAD, + "event_id": thread_root_id, + }, + }, + type="m.room.message", + ), + "alice is subscribed to thread so should be notified", + ) + def test_with_disabled_thread_subscriptions(self) -> None: """ Test what happens with threaded events when MSC4306 is disabled.