Locks the bug-fix invariants from PR #12195's last review pass and rounds out
worker channel coverage:
C1 testEmailSendFailureDoesNotPersistAlert — SMTP throw must NOT leave a dedup
row behind, retry must deliver and persist exactly once.
C2 testNotificationEventResetClearsAllState — reset() drops every state-bearing
field including preview (regression: missed in original reset body).
C3 testWebhookSendAlertResetsBetweenCalls — Webhooks::sendAlert must reset()
the DI-shared Notification event so two paused-webhook alerts in one worker
pass do not bleed recipients/subject/body into each other.
C4 testConsoleAdapterTreatsDuplicateAsDelivered — Duplicate on createDocument
must surface as a successful idempotent send, not a per-recipient error.
M7 testTrackingPixelRejectsJwtWithoutPurposeClaim — Track endpoint silently
ignores JWTs missing or with the wrong purpose claim (defends against
replaying session/reset JWTs to mark alerts read).
Worker happy-path tests: testEmailChannelHappyPath, testConsoleChannelHappyPath,
testWebhookChannelHappyPath cover the full per-channel dispatch contract end
to end, including HMAC signing for webhooks and the tracking pixel injection
+ post-send persistence for email.
Also extracts CapturingWebhook into its own PSR-4 file so reused across tests.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
- Account/Alerts/Track endpoint is scope:public (email clients have no
session); SDK Method now declares auth: [] so generators do not require
an auth header for an unauthenticated endpoint.
- NotificationsTest setUp now creates the production `_key_recipient`
UNIQUE composite index on (messageId, channel, userId, teamId), and
testPersistAlertReturnsExistingAlertIdOnDuplicate exercises the
DuplicateException → return-existing-alertId branch end-to-end (both
primary-key collision and unique-index collision via a sibling $id).
- Tracking-pixel e2e now asserts _APP_OPENSSL_KEY_V1 is set instead of
silently falling back to the .env.example placeholder, so a missing
CI secret fails loudly rather than passing against the wrong key.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The `alerts` collection lives in the platform database, but the Notifications
worker was injecting `dbForProject` and writing alerts there. Webhooks
dispatches with the user's project context, so alerts were being written to
the user's project DB (which has no `alerts` collection), and `/v1/account/alerts`
(which reads from `dbForPlatform`) returned nothing.
Switch the worker to inject `dbForPlatform` instead. All alert reads (dedup
lookup) and writes (persistAlert + ConsoleAdapter) now target the platform DB,
independent of which project the dispatching event belongs to.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Cover the new worker behaviors introduced in waves 1-3:
- testDedupQueriesByAttributeNotById: prove alreadyDelivered() queries the
messageId attribute, not getDocument($messageId), by seeding a row with
a non-matching $id but matching messageId.
- testConsoleChannelSkipsPersistAlert: confirm the action loop does NOT
call persistAlert for console recipients (the adapter persists).
- testConsoleZeroDeliveryThrows: surface adapter failures via the worker.
- testMultiRecipientFanoutNoCollision: same messageId across recipients
must produce distinct $id values.
- testRecipientStructRoundtripsUserIdAndTeamId: persisted alert carries
recipient userId/teamId.
- testTrackingPixelInjectedIntoEmailHtml: verify the tracking pixel is
spliced before the last </body> tag with the expected URL shape.
- testPersistAlertReturnsAlertIdAndStoresUserId: dispatchEmail's
persistAlert returns a resolvable id and stores userId + read=false.
ConsoleTest: add testMultiRecipientWithSameMessageIdGeneratesDistinctIds
and update existing tests to use the post-ST2 compound `$id` form
(messageId + 8-hex md5 suffix).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Apply the four Greptile P1 fixes to the Notifications worker and
extend it for C3 email read tracking and ST4's stripped SMTP plumbing.
P1 #1: alreadyDelivered() now queries the indexed messageId attribute
instead of getDocument($messageId). The action loop and Console
adapter both write compound `$id`s (messageId + recipient hash), so
the previous direct-id lookup always missed.
P1 #3: action() no longer calls persistAlert after dispatchConsole;
ConsoleAdapter persists internally. Email persists inside
dispatchEmail BEFORE the adapter send so the alertId is available for
the tracking pixel; webhook persists in the action loop after a
successful HTTP send.
P1 #4: dispatchConsole now throws when the adapter reports
`deliveredTo === 0`, surfacing the per-recipient error.
Recipient threading: dispatch() now takes the full recipient map and
returns the alertId (or null when persistence is the caller's
responsibility). persistAlert() reads userId/teamId from the
recipient and grants per-user / per-team-owner CRUD permissions,
falling back to payload permissions only when neither is set. The
returned alertId lets dispatchEmail splice a 1x1 tracking pixel
before the last `</body>` tag, signed with a 30-day HS256 JWT
(_APP_OPENSSL_KEY_V1) carrying {alertId, userId}.
SMTP resolution: ST4 stripped `smtp` and `customMailOptions` from
the Notification event payload, so the worker now resolves SMTP
from the injected project Document (mirroring Mails.php /
Memberships/Create.php), falling back to the env-driven cloud SMTP
adapter when the project has no enabled override.
Tests updated: SpyNotifications.dispatch() matches the new
signature and emulates per-channel persistence so existing routing
assertions keep their semantics. Memory `alerts` collection adds
the `read` boolean attribute to mirror platform.php.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Drop the global _APP_NOTIFICATIONS_WEBHOOK_SECRET env var. There's no
analogous global webhook secret in Appwrite; the existing Webhooks
worker carries a per-webhook signatureKey on the webhook document.
Move the same pattern into the Notification event: each webhook
recipient may carry an optional signatureKey, which the worker
forwards to the Webhook adapter for HMAC-SHA256 signing. Recipients
without a key are delivered unsigned and a tag is logged for audit.
Covers the dedup short-circuit, per-channel dispatch routing, alert
persistence, error tagging, and legacy single-recipient fallback in the
Notifications worker, plus the Console adapter's permission shape and the
Webhook adapter's HMAC-SHA256 signing contract, header layout, response
handling, and unsigned-when-secret-missing behaviour.
Worker dispatch helpers move from private to protected so a test spy can
override them without monkey-patching. The Swoole runtime hook flag
mutation is now guarded by class_exists so the action can run under bare
PHPUnit (no Swoole extension on the test host).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Raises `phpstan.neon` level from 3 to 4 and fixes the 549 new errors
that level 4 surfaces across 157 files. Fixes are root-cause — no
`@phpstan-ignore`, no `@var` casts, no baseline entries, no widened
types. A handful of latent bugs were fixed along the way:
- `app/controllers/general.php`: path-traversal guard was negating
`\substr(...)` before the strict comparison (`!\substr(...) === $base`
was always `false === $base`). Rewritten as `\substr(...) !== $base`.
- `src/Appwrite/Platform/Modules/Databases/Http/Databases/Logs/XList.php`
and `.../TablesDB/Logs/XList.php`: were importing the raw Matomo
`DeviceDetector` (whose `getDevice()` returns `?int`) but treating the
result as an array with `deviceName/deviceBrand/deviceModel` keys.
Swapped to `Appwrite\Detector\Detector`, matching the wrapper already
used a few lines below for `$os`/`$client`.
- `src/Appwrite/Platform/Modules/Functions/Workers/Builds.php`: a match
key was checking `$resourceKey === 'functions'` when `$resourceKey`
is `'functionId'|'siteId'` — always false. Switched to the intended
`$resource->getCollection() === 'functions'` check.
- `src/Appwrite/OpenSSL/OpenSSL.php`: `encrypt()` return type tightened
to `string|false` to match `openssl_encrypt`; this lets callers'
`=== false` error handling remain meaningful.
- `app/controllers/api/messaging.php`: removed a dead
`array_key_exists('from', [])` branch in the Msg91 provider (empty
array literal; branch was unreachable).
Large cleanup categories across the 549 fixes:
- Removed redundant `?? default` on array offsets and expressions that
PHPStan now knows are non-nullable.
- Removed unreachable statements (mostly `return;` after `throw` or
`markTestSkipped()`).
- Removed redundant `is_array`/`is_string`/`is_bool`/`instanceof` checks
on already-narrowed types.
- Added `default =>` arms (or throwing arms) to non-exhaustive matches
on `string`/`mixed` input.
- Removed dead `$document === false` branches where method return types
were tightened to non-nullable `Document`.
- Removed unused properties (`$version` on Etsy/Zoom OAuth2, `$paths` on
Installer State, `$source` on MigrationsWorker, `$account2` on two
GraphQL auth tests), unused traits (`ApiVectorsDB`, `DatabaseFixture`),
and an unused `cleanupStaleExecutions` task method.
- Replaced `assertTrue(true)` and redundant `assertIsArray`/`assertIsString`/
`assertNotNull` assertions with `addToAssertionCount(1)` or
`assertNotEmpty` where the runtime type was already known.
- Initialize $isUpgrade=false in Install.php action() to prevent undefined variable
- Assign $this->lockedDatabase in Upgrade.php before calling parent::action()
- Remove stack trace exposure from buildErrorDetails() in Http Install action
- Suppress raw exception messages for 500+ errors in Error handler
- Remove sessionSecret from progress details to prevent credential leak
- Hash name/email in analytics payload to avoid sending raw PII
- Validate and default dbService in compose.phtml to prevent invalid output
- Fix host normalization in progress.js redirect URL builder
- Release global lock on early return for existing installation conflict
- Consolidate duplicate database host/port assignment blocks
- Add @runInSeparateProcess to testRouteRegistration to prevent global state leak
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Jake BarnbyandClaude Opus 4.7
Chirag Aggarwal
Steven NguyenandGitHub