Push relevant users if there's a knock on a room (MSC4506) - #19955
Conversation
Adds the recipient_permission push rule condition (unstable kind org.matrix.msc4506.recipient_permission): matches iff the user the rules are being evaluated for has a power level >= that required to perform the power-levels action named by `key`. Uses it in a new default override rule .org.matrix.msc4506.rule.knock (inserted before .m.rule.member_event, which otherwise suppresses all member events) so that members who can invite -- i.e. accept the knock (MSC2403) -- are pushed when someone knocks. Also lifts the member-event fast-path in the bulk evaluator for knocks, which otherwise evaluates rules for nobody (a knock's sender == state_key). Gated behind experimental_features.msc4506_enabled. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Uk8aPxHn3BHCe52L226jdG
There was a problem hiding this comment.
Pull request overview
Implements experimental MSC4506 push notifications for room knocks.
Changes:
- Adds the
recipient_permissioncondition and knock default rule. - Integrates recipient power levels and feature gating into push evaluation.
- Adds Rust/Python tests, bindings, benchmarks, and changelog.
Reviewed changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
tests/push/test_push_rule_evaluator.py |
Tests the new condition. |
tests/push/test_bulk_push_rule_evaluator.py |
Tests knock notifications and gating. |
synapse/synapse_rust/push.pyi |
Updates Rust binding types. |
synapse/storage/databases/main/push_rule.py |
Passes the feature flag to rule filtering. |
synapse/push/bulk_push_rule_evaluator.py |
Computes recipient permissions and knock recipients. |
synapse/config/experimental.py |
Defines the experimental flag. |
rust/src/push/mod.rs |
Adds condition parsing and rule filtering. |
rust/src/push/evaluator.rs |
Evaluates recipient permissions. |
rust/src/push/base_rules.rs |
Defines the knock push rule. |
rust/benches/evaluator.rs |
Updates evaluator benchmarks. |
changelog.d/19955.feature |
Documents the feature. |
Suppressed comments (2)
synapse/push/bulk_push_rule_evaluator.py:565
- This lookup misses the room-version-specific creator power semantics. In v12 rooms, creators are deliberately absent from
content.users, whileget_user_power_levelgrants themCREATOR_POWER_LEVELfrom the create event (synapse/event_auth.py:1133-1145). They therefore receiveusers_defaulthere (normally 0) and are not notified wheninviteis 50 even though they can accept the knock. Calculate each recipient's level using the same create-event/auth-state logic as permission checks.
recipient_power_level = _coerce_power_level(
user_power_levels.get(uid, users_default_level), users_default_level
)
synapse/push/bulk_push_rule_evaluator.py:565
recipient_permissionremains active whenmsc4506_enabledis false: the Rust filter only removes the default MSC rule by ID, while a user-defined rule with this condition still receives a concrete power level and matches. PassNonewhile the feature is disabled so the condition is actually gated as described.
recipient_power_level = _coerce_power_level(
user_power_levels.get(uid, users_default_level), users_default_level
)
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| for key in list(action_power_levels.keys()): | ||
| level = action_power_levels[key] | ||
| if type(level) is not int: # noqa: E721 | ||
| try: | ||
| action_power_levels[key] = int(level) | ||
| except (TypeError, ValueError): | ||
| del action_power_levels[key] |
There was a problem hiding this comment.
Basically: if the power level is explicitly null (so None), then coerce it to 0, instead of ignoring it.
anoadragon453
left a comment
There was a problem hiding this comment.
Overall a useful addition, thank you!
I've done a rough review pass below.
| @@ -0,0 +1 @@ | |||
| Push relevant users if there's a knock on a room (MSC4506). | |||
There was a problem hiding this comment.
| Push relevant users if there's a knock on a room (MSC4506). | |
| Implement experimental support for [MSC4506](https://github.com/matrix-org/matrix-spec-proposals/pull/4506): notify relevant users if there's a knock on a room. |
| .match_condition(&condition, Some("@bob:example.org"), None, None, None) | ||
| .unwrap()); | ||
|
|
||
| // An action key we don't have a level for never matches. |
There was a problem hiding this comment.
It would be nice to have a test for the default behaviour of, say, invite not being defined in the m.room.power_levels state event.
Looks like the default behaviour is part of bulk_push_rule_evaluator.py.
| # own level, for the `recipient_permission` condition. As above, | ||
| # non-integer levels in old room versions are interpreted as integers | ||
| # where possible and otherwise dropped. | ||
| action_power_levels = { |
There was a problem hiding this comment.
| action_power_levels = { | |
| # Note: membership state events do not use `state_default` as a fallback. | |
| action_power_levels = { |
| for key in list(action_power_levels.keys()): | ||
| level = action_power_levels[key] | ||
| if type(level) is not int: # noqa: E721 | ||
| try: | ||
| action_power_levels[key] = int(level) | ||
| except (TypeError, ValueError): | ||
| del action_power_levels[key] |
There was a problem hiding this comment.
nit:
| for key in list(action_power_levels.keys()): | |
| level = action_power_levels[key] | |
| if type(level) is not int: # noqa: E721 | |
| try: | |
| action_power_levels[key] = int(level) | |
| except (TypeError, ValueError): | |
| del action_power_levels[key] | |
| for key, level in enumerate(action_power_levels): | |
| if type(level) is not int: # noqa: E721 | |
| try: | |
| action_power_levels[key] = int(level) | |
| except (TypeError, ValueError): | |
| del action_power_levels[key] |
| users_default_level = _coerce_power_level( | ||
| power_levels.get("users_default", 0), 0 | ||
| ) | ||
| user_power_levels = power_levels.get("users", {}) |
There was a problem hiding this comment.
This doesn't appear to take into account the fact that room creators have infinite power level in v12+ rooms, and are not included in m.room.power_levels->users.
There was a problem hiding this comment.
This feature will need an experimental tracking issue before being merged.
| for key in list(action_power_levels.keys()): | ||
| level = action_power_levels[key] | ||
| if type(level) is not int: # noqa: E721 | ||
| try: | ||
| action_power_levels[key] = int(level) | ||
| except (TypeError, ValueError): | ||
| del action_power_levels[key] |
There was a problem hiding this comment.
Basically: if the power level is explicitly null (so None), then coerce it to 0, instead of ignoring it.
Implement MSC4506
Adds the
recipient_permissionpush rule condition; matches if and only if the user the rules are being evaluated for has a power level >= that required to perform the power-levels action named bykey.Uses it in a new default override rule
.org.matrix.msc4506.rule.knock(inserted before.m.rule.member_event, which otherwise suppresses all member events) so that members who can invite (i.e. accept the knock) are pushed when someone knocks.Gated behind
experimental_features.msc4506_enabled.Pull Request Checklist
EventStoretoEventWorkerStore.".code blocks.