Show intentional mention push rules in notification settings - #35031
Merged
florianduros merged 11 commits intoSep 15, 2026
Merged
florianduros merged 11 commits into
florianduros merged 11 commits into
Conversation
Matrix v1.17 removed the legacy text-matching mention push rules (.m.rule.contains_display_name, .m.rule.contains_user_name, .m.rule.roomnotif) from the base rule set (MSC4210), and Synapse is about to stop serving them by default. Settings > Notifications built its "Mentions and keywords" rows from those rule IDs, so against such a server it lost both mention rows. The section now renders .m.rule.is_user_mention and .m.rule.is_room_mention, with the legacy rules as their synced rules: servers that still serve them stay in sync, others get no writes. The displayed state comes from the intentional rule alone, and the intentional rule wins when the legacy rules disagree. The two legacy user-mention rows collapse into one. Adds unit tests for the new rows and the sync monitor, and Playwright specs that run against Synapse with msc4210_enabled both off and on.
The first version made the intentional mention rules the rows everywhere, which dropped the "Messages containing my display name" row and changed the displayed state and sync direction for servers that still serve the legacy rules. Users of such servers must see no change. The legacy definitions, the sync monitor, the strings and the screenshot baseline are back to what develop has. .m.rule.is_user_mention and .m.rule.is_room_mention get definitions of their own and are only rendered when the legacy rule they replace is absent from the user's rules; otherwise they stay hidden and remain the legacy rows' synced rules, as before. Unit and Playwright tests now assert both server modes.
|
|
github-actions
Bot
added
the
Z-Community-PR
barodeur
commented
Sep 11, 2026
Comment on lines
+24
to
+36
| /** | ||
| * Record every failed request to the push rules API, so a test can assert that | ||
| * the client never writes to a rule the server does not have. | ||
| */ | ||
| function trackPushRuleErrors(page: Page): string[] { | ||
| const errors: string[] = []; | ||
| page.on("response", (response) => { | ||
| if (response.url().includes("/pushrules/") && response.status() >= 400) { | ||
| errors.push(`${response.status()} ${response.request().method()} ${response.url()}`); | ||
| } | ||
| }); | ||
| return errors; | ||
| } |
Contributor
Author
There was a problem hiding this comment.
This should prevent introducing a bug similar to #34999. Where Element Web tries to write rules that aren't served anymore and HS returns 404.
A failed push rule write now re-reads the rules from the server, so that a settings tab left open across a homeserver upgrade recovers on the next toggle instead of retrying against rows the server no longer serves. Tests: the two write tests for the legacy rows are merged into one; unit tests cover the mid-session switch with the legacy write accepted and with it rejected; the sync monitor test that will go away with the legacy definitions is named as transitional, and a guard checks that matrix-js-sdk does not add legacy rules as client defaults; a Playwright test follows a preference across the switch. Claude-Session: https://claude.ai/code/session_01BGRjcEDsBoUibL17XP98ww
3 tasks
…-intentional-rules
Contributor
Author
|
CI seems to be failing because of a flaky test |
Comment on lines
+92
to
+98
| @@ -93,6 +93,9 @@ const RULE_DISPLAY_ORDER: string[] = [ | |||
| RuleId.ContainsUserName, | |||
| RuleId.AtRoomNotification, | |||
| RuleId.ContainsDisplayName, | |||
| // Shown in place of the legacy rules above once the server no longer serves them | |||
| RuleId.IsUserMention, | |||
| RuleId.IsRoomMention, | |||
Member
There was a problem hiding this comment.
Suggested change
| RuleId.IsRoomMention, | |
| // Mentions | |
| RuleId.IsUserMention, | |
| RuleId.IsRoomMention, | |
| // Legacy mentions rules, takes priority until the server no longer serves them | |
| RuleId.ContainsUserName, | |
| RuleId.AtRoomNotification, | |
| RuleId.ContainsDisplayName, |
We can put the new rules as default and put the legacy comment on the old ones
| await expect(userMentionRow.getByRole("radio", { name: "Off", exact: true })).toBeChecked(); | ||
| await expect(userMentionRow.getByText(UPDATE_ERROR)).toHaveCount(0); | ||
|
|
||
| let rules = await fetchRules(); |
Member
There was a problem hiding this comment.
Instead of fetching the push rules. I wonder if we should send a mention in a room and check that the notification is correct. It'll completely test that it's working from the settings to the room
Contributor
Author
There was a problem hiding this comment.
I just modified the test in 1b2a04d so
- for each notification configuration
- a bot would
- create a room
- invite the user
- then mention the room or the user
- The notification is then verified as aria-label on the room row
- a bot would
┌──────────────┬──────────────────────────────────┬──────────────┬─────────────────┐
│ Room name │ Settings state when the bot │ Bot message │ Expected label │
│ │ sends │ │ │
├──────────────┼──────────────────────────────────┼──────────────┼─────────────────┤
│ defaults │ server defaults, both rules │ user mention │ 1 unread │
│ user │ Noisy │ │ mention │
├──────────────┼──────────────────────────────────┼──────────────┼─────────────────┤
│ defaults │ server defaults, both rules │ @room │ 1 unread │
│ room │ Noisy │ mention │ mention │
├──────────────┼──────────────────────────────────┼──────────────┼─────────────────┤
│ user off │ user mentions Off, @room On │ user mention │ 1 unread │
│ │ │ │ message │
├──────────────┼──────────────────────────────────┼──────────────┼─────────────────┤
│ room on │ user mentions Off, @room On │ @room │ 1 unread │
│ │ │ mention │ message │
├──────────────┼──────────────────────────────────┼──────────────┼─────────────────┤
│ user back on │ user mentions Noisy, @room On │ user mention │ 1 unread │
│ │ │ │ mention │
└──────────────┴──────────────────────────────────┴──────────────┴─────────────────┘
Review feedback: rather than only reading the push rules back from the homeserver, have a bot mention the user in a room after each change and assert what the room list shows. The homeserver computes the unread and highlight counts from the user's push rules, so this covers the whole path from the settings page to the room, in both server modes. The push rules are still read back once, to check which rules each toggle wrote: the legacy rules along with the intentional ones while served, and only the intentional ones otherwise. Claude-Session: https://claude.ai/code/session_01BGRjcEDsBoUibL17XP98ww
florianduros
approved these changes
Sep 15, 2026
This branch was successfully deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fix #35032
Synapse is going to stop advertising the legacy mention rules by default (MSC4210, via its
msc4210_enabledfeature flag). It will only serve the intentional mention rules.When that feature is enabled, the mention notification settings disappear completely:
Screenshot
This PR makes sure users can still configure their mention notifications when only the intentional rules are served.
Nothing changes for a server that still serves the legacy rules: same rows, same displayed state, same writes to both rule sets. The intentional rows only appear once the legacy rules are absent from the user's push rules.
Server side: element-hq/synapse#20215 migrates existing legacy overrides onto the intentional rules before Synapse stops serving the legacy rules, so preferences survive the switch for every user.
Checklist
public/exportedsymbols have accurate TSDoc documentation.