fix(messaging): reach targetless users through Appwrite push when it isn't the default by ArnabChatterjee20k · Pull Request #14586 · appwrite/appwrite · GitHub
Skip to content

fix(messaging): reach targetless users through Appwrite push when it isn't the default - #14586

Merged
ArnabChatterjee20k merged 3 commits into
fix/mqtt-topic-send-by-topic-idfrom
fix/mqtt-user-fallback-appwrite-provider
Oct 16, 2026
Merged

ArnabChatterjee20k merged 3 commits into
fix/mqtt-topic-send-by-topic-idfrom
fix/mqtt-user-fallback-appwrite-provider

Conversation

@ArnabChatterjee20k

@ArnabChatterjee20k ArnabChatterjee20k commented Oct 16, 2026 •

Copy link
Copy Markdown
Member

Stacked on #14580. Review and merge that first; this PR then retargets to main.

What does this PR do?

createPush({ users: [...] }) reaches a user who has no push target through Appwrite push on users/<userId>, so an app can receive direct messages without registering a target. Until now that no-target fallback in streamRecipients ran only when $default was Appwrite. $default is findOne(providers, enabled, type = push), in practice the first push provider created. A project that set up FCM or APNs before Appwrite push got No valid recipients found. for those users, even with an Appwrite provider enabled.

The default provider doesn't send by itself. A user with no targets has no FCM or APNs token, so in that case nothing reached them at all.

