Skip to content

Remove the legacy mention push rules by default - #20220

Open
barodeur wants to merge 12 commits into
developfrom
barodeur/disable-legacy-mention-rules-by-default
Open

barodeur wants to merge 12 commits into
developfrom
barodeur/disable-legacy-mention-rules-by-default

Conversation

@barodeur

@barodeur barodeur commented Sep 11, 2026

Copy link
Copy Markdown
Collaborator

🚨 ⚠️ DO NOT MERGE. This will break Element Web: element-hq/element-web#35032

Fix element-hq/element-web#35031


Part of #19415 (Support Matrix 1.17).

Companion test PRs:

What this does: experimental_features.msc4210_enabled default value changed true (MSC4210). 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, element-web@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.

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

Opt-out

experimental_features:
  msc4210_enabled: false

Pull Request Checklist

  • Pull request is based on the develop branch
  • Pull request includes a changelog file. 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.
  • Code style is correct (run the linters)

Matrix v1.17 removed `.m.rule.contains_display_name`,
`.m.rule.contains_user_name` and `.m.rule.roomnotif` from the base push
rule set. Default `msc4210_enabled` to `true` so that Synapse serves the
v1.17 rule set; the option remains as an opt-out.
… merge)

Synapse CI only fetches Complement from matrix-org/complement, so the
matching Complement branch on the fork is never picked up. Try the fork
first, falling back to matrix-org, so this PR can be tested against
that branch before it is merged into Complement main.

Revert before merging.
@barodeur
barodeur marked this pull request as ready for review September 15, 2026 09:10
@barodeur
barodeur requested a review from a team as a code owner September 15, 2026 09:10
@barodeur
barodeur requested review from sandhose and removed request for a team September 15, 2026 09:10
erikjohnston pushed a commit to matrix-org/sytest that referenced this pull request Sep 15, 2026
…ention (#1430)

"Don't get pushed for rooms you've muted" expects the second message to be
pushed because its body contains the recipient's user ID. That relies on
the legacy .m.rule.contains_user_name push rule, which Matrix v1.17
(MSC4210) removes and which Synapse disables by default from 1.161.0
(element-hq/synapse#20220).

Send an intentional mention via m.mentions instead, so the message is
pushed by .m.rule.is_user_mention on every server that follows the
current spec.
reivilibre pushed a commit to matrix-org/complement that referenced this pull request Sep 15, 2026
#920)

The two messages that are expected to highlight Bob only contained his
user ID in the body text. That relies on the legacy push rules
`.m.rule.contains_user_name` / `.m.rule.contains_display_name`, which
Matrix v1.17 removes (MSC4210). Synapse disables them by default from
1.161.0 (element-hq/synapse#20220), so the test fails there with
`highlight_count: 0`.

Add an `m.mentions` block to both messages so they are highlighted via
`.m.rule.is_user_mention` (Matrix v1.7). The expected counts are
unchanged: a message with `m.mentions` produces exactly one highlight
whether or not the legacy rules are enabled.
Comment thread tests/push/test_http.py Outdated
"m.room.message",
{
"msgtype": "m.text",
"body": "Oh, user, hello!",

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I feel like we should not be mentioning the display name in the body anymore, as what we want to test is the intentional mention? Not really a strong opinion about this

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see what you mean.
I think we should try to keep a realistic payload in those higher level tests so probably @user in the body as well as the appropriate m.mentions.

But I agree in other tests, some lower level unit tests, we could verify that m.mentions is sufficient without @user or the display name being present in the body.

Comment thread tests/push/test_http.py
"m.room.message",
{
"msgtype": "m.text",
"body": "@room eeek! There's a spider on the table!",

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same for @room mentions

The message body used the bare display name, which is what the legacy
mention rules matched on. Write it as a client would render a mention
pill, `@user`, next to `m.mentions`, and name and document the test
accordingly.
@MadLittleMods MadLittleMods added A-Push A-Spec-Compliance Matrix spec compliance labels Sep 23, 2026

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-Push A-Spec-Compliance Matrix spec compliance

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants