Three follow-ups from CI that the level-4 pass got wrong:
1. `account.php` / `users.php`: `Document::find()` returns `mixed`
(specifically `Document|false` in practice), not `Document`. The
earlier `@var Document $oldTarget` docblocks were lies, and the
runtime `instanceof Document` guards were load-bearing — removing
them caused `Call to a member function isEmpty() on false` 500s
on the `PATCH /v1/users/:id/email` and `/phone` endpoints (and the
analogous `/v1/account/email`, `/v1/account/phone` flows). Dropped
the misleading `@var` docblocks and restored
`$oldTarget instanceof Document && !$oldTarget->isEmpty()`.
2. `Installer/Runtime/Config::setEnabledDatabases()` is a boundary
that actually takes arbitrary user/compose input — not a trusted
`string[]`. The `is_string($v)` filter was covering for that, and
`ConfigTest::testSetEnabledDatabasesFiltersInvalid` explicitly
asserts it. Widened the PHPDoc to `array<mixed>` and restored
`is_string($v) && $v !== ''` in the filter.
3. `OAuth2/Apple::getAppSecret()` wrapped `json_decode` in a
`try/catch (\Throwable)` — but `json_decode` without
`JSON_THROW_ON_ERROR` returns `null` on failure, it doesn't throw.
PHP 8.3's PHPStan flagged the catch as dead (PHP 8.5 didn't, which
is why it slipped through locally). Replaced with
`if (!\is_array($secret)) throw`, which preserves the original
"invalid secret" guard.
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.
Exposes two new optional boolean params on the three migration
creation endpoints so CSV / JSON / appwrite-to-appwrite imports can
choose how to handle rows whose IDs already exist at the destination.
Endpoints updated (app/controllers/api/migrations.php):
- POST /v1/migrations/appwrite
- POST /v1/migrations/csv/imports
- POST /v1/migrations/json/imports
Parameter semantics:
- overwrite=true -> destination uses upsertDocuments instead of
createDocuments; existing rows are replaced
with imported values
- skip=true -> destination wraps createDocuments in
skipDuplicates; existing rows are preserved
unchanged, duplicate-id rows silently no-op
- both false -> default; fails fast on DuplicateException
(original behavior, unchanged)
- both true -> overwrite wins (upsert subsumes skip)
Both params are stored in the migration Document's options array
(matches the existing pattern for destination behavior config like
path, size, delimiter, bucketId, etc.) and read back in the worker's
processDestination() to instantiate DestinationAppwrite with the
new constructor params.
Feature-branch note: depends on utopia-php/migration#feat/skip-duplicates
(DestinationAppwrite constructor params) which in turn depends on
utopia-php/database#852 (skipDuplicates scope guard). composer.json is
temporarily pinned to dev-feat/skip-duplicates and
dev-csv-import-upsert-v2 respectively; both must be reset to proper
release versions once the upstream PRs merge.
- SessionCreated event now carries only domain data (no isFirstSession)
- Mails listener uses ordered guard clauses, deferring the DB query
until cheaper checks pass
- Drop $user Document allocation in favour of direct array access
- Inline FileName validator and $smtpEnabled into their use sites
- Extract $isBranded to eliminate duplicate APP_BRANDED_EMAIL_BASE_TEMPLATE check
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Moves session alert email side effect out of the account controller
into a dedicated `Mails` listener that reacts to a new `SessionCreated`
bus event. The event is now always dispatched on session creation; the
listener owns all conditional logic (first session, sessionAlerts flag,
email-link sessions, user email presence).
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>