Skip to content

default push notification to be noisy#6281

Open
Reaster0 wants to merge 1 commit intomatrix-org:mainfrom
Reaster0:default-noisy-push2
Open

default push notification to be noisy#6281
Reaster0 wants to merge 1 commit intomatrix-org:mainfrom
Reaster0:default-noisy-push2

Conversation

@Reaster0
Copy link

some servers (continuwuity) don't send any actions/tweaks in the push notifications after push registration using the flag event_id_only that discard them (just as synapse btw),

in that case they send push notifications without filling the "tweaks" field https://spec.matrix.org/v1.17/push-gateway-api/#post_matrixpushv1notify_request_device

other clients like fluffychat, element classic, etc default this situation to noisy notifications but element x default to silent (because of the behaviour of this lib) in that case there's no way to get noisy notifications on element x and continuwuity,

more informations about this issue can be found there:
element-hq/element-x-ios#5132
https://forgejo.ellis.link/continuwuation/continuwuity/issues/1533
https://forgejo.ellis.link/continuwuation/continuwuity/issues/1424

@Reaster0 Reaster0 requested a review from a team as a code owner March 12, 2026 11:04
@Reaster0 Reaster0 requested review from andybalaam and removed request for a team March 12, 2026 11:05
@codecov
Copy link

codecov bot commented Mar 12, 2026

Codecov Report

❌ Patch coverage is 75.00000% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 89.91%. Comparing base (a48f238) to head (a73514f).
✅ All tests successful. No failed tests found.

Files with missing lines Patch % Lines
crates/matrix-sdk-ui/src/notification_client.rs 75.00% 1 Missing ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##             main    #6281   +/-   ##
=======================================
  Coverage   89.90%   89.91%           
=======================================
  Files         373      373           
  Lines      102570   102573    +3     
  Branches   102570   102573    +3     
=======================================
+ Hits        92219    92229   +10     
  Misses       6790     6790           
+ Partials     3561     3554    -7     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@codspeed-hq
Copy link

codspeed-hq bot commented Mar 12, 2026

Merging this PR will not alter performance

✅ 50 untouched benchmarks


Comparing Reaster0:default-noisy-push2 (a73514f) with main (a48f238)

Open in CodSpeed

@BillCarsonFr
Copy link
Member

@Reaster0 I already commented in your initial PR #6277 (comment)

Your change request is non complient to spec. As per spec notify means that the event should generate a notification. The fact that a sound must be played depends on the sound tweak.

As I said here element-hq/element-x-ios#5132 (comment) when disucssing with kovapatrik, it looks like continuwuity and synapse are providing different base ruleset.

So until we sort this out, we would be comparing apples to oranges.
Maybe the rule triggerd by the continuwuity base ruleset is just silent?

@Reaster0
Copy link
Author

Reaster0 commented Mar 12, 2026

(in reality i'm making theses pr juste to get some reactions, i have no real goal of getting this one merged, it's more like a draft idea)
hi @BillCarsonFr could you send me the spec part that specify that no sound tweak should me interpreted by the client as a silent notification?
i was discussing with @nex:continuwuity.org to get a debug build that print the push debug log and noticed that the "tweaks" field was always empty in my testings with element X or fluffychat, and fluffychat gave me noisy notifications, while EX not, so one of the two is not following this "spec"

probably just as synapse who discard tweaks when "event_id_only" is set, even if the spec say otherwise:

Homeserver behaviour

This describes the format used by “HTTP” pushers to send notifications of events to Push Gateways. If the endpoint returns an HTTP error code, the homeserver SHOULD retry for a reasonable amount of time using exponential backoff.

When pushing notifications for events, the homeserver is expected to include all of the event-related fields in the /notify request. When the homeserver is performing a push where the format is "event_id_only", only the event_id, room_id, counts, and devices are required to be populated.

Note that most of the values and behaviour of this endpoint is described by the Client-Server API’s Push Module.

http://spec.matrix.org/v1.17/push-gateway-api/#post_matrixpushv1notify_request_device
the spec don't specify that the homeserver should drop tweaks (clients[].tweaks) when event_id_only is set,
but synapse seems to do it anyway (but still send some after? maybe with a default value? idk can't check/confirm)

@BillCarsonFr
Copy link
Member

BillCarsonFr commented Mar 12, 2026

hi @BillCarsonFr could you send me the spec part that specify that no sound tweak should me interpreted by the client as a silent notification?

https://spec.matrix.org/v1.17/client-server-api/#actions

notify: This causes each matching event to generate a notification.
set_tweak -> sound : set_tweak [...] the sound to be played when this notification arrives

Regarding that:

discard tweaks when "event_id_only"

Modern Element clients only use the event_id_only mode.
The other mode was mainly for non-encrypted room, where the server can process the push rules, and directly find if an event should send a push with the message in payload and if it should make a sound.

Most of the things are encrypted now, so the server cannot process the push rules.
That means that now the homeserver send a data push (silent push) for every message.
Then clients on reception of the push, do sync to get the message, decrypt the message then process the pushrules locally; After that they know if the push should result in a visible notification and if it should make noise.

And in our specific case, it appears that synapse and continuwuity are using different base rulesets.
So the client is processing a different ruleset, that alone could explain why the same message is noisy or not for the same client.

@andybalaam andybalaam removed their request for review March 12, 2026 13:19
@andybalaam
Copy link
Member

Removing myself as a reviewer since this is not intended to be merged (?) You will need to get a reviewer if/when this is ready to be merged.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants