fix(client): restore message notification fallback when push is unavailable#117
Conversation
Code Coverage OverviewLanguages: JavaScript JavaScript / code-coverage/vitestThe overall coverage in the branch remains at 22%, unchanged from the branch. Show a code coverage summary of the most impacted files.
Updated |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 65c1c09e16
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8c7a080786
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Deploying with
|
| Status | Preview URL | Commit | Alias | Updated (UTC) |
|---|---|---|---|---|
| ✅ Deployment successful! | https://pr-117-charm.justin-tech.workers.dev | a434f7c | pr-117 |
Wed, 17 Jun 2026 02:43:35 GMT |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a434f7cdd3
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| user_id: mx.getUserId() ?? undefined, | ||
| }, | ||
| }); | ||
| const noti = new window.Notification(osPayload.title, osPayload.options); |
There was a problem hiding this comment.
When web push is enabled and a desktop tab is hidden/unfocused but still syncing, this line now always creates a page-owned OS notification; useAppVisibility also enables the Matrix web pusher when the tab goes hidden, so the service worker can show the Sygnal push for the same event. This reintroduces duplicate notifications for any background tab that still has a live sync connection; the page fallback needs to be skipped when a usable push transport is active.
Useful? React with 👍 / 👎.
| }); | ||
| throw err; | ||
| } | ||
| navigator.serviceWorker.controller?.postMessage({ |
There was a problem hiding this comment.
On a first install or after a service-worker update, navigator.serviceWorker.ready can resolve while the current tab is still uncontrolled because sw.ts intentionally avoids clients.claim(). In that state this optional controller?.postMessage is dropped, and even when it is delivered the page never observes failures from the service worker's /pushers/set fetch, so enablePushNotifications can resolve and settings can mark background push enabled after only creating a browser subscription with no Matrix pusher registered.
Useful? React with 👍 / 👎.
| lang: navigator.language || 'en', | ||
| data: { | ||
| url: clientConfig.pushNotificationDetails?.pushNotifyUrl, | ||
| format: 'event_id_only' as const, |
There was a problem hiding this comment.
For deployments with pushNotificationDetails.declarativeWebPushFallback enabled, the previous pusher builder added declarative_web_push: true to the pusher data so compatible gateways would emit Declarative Web Push payloads. The new hand-built pusher data omits that flag in this registration path (and the reuse path has the same shape), so those installs silently lose the configured WebKit declarative fallback.
Useful? React with 👍 / 👎.
1 participant
Description
Revert the fork-specific push suppression and startup reconciliation changes, and align Charm’s push/visibility behavior with upstream again.
This follow-up does three things:
togglePusher(...)visibility handling instead of keeping web push permanently enabledsetAppVisiblereporting instead of heartbeat / foreground probe suppressionThe goal is to get background push working again with the same simpler contract upstream uses, while keeping the existing media/session recovery work from the broader branch.
Related to #116
Type of change
Checklist: