Clarify that .m.rule.master has a higher priority than any push rule

[zecakeh]

This is a bit controversial because this is only enforced by Synapse. Dendrite and Conduit treat it like other server-default push rules, as the spec says. However Synapse's implementation was probably the source of the spec's definition in the first place, and clients like Element Web rely on this behavior to disable notifications.

Related to #668.

Signed-off-by: Kévin Commaille zecakeh@tedomum.fr

Related PR in Ruma that would allow to fix Conduit's implementation: ruma/ruma#1364

Preview: https://pr1320--matrix-spec-previews.netlify.app

[dbkr]

I'm not really sure it was ever intended to work this way, except that the master rule should always be first & therefore naturally highest precedence. It feels like a weird exception. Do you have a link to where Synapse implements this behaviour?

[zecakeh]

Sure, currently it's visible in Rust that the rules are assembled like that: https://github.com/matrix-org/synapse/blob/develop/rust/src/push/mod.rs#L384-L400

Where m.rule.master is part of the BASE_PREPEND_OVERRIDE_RULES.

It was already introduced as the only preprended rule in this commit: matrix-org/synapse@04f8478

[dbkr]

Right - this is what I was trying to say: the master rule isn't explicitly first, it's just implicitly first because it's the first prepend override rule.

[zecakeh]

Does that mean we should define prepend override rules in the spec instead, with this rule inside?

[dbkr]

Oh, I think I see what you're saying now: the spec says default rules have lower precedence than user defined rules but synapse prepends some override rules to the start (of which the only one is the master rule) giving them higher precedence. In which case, apologies, I think you were correct in the first place. There's a bit of a question over whether a user could, if they really wanted, manually define rules that the master rule doesn't affect, but agreed it's probably best to fix the spec to match reality since clients expect the rule to work to turn off push altogether.

Will re-request review in case someone disagrees with any of the above.

[richvdh]

There's a bit of a question over whether a user could, if they really wanted, manually define rules that the master rule doesn't affect

Currently they can't do this with Synapse.

[richvdh]

lgtm