Fix

  • checkAppwritePushEnabled() becomes getAppwritePushProvider(). It returns the enabled Appwrite push provider (the default when that's Appwrite), resolved once per send and seeded into the provider cache.
  • The targetless fallback yields that provider's page, whatever the default is.
  • The wake-up flag uses the same lookup, so its behaviour is unchanged.

The project's default provider is left alone on purpose. Targets saved without a providerId follow the default, and making Appwrite the default would reroute their FCM and APNs tokens to MQTT.

message.providers only lists providers the send used

The worker built the message.providers span, and its own appended search text, from $providers, the resolved-provider cache. That cache is seeded with the default provider, and now with the enabled Appwrite provider. So a send delivered entirely through another provider still reported providers it never touched.

sendBatch() now returns the provider it ran on. The worker collects those providers from page batches and wake-up batches, and builds the span from that set. The cache keeps its seeding, so there are no extra queries.

The message search field itself was never affected. The messageSearch attribute filter (app/init/database/filters.php) recomputes it on every write from the ID, description, status, title and type, so the worker's appended provider names are overwritten on save. That append loop is effectively dead code and is left as is here.

Matrix: user sends with Appwrite + FCM enabled

U's live push targets Default = Appwrite Default = FCM, before Default = FCM, after
none MQTT users/U ❌ failed, no recipients MQTT users/U
FCM / APNs only FCM / APNs full FCM / APNs full unchanged
Appwrite (± APNs) MQTT (+ APNs wake-up) MQTT (+ APNs wake-up) unchanged

Users holding an FCM or APNs target but no Appwrite target are deliberately left alone. They already get the full notification, and adding MQTT would deliver it twice.

Trade-off

For each targetless user, a send:

  • provisions their users/<userId> topic row on first use;
  • writes a pushLedger row (kept for 7 days for replay);
  • counts toward deliveredTotal and messages.sent, whether or not a device is connected.

This already happened when Appwrite was the default. It now applies whenever an Appwrite provider is enabled. We keep it on purpose: receiving on users/<userId> with no target setup is a deliberate convenience.

Test Plan

E2E in tests/e2e/Services/Mqtt/MqttServerTest.php, each in a fresh project so FCM really is the first enabled push provider:

  • testUserTopicReceivesPushWhenAnotherProviderIsDefault: FCM is created first, then Appwrite. A targetless user receives the message on users/<userId> and the message ends sent.
  • testUserPushWithoutEnabledAppwriteProviderFails: FCM is enabled and Appwrite disabled. A targetless user's message ends failed.

The FCM provider uses placeholder credentials. The user holds no FCM token, so Google is never called.

The Mqtt CI lane mapping and the rust-toolchain repin come from #14580.

Locally I ran php -l, Pint and PHPStan on the changed files. PHPStan reports no new errors; the only ones are the Utopia\Database\Id not-found errors that main also shows because the local vendor is stale. E2E runs in CI.

Related PRs and Issues

🤖 Generated with Claude Code

@tenki-reviewer

tenki-reviewer Bot commented Oct 16, 2026 •

Copy link
Copy Markdown

Review complete. No blocking issues — 1 nitpick below.

🧹 Nitpicks (1) — 🟢 1 low
  • 🟢 Unused Appwrite provider pollutes message search (Messaging.php:260) — The worker now seeds $providers[$appwrite->getId()] unconditionally whenever an enabled Appwrite push provider exists, even when delivery runs entirely through another provider such as FCM (src/Appwrite/Platform/Workers/Messaging.php:259-261).

This PR makes the messaging worker treat the Appwrite push provider as a regular deliverable provider: it is added to the $providers map, targetless users resolve to it via a new getAppwritePushProvider() lookup, and the wakeup signal fires when any enabled Appwrite push provider exists. CI and QA tooling are updated to cover the messaging worker and MQTT push adapters.

Files Change
src/Appwrite/Platform/Workers/Messaging.php Refactors checkAppwritePushEnabled into getAppwritePushProvider(), includes the Appwrite provider in provider iteration and targetless fallback delivery.
tests/e2e/Services/Mqtt/MqttServerTest.php Extends MQTT e2e coverage for the new provider behavior.
.github/ci/e2e.json Maps the messaging worker/push adapter changes to the MQTT CI lane.
.github/workflows/console-snippets-qa-server.yml QA workflow adjustments for console snippets against the server.

Reviewed commit: 5988a68

@hansi-codes

hansi-codes Bot commented Oct 16, 2026 •

Copy link
Copy Markdown
Contributor

🟢 Tier S · Ready to merge

The incremental changes only remove assertions that cannot observe the messageSearch behavior and simplify the test helper; no new defect was found.

The worker resolves an enabled Appwrite push provider independently of the default and uses it to reach targetless users, while tracking providers actually used by send batches for message metadata. MQTT E2E coverage exercises targetless delivery with another provider as default and failure when Appwrite push is disabled. The latest test update removes message-search assertions and simplifies the FCM test helper.

Latest changes: The new commits remove the provider-search assertions because the messageSearch filter makes them unobservable, and remove the now-unneeded custom FCM name parameter.

Verdict New comments Fixed Still open
✅ Approved 0 0 0
📂 Walkthrough · 2
File Change
src/Appwrite/Platform/Workers/Messaging.php Resolves the enabled Appwrite push provider independently of the default, uses it for targetless recipients, and derives message provider metadata from providers used by batches.
tests/e2e/Services/Mqtt/MqttServerTest.php Covers targetless-user delivery and failure cases, and simplifies the FCM provider helper after removing unobservable search assertions.

Reviewed the commits since cc6f082 · Details · Comment @hansi-codes review to re-run, or mention @hansi-codes with a question.

@hansi-codes hansi-codes Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟢 Tier S · Looks good to merge. Summary

@ArnabChatterjee20k
ArnabChatterjee20k force-pushed the fix/mqtt-user-fallback-appwrite-provider branch from 5988a68 to e211d5a Compare October 16, 2026 07:22
@ArnabChatterjee20k
ArnabChatterjee20k changed the base branch from main to fix/mqtt-topic-send-by-topic-id October 16, 2026 07:22
@github-actions

github-actions Bot commented Oct 16, 2026 •

Copy link
Copy Markdown

Security rules

No new WARNING or ERROR findings from security rules.

19 existing findings tracked in .semgrep/baseline.json
  • php.appwrite.guest-write-without-abuse-limit (12)
  • php.appwrite.secret-compare-timing (5)
  • php.appwrite.weak-secret-env-default (2)

Posted by Checks / Rules. Re-runs update this comment in place. Rule details and baseline: .semgrep/README.md.

@github-actions

github-actions Bot commented Oct 16, 2026 •

Copy link
Copy Markdown

✨ Benchmark results

Comparing fix/mqtt-topic-send-by-topic-id (before) → fix/mqtt-user-fallback-appwrite-provider (after).

Metric Before After Change
🚀 Requests/sec 219.67 225.64 ⚪ +2.7%
⏱️ Latency P50 77.04 ms 77.75 ms ⚪ +0.9%
⏱️ Latency P95 187.1 ms 174.48 ms 🟢 -6.7%
Per-scenario breakdown & investigation details

Metrics below reflect the current branch (after). Δ P95 compares against the base.

Scenario P50 (ms) P95 (ms) Requests RPS Δ P95 (ms)
API total 77.75 174.48 13,908 225.64 -12.62
Account 146.24 267.4 732 12.21 -37.22
TablesDB 76.38 141.3 7,564 124.56 0
Storage 68.67 159.2 3,660 62.28 -22.81
Functions 107.93 197.53 1,952 33.96 -12.74

Top API waits (after)

API request Max wait (ms)
account.prefs.update 450.74
tablesdb.rows.delete 414.73
functions.create 403.9
account.name.update 394.84
functions.variables.update 348.05

@ArnabChatterjee20k
ArnabChatterjee20k force-pushed the fix/mqtt-user-fallback-appwrite-provider branch from 3af37c0 to 3377959 Compare October 16, 2026 07:38
@ArnabChatterjee20k ArnabChatterjee20k changed the title fix(messaging): reach targetless users through Appwrite push when it isn't the default fix(messaging): require an Appwrite target for Appwrite push user sends Oct 16, 2026
…isn't the default

A users[] push reached a user with no push target on users/<userId> only
when the default (first enabled) push provider was Appwrite. With FCM or
APNs set up first, the message failed with "No valid recipients found."
even though an Appwrite provider was enabled.

Resolve the enabled Appwrite push provider once per send and use it for
the targetless fallback, whichever provider is the default. Users holding
other push targets are unchanged, so no notification is sent twice.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@ArnabChatterjee20k
ArnabChatterjee20k force-pushed the fix/mqtt-user-fallback-appwrite-provider branch from 3377959 to bdbea83 Compare October 16, 2026 07:46
@ArnabChatterjee20k ArnabChatterjee20k changed the title fix(messaging): require an Appwrite target for Appwrite push user sends fix(messaging): reach targetless users through Appwrite push when it isn't the default Oct 16, 2026
hansi-codes[bot]
hansi-codes Bot previously requested changes Oct 16, 2026

@hansi-codes hansi-codes Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 Tier B · 1 blocking finding to address. Summary

Comment thread src/Appwrite/Platform/Workers/Messaging.php
The worker appended every provider in its resolved-provider cache to the
message's search text and the message.providers span. That cache is
seeded with the default provider, and now the enabled Appwrite provider,
so a send delivered entirely through another provider was still
searchable by providers it never used.

Record the provider of each batch result, wake-ups included, and build
the search and span from that set. The cache keeps its seeding.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@ArnabChatterjee20k
ArnabChatterjee20k force-pushed the fix/mqtt-user-fallback-appwrite-provider branch from 3a821b0 to cc6f082 Compare October 16, 2026 07:51
@hansi-codes
hansi-codes Bot dismissed their stale review October 16, 2026 07:54

The findings that requested changes are resolved. See the summary comment for what is still open.

@ArnabChatterjee20k ArnabChatterjee20k mentioned this pull request Oct 16, 2026
4 tasks done
…akes unobservable

The messageSearch attribute filter recomputes a message's search text on
every write from its id, description, status, title and type, so provider
names are never stored and cannot be asserted through the search API.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

@hansi-codes hansi-codes Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟢 Tier S · Looks good to merge. Summary

@ArnabChatterjee20k
ArnabChatterjee20k merged commit 59046f8 into fix/mqtt-topic-send-by-topic-id Oct 16, 2026
29 checks passed
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.

1 participant