1.7.x #9897
1.7.x #9897
Conversation
|
Important Review skippedReview was skipped as selected files did not have any reviewable changes. 💤 Files selected but had no reviewable changes (1)
You can disable this status message by setting the 📝 WalkthroughWalkthroughThis update introduces extensive refactoring and feature additions across the codebase. The core change is a systematic replacement of all occurrences of Estimated code review effort5 (>120 minutes) Possibly related PRs
Suggested reviewers
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
Security Scan Results for PRDocker Image Scan Results
Source Code Scan Results🎉 No vulnerabilities found! |
commit: |
✨ Benchmark results
⚡ Benchmark Comparison
|
…timer interval in StatsResources
chore: fix benchmark CI
Fix: build duration calculation
…git-deployment Create commit & branch url for first git deployment when site is linked to repo
…syntax Use ->action(...)) instead of ->callback([$this, 'action']);
# Conflicts: # .github/workflows/benchmark.yml
Override project via custom domains log
…atsResources initialization
…or disableSubqueries to use class filters
fix: prevent injected $user from being shadowed
chore: update react native to 0.10.1 and dotnet to 0.14.0
Update cli 8.2.1
fix: build usage specification
Handle redirect validator in specs + GraphQL type mapper
chore: update dart 16.1.0, flutter 17.0.2 and cli 8.2.2
chore: improve invalid scheme error in origin check
# Conflicts: # app/config/specs/open-api3-1.7.x-console.json # app/config/specs/open-api3-1.7.x-server.json # app/config/specs/open-api3-latest-console.json # app/config/specs/open-api3-latest-server.json # app/config/specs/swagger2-1.7.x-console.json # app/config/specs/swagger2-1.7.x-server.json # app/config/specs/swagger2-latest-console.json # app/config/specs/swagger2-latest-server.json # app/init/constants.php # composer.lock # docs/references/databases/upsert-documents.md # tests/e2e/Services/Account/AccountCustomClientTest.php
There was a problem hiding this comment.
Actionable comments posted: 13
🔭 Outside diff range comments (6)
app/controllers/api/messaging.php (3)
2945-2952: Robustness: guard against buckets withoutsequence
$dbForProject->getDocument('bucket_' . $bucket->getSequence(), $fileId)assumes every bucket has a populatedsequence.
If a legacy bucket was created before back-fill, this call returns the wrong collection, raisingbucket_not_found.
Defensively bail out early:$seq = $bucket->getSequence(); -if (!$seq) { - throw new Exception(Exception::STORAGE_BUCKET_MISSING_SEQUENCE); -} -$file = $dbForProject->getDocument('bucket_' . $seq, $fileId); +$file = $dbForProject->getDocument('bucket_' . $seq, $fileId);
3245-3253:$currentScheduledAtis undefined – results inNoticeand wrong expiry
$scheduleTime = $currentScheduledAt ?? $scheduledAt;references$currentScheduledAt, which isn’t set in this scope.
PHP will raise a notice and$scheduleTimewill always equal$scheduledAt.Quick fix:
- $scheduleTime = $currentScheduledAt ?? $scheduledAt; + $scheduleTime = $scheduledAt;(or compute
$currentScheduledAtearlier like the update-paths do).
2208-2214: Incorrect cursor assignment – array-offset on aDocument
$cursorDocument[0]treats aDocumentas an array:$cursor->setValue($cursorDocument[0]);Use the object directly:
- $cursor->setValue($cursorDocument[0]); + $cursor->setValue($cursorDocument);Otherwise a fatal “Cannot use object as array” is triggered when the cursor code runs.
app/config/locale/translations/kn.json (1)
11-31: Fix Kannada locale code in translations
Thekn.jsonfile’ssettings.localeis set to"ka"(Georgian) instead of"kn"(Kannada), which will break locale detection at runtime. No other Kannada translation files exist in the repo.• File:
app/config/locale/translations/kn.json(line 3)- "settings.locale": "ka", + "settings.locale": "kn",app/controllers/api/migrations.php (1)
86-90:statusCounters/resourceDatastored as raw JSON stringsPersisting these as the literal string
'{}'means all later reads must remember tojson_decode(); previously these were native arrays.
Double-encoding mistakes are easy to introduce and type-hints / JSON filters in the DB won’t help.Please confirm every read-site was updated, or keep them as arrays and let the DB encode.
app/controllers/api/health.php (1)
526-542: Add null check for Redis publisher when fallback is enabled.While the implementation looks good, there's a potential issue if
_APP_WORKER_REDIS_FALLBACKcontains 'databases' but$publisherRedisis null.Add a null check to prevent potential runtime errors:
$isRedisFallback = \str_contains(System::getEnv('_APP_WORKER_REDIS_FALLBACK', ''), 'databases'); + if ($isRedisFallback && $publisherRedis === null) { + throw new Exception(Exception::GENERAL_SERVER_ERROR, 'Redis publisher not available for fallback'); + } + $size = $isRedisFallback ? $publisherRedis->getQueueSize(new Queue($name)) : $publisher->getQueueSize(new Queue($name));
🧹 Nitpick comments (28)
.github/workflows/benchmark.yml (1)
69-70:--output-format jsonrequires oha ≥ 0.5 – verify tool versionOlder
oha(<0.5.0) only supports the short-jflag. The repo you’re installing from (azlux) currently ships 0.6.0 for Debian stable, but that might drift.You could make the workflow future-proof:
-run: 'oha -z 180s http://localhost/v1/health/version --output-format json > benchmark.json' +run: | + FORMAT_ARG="--output-format json" + oha --help | grep -q -- '--output-format' || FORMAT_ARG="-j" + oha -z 180s http://localhost/v1/health/version $FORMAT_ARG > benchmark.jsonAlso applies to: 82-83
app/config/collections/projects.php (1)
1289-1290: Superfluous blank line – optional trimThe added empty line before the
deploymentssection is harmless but slightly diverges from the tight grouping used in the rest of the file.
Feel free to keep or remove – stylistic only.app/init/registers.php (1)
39-40: Ensure multiplePublicDomain::allow()calls accumulateIf
allow()overwrites instead of merges, the first domain will be dropped. Safer:PublicDomain::allow(['request-catcher-sms', 'request-catcher-webhook']);Please verify behaviour in
PublicDomainimplementation.app/config/locale/templates/email-inner-base.tpl (1)
3-3: Good UX improvement with styled button, but consider email client compatibility.The replacement of the plain URL with a styled button improves user experience and call-to-action visibility. The use of
{{buttonText}}for localization is also good practice.Consider testing the button styling across different email clients, as some may not fully support the inline CSS. You might want to add fallback styles or consider using email-safe CSS properties.
<!-- Consider adding fallback for older email clients --> <p><a href="{{redirect}}" target="_blank" style="font-size: 14px; font-family: Inter, Arial, sans-serif; color: #ffffff !important; text-decoration: none; background-color: #2D2D31; border-radius: 8px; padding: 9px 14px; border: 1px solid #414146; display: inline-block; text-align:center; box-sizing: border-box; mso-padding-alt: 9px 14px;">{{buttonText}}</a></p>app/config/locale/translations/jv.json (1)
11-11: Verify language consistency (Javanese vs. Indonesian)Most existing strings in this file use Javanese, but these new labels read closer to standard Indonesian.
Consider having a native speaker confirm the wording to keep the locale coherent.Also applies to: 24-24, 31-31
app/config/locale/translations/lt.json (1)
11-11: Minor wording check recommendedLine 24 uses “Atstatyti slaptažodį”, whereas earlier strings favour “Atkūrimas/atkurti”.
If consistency matters, you may want to align the verb choice.Also applies to: 24-24, 31-31
app/config/locale/translations/he.json (1)
24-24: Hebrew phrasing tweakPrefer “איפוס סיסמה” (or “אפס סיסמה”) for natural grammar and correct spelling.
app/controllers/api/messaging.php (1)
2510-2519: *Sequence stored under “InternalId” – consider renaming or documentingThese attributes now hold
getSequence()values, yet their names still readtopicInternalId,targetInternalId, anduserInternalId.
That mismatch can confuse maintainers and breaks the “Principle of Least Surprise”. Either:
- Rename the keys to
topicSequence,targetSequence,userSequence, or- Add an inline comment explaining the historical reason for keeping the old names.
At minimum, ensure DB indexes / migrations are aligned with the new integer sequence values.
app/config/locale/translations/la.json (1)
4-12: Minor localisation & formatting concerns
- Line 4:
"ltr*"contains an asterisk that looks accidental – should be"ltr".- Line 24: Button text
"Reset password"is still English, unlike the other two keys just added. Consider a Latin equivalent (e.g."Tesseram restitue").- "settings.direction": "ltr*", + "settings.direction": "ltr", - "emails.recovery.buttonText": "Reset password", + "emails.recovery.buttonText": "Tesseram restitue",Also applies to: 24-25, 31-32
app/config/locale/translations/de.json (1)
28-28: Minor typo in unchanged textUnrelated to this PR’s additions, but line 28 has a spelling error:
"Du erhälst …"→ should be"Du erhältst …".app/config/errors.php (1)
396-405: Consider using 403 for “prohibited” actions
MEMBERSHIP_DELETION_PROHIBITEDandMEMBERSHIP_DOWNGRADE_PROHIBITEDare user-action blocks rather than malformed requests; HTTP 403 (“Forbidden”) would align better with similar errors (e.g.,STORAGE_FILE_NOT_PUBLIC).
If you stick with 400, please ensure the controllers convert it to the intended client-side meaning for SDKs..env (2)
25-25: Place_APP_CONSOLE_DOMAINabove whitelist keys to satisfy linter & avoid surprises
dotenv-linterflags the key order. More importantly, keeping domain-defining variables together improves readability and reduces risk of accidentally duplicating or missing a value.-_APP_CONSOLE_DOMAIN=localhost _APP_CONSOLE_HOSTNAMES=localhost,appwrite.io,*.appwrite.io +_APP_CONSOLE_DOMAIN=localhost
88-90: Validate host/path & fix ordering of new runtime keys
_APP_BROWSER_HOSTcurrently contains a full path (...:3000/v1). Most Appwrite components expect the base origin only (e.g.http://appwrite-browser:3000). Embedding/v1may break URL assembly in the console & SDKs – please double-check.- Ordering: move these keys upwards to comply with
dotenv-linter(before_APP_COMPRESSION_MIN_SIZE_BYTES,_APP_FUNCTIONS_TIMEOUT,_APP_SITES_TIMEOUTrespectively) if you want a clean lint run.No functional bug, but worth tidying up before shipping.
app/controllers/api/project.php (1)
153-153: Avoid duplicate breakdown blocks and ensure type consistency– The breakdown for
executionsMbSecondsandbuildsMbSecondsis built twice (lines ~150-180 and again ~255-285). Remove the second occurrence to reduce DB reads and keep variable intent clear.– For each
str_replace(..., $resource->getSequence(), …)cast the sequence to string to avoid subtle metric-ID mismatches:$metric = str_replace( ['{resourceType}', '{resourceInternalId}'], [RESOURCE_TYPE_FUNCTIONS, (string)$function->getSequence()], METRIC_RESOURCE_TYPE_ID_EXECUTIONS );Also applies to: 169-169, 185-185, 201-201, 217-217, 234-234, 241-241, 258-258, 274-274
app/controllers/api/projects.php (1)
1261-1264: Long constant list – hard to maintainThe
typewhitelist enumerates 14Platform::TYPE_*constants inline.
Adding a new platform now requires touching every controller.Expose a helper in
Appwrite\Network\Platform, e.g.:public static function allowedTypes(): array { return [self::TYPE_WEB, …]; }and use:
new WhiteList(Platform::allowedTypes(), true)app/controllers/api/migrations.php (1)
363-366: Path construction loses the leading slash
$deviceForImports->getPath()already returns an absolute path.
Prepending your own migration-specific filename without a separator keeps the path flat (good) but makes Windows path handling rely on implementation details (\vs/).
ConsiderDIRECTORY_SEPARATORorjoin()for portability.app/config/platforms.php (1)
234-239: New CLI keys need loader support
repoBranchandexclude.servicesare new. Ensure:
- The build pipeline reading
platforms.phpignores unknown keys, or- The parser is updated accordingly.
Otherwise JSON-encoding this config for the docs site may explode.
app/realtime.php (2)
82-85: Cache keyed by sequence – OK, but watch collisions
$project->getSequence()is unique only per table cluster; with shared tables off that’s fine, with shared tables on it’s global.
If you ever shard projects across DB hosts the same sequence could appear twice and return the wrong connection.Storing as
"{$dsn->getHost()}:{$sequence}"avoids the edge-case.
560-563: Origin check skips empty originsGood tightening, but note Safari iOS sometimes omits the header for localhost.
If mobile dev-tools start failing, you may need an explicit allow-list fornullorigins in dev mode.app/controllers/mock.php (2)
162-162: Consider updating the field name for consistency.While the method has been changed from
getInternalId()togetSequence(), the field nameprojectInternalIdstill references "internal ID". This inconsistency could lead to confusion during maintenance.Consider renaming the field to
projectSequenceorprojectSequenceIdto match the new data source:- 'projectInternalId' => $project->getSequence(), + 'projectSequence' => $project->getSequence(),Note: This would require updating all references to this field throughout the codebase.
212-212: Variable name doesn't match its content.The variable
$projectInternalIdnow stores a sequence value instead of an internal ID, which is misleading.Rename the variable to accurately reflect its content:
- $projectInternalId = $project->getSequence(); + $projectSequence = $project->getSequence();Also update its usage on line 227:
- 'projectInternalId' => $projectInternalId, + 'projectInternalId' => $projectSequence,app/controllers/shared/api.php (2)
567-567: Track this temporary cache TTL increase.The comment indicates this is a temporary change, but there's no indication of when it should be reverted or what condition needs to be met.
This temporary change could become permanent if not tracked properly. Would you like me to create an issue to track reverting this TTL back to 30 days once the underlying issue is resolved?
800-806: Consider making database event logging configurable.While this logging is valuable for debugging, it could become verbose in production environments with high database activity.
Consider adding a configuration flag to control this logging:
if (!empty($queueForDatabase->getType())) { - Console::info("Triggering database event: \n" . \json_encode([ - 'projectId' => $project->getId(), - 'databaseId' => $queueForDatabase->getDatabase()?->getId(), - 'collectionId' => $queueForDatabase->getCollection()?->getId(), - 'documentId' => $queueForDatabase->getDocument()?->getId(), - ])); + if (System::getEnv('_APP_LOGGING_DATABASE_EVENTS', 'disabled') === 'enabled') { + Console::info("Triggering database event: \n" . \json_encode([ + 'projectId' => $project->getId(), + 'databaseId' => $queueForDatabase->getDatabase()?->getId(), + 'collectionId' => $queueForDatabase->getCollection()?->getId(), + 'documentId' => $queueForDatabase->getDocument()?->getId(), + ])); + } $queueForDatabase->trigger(); }app/controllers/api/teams.php (1)
1091-1108: Good security enhancement to prevent orphaned organizations.The logic correctly prevents the last owner from removing their owner role. Consider adding a comment explaining why we limit the count to 2 for better code clarity.
// Quick check: fetch up to 2 owners to determine if only one exists + // We only need to know if there's 1 or more than 1 owner, so limit to 2 for efficiency $ownersCount = $dbForProject->count(app/init/resources.php (1)
83-91: Consider documenting the purpose of Redis stub resources.The
publisherRedisandconsumerRedisresources are defined as empty stubs. While the AI summary mentions these support fallback mechanisms in health check controllers, their purpose and future implementation plans should be documented.Add a comment explaining the purpose:
+// Stub resources for Redis-based publisher/consumer fallback support +// Used by health check controllers when primary queue brokers are unavailable App::setResource('publisherRedis', function () { // Stub });app/controllers/api/vcs.php (1)
298-304: Improved domain generation with hash-based uniqueness.The new domain generation logic properly handles long branch names and ensures uniqueness through hashing. The 16-character truncation with hash suffix is a good balance between readability and uniqueness.
Consider adding a comment explaining the domain format:
// VCS branch preview if (!empty($providerBranch)) { + // Domain format: branch-{first16chars}[-{hash7}]-{resourceProjectHash}.{domain} + // Hash suffix only added if branch name exceeds 16 characters $branchPrefix = substr($providerBranch, 0, 16);Also applies to: 335-335
app/config/locale/translations/ne.json (1)
24-24: Minor wording tweak for natural Nepali“रिसेट पासवर्ड” is an English-first phrasing. For a smoother native tone you can invert the words and add the verb:
-"emails.recovery.buttonText": "रिसेट पासवर्ड", +"emails.recovery.buttonText": "पासवर्ड रिसेट गर्नुहोस्",app/config/locale/translations/nb.json (1)
24-24: Align wording with existing Norwegian stringsOther recovery strings use “Nullstille”. To stay consistent, consider:
-"emails.recovery.buttonText": "Tilbakestill passord", +"emails.recovery.buttonText": "Nullstill passord",
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (39)
app/config/specs/open-api3-1.7.x-client.jsonis excluded by!app/config/specs/**app/config/specs/open-api3-1.7.x-console.jsonis excluded by!app/config/specs/**app/config/specs/open-api3-1.7.x-server.jsonis excluded by!app/config/specs/**app/config/specs/open-api3-1.8.x-client.jsonis excluded by!app/config/specs/**app/config/specs/open-api3-1.8.x-console.jsonis excluded by!app/config/specs/**app/config/specs/open-api3-1.8.x-server.jsonis excluded by!app/config/specs/**app/config/specs/open-api3-latest-client.jsonis excluded by!app/config/specs/**app/config/specs/open-api3-latest-console.jsonis excluded by!app/config/specs/**app/config/specs/open-api3-latest-server.jsonis excluded by!app/config/specs/**app/config/specs/swagger2-1.7.x-client.jsonis excluded by!app/config/specs/**app/config/specs/swagger2-1.7.x-console.jsonis excluded by!app/config/specs/**app/config/specs/swagger2-1.7.x-server.jsonis excluded by!app/config/specs/**app/config/specs/swagger2-1.8.x-client.jsonis excluded by!app/config/specs/**app/config/specs/swagger2-1.8.x-console.jsonis excluded by!app/config/specs/**app/config/specs/swagger2-1.8.x-server.jsonis excluded by!app/config/specs/**app/config/specs/swagger2-latest-client.jsonis excluded by!app/config/specs/**app/config/specs/swagger2-latest-console.jsonis excluded by!app/config/specs/**app/config/specs/swagger2-latest-server.jsonis excluded by!app/config/specs/**composer.lockis excluded by!**/*.lockdocs/examples/1.7.x/console-cli/examples/databases/upsert-document.mdis excluded by!docs/examples/**docs/examples/1.7.x/console-cli/examples/databases/upsert-documents.mdis excluded by!docs/examples/**docs/examples/1.7.x/console-cli/examples/proxy/create-redirect-rule.mdis excluded by!docs/examples/**docs/examples/1.7.x/console-cli/examples/vcs/get-repository-contents.mdis excluded by!docs/examples/**docs/examples/1.7.x/server-dart/examples/databases/upsert-document.mdis excluded by!docs/examples/**docs/examples/1.7.x/server-dart/examples/databases/upsert-documents.mdis excluded by!docs/examples/**docs/examples/1.7.x/server-dotnet/examples/databases/upsert-document.mdis excluded by!docs/examples/**docs/examples/1.7.x/server-dotnet/examples/databases/upsert-documents.mdis excluded by!docs/examples/**docs/examples/1.7.x/server-nodejs/examples/databases/upsert-document.mdis excluded by!docs/examples/**docs/examples/1.7.x/server-nodejs/examples/databases/upsert-documents.mdis excluded by!docs/examples/**docs/references/databases/decrement-document-attribute.mdis excluded by!docs/references/**docs/references/databases/increment-document-attribute.mdis excluded by!docs/references/**docs/references/databases/update-documents.mdis excluded by!docs/references/**docs/references/vcs/get-repository-contents.mdis excluded by!docs/references/**docs/sdks/cli/CHANGELOG.mdis excluded by!docs/sdks/**docs/sdks/dart/CHANGELOG.mdis excluded by!docs/sdks/**docs/sdks/dotnet/CHANGELOG.mdis excluded by!docs/sdks/**docs/sdks/flutter/CHANGELOG.mdis excluded by!docs/sdks/**docs/sdks/nodejs/CHANGELOG.mdis excluded by!docs/sdks/**docs/sdks/react-native/CHANGELOG.mdis excluded by!docs/sdks/**
📒 Files selected for processing (107)
.coderabbit.yaml(1 hunks).env(2 hunks).github/workflows/benchmark.yml(2 hunks).github/workflows/sdk-preview.yml(1 hunks).github/workflows/tests.yml(4 hunks)CONTRIBUTING.md(2 hunks)README-CN.md(3 hunks)README.md(3 hunks)app/cli.php(6 hunks)app/config/collections/common.php(0 hunks)app/config/collections/projects.php(1 hunks)app/config/console.php(2 hunks)app/config/errors.php(1 hunks)app/config/locale/templates/email-inner-base.tpl(1 hunks)app/config/locale/translations/af.json(2 hunks)app/config/locale/translations/ar-ma.json(2 hunks)app/config/locale/translations/ar.json(2 hunks)app/config/locale/translations/as.json(2 hunks)app/config/locale/translations/az.json(2 hunks)app/config/locale/translations/be.json(2 hunks)app/config/locale/translations/bh.json(2 hunks)app/config/locale/translations/bn.json(2 hunks)app/config/locale/translations/ca.json(2 hunks)app/config/locale/translations/da.json(2 hunks)app/config/locale/translations/de.json(2 hunks)app/config/locale/translations/el.json(2 hunks)app/config/locale/translations/en.json(2 hunks)app/config/locale/translations/eo.json(2 hunks)app/config/locale/translations/es.json(2 hunks)app/config/locale/translations/fa.json(2 hunks)app/config/locale/translations/fr.json(2 hunks)app/config/locale/translations/ga.json(2 hunks)app/config/locale/translations/gu.json(2 hunks)app/config/locale/translations/he.json(2 hunks)app/config/locale/translations/hi.json(2 hunks)app/config/locale/translations/hr.json(2 hunks)app/config/locale/translations/hu.json(2 hunks)app/config/locale/translations/id.json(2 hunks)app/config/locale/translations/it.json(2 hunks)app/config/locale/translations/ja.json(2 hunks)app/config/locale/translations/jv.json(2 hunks)app/config/locale/translations/kn.json(2 hunks)app/config/locale/translations/ko.json(2 hunks)app/config/locale/translations/la.json(2 hunks)app/config/locale/translations/lb.json(2 hunks)app/config/locale/translations/lt.json(2 hunks)app/config/locale/translations/lv.json(2 hunks)app/config/locale/translations/ml.json(2 hunks)app/config/locale/translations/mr.json(2 hunks)app/config/locale/translations/ms.json(2 hunks)app/config/locale/translations/nb.json(2 hunks)app/config/locale/translations/ne.json(2 hunks)app/config/locale/translations/nl.json(2 hunks)app/config/locale/translations/nn.json(2 hunks)app/config/locale/translations/or.json(2 hunks)app/config/locale/translations/pl.json(2 hunks)app/config/locale/translations/pt-br.json(2 hunks)app/config/locale/translations/pt-pt.json(2 hunks)app/config/locale/translations/ro.json(2 hunks)app/config/locale/translations/ru.json(2 hunks)app/config/locale/translations/sa.json(2 hunks)app/config/locale/translations/sd.json(2 hunks)app/config/locale/translations/si.json(2 hunks)app/config/locale/translations/sk.json(2 hunks)app/config/locale/translations/sn.json(2 hunks)app/config/locale/translations/sv.json(2 hunks)app/config/locale/translations/ta.json(2 hunks)app/config/locale/translations/te.json(2 hunks)app/config/locale/translations/th.json(2 hunks)app/config/locale/translations/tl.json(2 hunks)app/config/locale/translations/tr.json(2 hunks)app/config/locale/translations/uk.json(2 hunks)app/config/locale/translations/ur.json(2 hunks)app/config/locale/translations/vi.json(2 hunks)app/config/locale/translations/zh-cn.json(2 hunks)app/config/locale/translations/zh-tw.json(2 hunks)app/config/platforms.php(10 hunks)app/config/storage/inputs.php(1 hunks)app/config/storage/outputs.php(1 hunks)app/config/storage/resource_limits.php(1 hunks)app/config/template-runtimes.php(2 hunks)app/config/templates/site.php(1 hunks)app/controllers/api/account.php(54 hunks)app/controllers/api/avatars.php(4 hunks)app/controllers/api/databases.php(92 hunks)app/controllers/api/health.php(3 hunks)app/controllers/api/messaging.php(12 hunks)app/controllers/api/migrations.php(3 hunks)app/controllers/api/project.php(8 hunks)app/controllers/api/projects.php(22 hunks)app/controllers/api/proxy.php(4 hunks)app/controllers/api/storage.php(36 hunks)app/controllers/api/teams.php(14 hunks)app/controllers/api/users.php(15 hunks)app/controllers/api/vcs.php(22 hunks)app/controllers/general.php(26 hunks)app/controllers/mock.php(2 hunks)app/controllers/shared/api.php(7 hunks)app/http.php(2 hunks)app/init/configs.php(1 hunks)app/init/constants.php(1 hunks)app/init/database/filters.php(14 hunks)app/init/registers.php(1 hunks)app/init/resources.php(18 hunks)app/realtime.php(4 hunks)app/views/general/404.phtml(0 hunks)app/views/general/error.phtml(1 hunks)
⛔ Files not processed due to max files limit (1)
- app/views/install/compose.phtml
💤 Files with no reviewable changes (2)
- app/views/general/404.phtml
- app/config/collections/common.php
🧰 Additional context used
🧠 Learnings (17)
README.md (1)
Learnt from: stnguyen90
PR: #10119
File: app/controllers/api/account.php:1226-1232
Timestamp: 2025-07-08T01:20:14.364Z
Learning: In Appwrite, _APP_DOMAIN is a required environment variable that must always be set for the system to function properly.
README-CN.md (1)
Learnt from: stnguyen90
PR: #10119
File: app/controllers/api/account.php:1226-1232
Timestamp: 2025-07-08T01:20:14.364Z
Learning: In Appwrite, _APP_DOMAIN is a required environment variable that must always be set for the system to function properly.
app/config/console.php (1)
Learnt from: stnguyen90
PR: #10119
File: app/controllers/api/account.php:1226-1232
Timestamp: 2025-07-08T01:20:14.364Z
Learning: In Appwrite, _APP_DOMAIN is a required environment variable that must always be set for the system to function properly.
app/config/templates/site.php (1)
Learnt from: stnguyen90
PR: #10119
File: app/controllers/api/account.php:1226-1232
Timestamp: 2025-07-08T01:20:14.364Z
Learning: In Appwrite, _APP_DOMAIN is a required environment variable that must always be set for the system to function properly.
app/http.php (1)
Learnt from: ItzNotABug
PR: #9693
File: src/Appwrite/Platform/Modules/Databases/Http/Databases/Tables/Update.php:57-59
Timestamp: 2025-06-19T09:20:03.312Z
Learning: In table-related endpoints (such as src/Appwrite/Platform/Modules/Databases/Http/Databases/Tables/Update.php), parameter descriptions should use "table" and "row" terminology instead of "collection" and "document" for clarity and consistency.
app/controllers/api/proxy.php (1)
Learnt from: ItzNotABug
PR: #9693
File: src/Appwrite/Platform/Modules/Databases/Http/Databases/Collections/Indexes/Delete.php:85-86
Timestamp: 2025-05-10T04:12:13.037Z
Learning: The class Appwrite\Platform\Modules\Databases\Http\Databases\Collections\Indexes\Action defines a method getGrantParentNotFoundException() which is used to handle cases where a parent resource (like a database collection) is not found.
app/controllers/api/migrations.php (3)
Learnt from: ItzNotABug
PR: #9661
File: docs/references/migrations/migration-csv.md:1-1
Timestamp: 2025-04-17T08:08:59.449Z
Learning: Documentation files in docs/references/migrations/ follow a minimalist pattern with 1-2 sentences describing the endpoint's functionality, without including detailed API specifications like HTTP methods, parameters, headers, or examples.
Learnt from: ItzNotABug
PR: #9693
File: src/Appwrite/Platform/Modules/Databases/Http/Databases/Tables/Update.php:57-59
Timestamp: 2025-06-19T09:20:03.312Z
Learning: In table-related endpoints (such as src/Appwrite/Platform/Modules/Databases/Http/Databases/Tables/Update.php), parameter descriptions should use "table" and "row" terminology instead of "collection" and "document" for clarity and consistency.
Learnt from: ItzNotABug
PR: #9661
File: app/controllers/api/migrations.php:355-360
Timestamp: 2025-04-17T08:07:25.194Z
Learning: When accessing files after a transfer between devices in Appwrite, ItzNotABug has indicated that using the original path on the destination device for operations like getFileSize() is acceptable in their implementation, as both the original and transferred files are considered equivalent.
.env (1)
Learnt from: stnguyen90
PR: #10119
File: app/controllers/api/account.php:1226-1232
Timestamp: 2025-07-08T01:20:14.364Z
Learning: In Appwrite, _APP_DOMAIN is a required environment variable that must always be set for the system to function properly.
app/init/database/filters.php (2)
Learnt from: ItzNotABug
PR: #9693
File: src/Appwrite/Platform/Modules/Databases/Http/Databases/Tables/Update.php:57-59
Timestamp: 2025-06-19T09:20:03.312Z
Learning: In table-related endpoints (such as src/Appwrite/Platform/Modules/Databases/Http/Databases/Tables/Update.php), parameter descriptions should use "table" and "row" terminology instead of "collection" and "document" for clarity and consistency.
Learnt from: ItzNotABug
PR: #9669
File: src/Appwrite/Utopia/Request/Filters/V19.php:56-71
Timestamp: 2025-05-03T05:58:34.294Z
Learning: According to the developer, wildcard select handling in V19 request filter is only needed for listDocuments operations, not for getDocument operations, despite the doc-block mentioning both.
app/controllers/api/projects.php (1)
Learnt from: ItzNotABug
PR: #9693
File: src/Appwrite/Platform/Modules/Databases/Http/Databases/Tables/Update.php:57-59
Timestamp: 2025-06-19T09:20:03.312Z
Learning: In table-related endpoints (such as src/Appwrite/Platform/Modules/Databases/Http/Databases/Tables/Update.php), parameter descriptions should use "table" and "row" terminology instead of "collection" and "document" for clarity and consistency.
app/controllers/shared/api.php (1)
Learnt from: ItzNotABug
PR: #9693
File: src/Appwrite/Platform/Modules/Databases/Http/Databases/Collections/Delete.php:88-95
Timestamp: 2025-05-09T04:06:04.338Z
Learning: It's acceptable to use DATABASE_TYPE_DELETE_COLLECTION event type for both collections and tables because workers process these events based on whether setCollection() or setTable() was called, not on the event type itself.
.github/workflows/tests.yml (1)
Learnt from: stnguyen90
PR: #10119
File: app/controllers/api/account.php:1226-1232
Timestamp: 2025-07-08T01:20:14.364Z
Learning: In Appwrite, _APP_DOMAIN is a required environment variable that must always be set for the system to function properly.
app/controllers/api/account.php (2)
Learnt from: ItzNotABug
PR: #9673
File: app/controllers/api/migrations.php:364-373
Timestamp: 2025-04-21T06:46:05.356Z
Learning: In the Appwrite codebase, OpenSSL parameters (key, IV, tag) are expected to be present and valid. The absence of these parameters or changes to them after file upload are considered deployment/configuration errors outside the scope of the application code to handle with explicit validation.
Learnt from: abnegate
PR: #9743
File: src/Appwrite/Utopia/Request.php:0-0
Timestamp: 2025-05-12T06:07:51.470Z
Learning: In Appwrite's request handling architecture, the Request::getParams() method selects which SDK method to use based on parameter matching, but does not validate required parameters. The action execution flow handles parameter validation separately. This separation of concerns allows for more flexible handling of multi-method routes.
app/controllers/api/storage.php (3)
Learnt from: ItzNotABug
PR: #9661
File: app/controllers/api/migrations.php:355-360
Timestamp: 2025-04-17T08:07:25.194Z
Learning: When accessing files after a transfer between devices in Appwrite, ItzNotABug has indicated that using the original path on the destination device for operations like getFileSize() is acceptable in their implementation, as both the original and transferred files are considered equivalent.
Learnt from: ItzNotABug
PR: #9661
File: app/controllers/api/migrations.php:332-337
Timestamp: 2025-04-17T08:07:54.565Z
Learning: In the Appwrite codebase, error handling intentionally uses the same error codes for different failure conditions (like STORAGE_BUCKET_NOT_FOUND for both non-existent buckets and authorization failures) as a security measure to prevent information disclosure about what specifically went wrong.
Learnt from: ItzNotABug
PR: #9673
File: app/controllers/api/migrations.php:360-386
Timestamp: 2025-04-21T06:43:39.808Z
Learning: In the Appwrite codebase, only files under 20MB are either compressed or encrypted or both, which means loading these files entirely into memory is acceptable from a resource usage perspective.
app/controllers/api/vcs.php (1)
Learnt from: ItzNotABug
PR: #9752
File: app/config/collections/platform.php:1423-1423
Timestamp: 2025-05-13T12:02:08.071Z
Learning: In the Appwrite database schema, field names like 'resourceInternalId'/'resourceType' and 'deploymentInternalId'/'deploymentResourceType' are functionally equivalent and can coexist in the codebase without causing errors. The docker build and stack work fine with these naming inconsistencies.
app/controllers/general.php (2)
Learnt from: abnegate
PR: #9743
File: src/Appwrite/Utopia/Request.php:33-36
Timestamp: 2025-05-12T06:05:27.133Z
Learning: In Appwrite's architecture, the Request class should only filter parameters when filters are explicitly registered. If no filters are present, all parameters should be passed through, and the route action is responsible for validating which parameters are present/valid.
Learnt from: abnegate
PR: #9743
File: src/Appwrite/Utopia/Request.php:0-0
Timestamp: 2025-05-12T06:07:51.470Z
Learning: In Appwrite's request handling architecture, the Request::getParams() method selects which SDK method to use based on parameter matching, but does not validate required parameters. The action execution flow handles parameter validation separately. This separation of concerns allows for more flexible handling of multi-method routes.
app/controllers/api/databases.php (4)
Learnt from: ItzNotABug
PR: #9693
File: src/Appwrite/Platform/Modules/Databases/Http/Databases/Tables/Update.php:57-59
Timestamp: 2025-06-19T09:20:03.312Z
Learning: In table-related endpoints (such as src/Appwrite/Platform/Modules/Databases/Http/Databases/Tables/Update.php), parameter descriptions should use "table" and "row" terminology instead of "collection" and "document" for clarity and consistency.
Learnt from: ItzNotABug
PR: #9693
File: src/Appwrite/Platform/Modules/Databases/Http/Databases/Tables/Indexes/Delete.php:40-40
Timestamp: 2025-06-25T06:25:45.054Z
Learning: In Appwrite's database module, the event label for deleting a table index intentionally uses 'update' (e.g., 'databases.[databaseId].tables.[tableId].indexes.[indexId].update') instead of 'delete', to match legacy behavior as seen in previous controller implementations.
Learnt from: ItzNotABug
PR: #9693
File: src/Appwrite/Platform/Modules/Databases/Http/Databases/Collections/Indexes/Delete.php:85-86
Timestamp: 2025-05-10T04:12:13.037Z
Learning: The class Appwrite\Platform\Modules\Databases\Http\Databases\Collections\Indexes\Action defines a method getGrantParentNotFoundException() which is used to handle cases where a parent resource (like a database collection) is not found.
Learnt from: ItzNotABug
PR: #9693
File: src/Appwrite/Platform/Modules/Databases/Http/Databases/Collections/Delete.php:88-95
Timestamp: 2025-05-09T04:06:04.338Z
Learning: It's acceptable to use DATABASE_TYPE_DELETE_COLLECTION event type for both collections and tables because workers process these events based on whether setCollection() or setTable() was called, not on the event type itself.
🧬 Code Graph Analysis (4)
app/http.php (1)
src/Appwrite/Migration/Migration.php (1)
createCollection(209-239)
app/config/errors.php (1)
src/Appwrite/Extend/Exception.php (1)
Exception(7-408)
app/controllers/api/account.php (7)
src/Appwrite/Network/Validator/Redirect.php (1)
Redirect(7-25)src/Appwrite/URL/URL.php (1)
URL(5-121)src/Appwrite/Utopia/Request.php (1)
Request(13-236)src/Appwrite/Utopia/Response/Model/Document.php (1)
Document(8-97)src/Appwrite/Event/Audit.php (1)
getHostname(137-140)src/Appwrite/Event/Event.php (1)
Event(10-593)src/Appwrite/Network/Validator/Origin.php (1)
isValid(34-65)
app/init/resources.php (4)
src/Appwrite/Network/Platform.php (1)
Platform(5-159)src/Appwrite/Utopia/Request.php (1)
Request(13-236)src/Appwrite/Utopia/Response/Model/Document.php (1)
Document(8-97)src/Appwrite/Network/Validator/Origin.php (2)
Origin(9-130)isValid(34-65)
🪛 dotenv-linter (3.3.0)
.env
[warning] 25-25: [UnorderedKey] The _APP_CONSOLE_DOMAIN key should go before the _APP_CONSOLE_HOSTNAMES key
[warning] 88-88: [UnorderedKey] The _APP_BROWSER_HOST key should go before the _APP_COMPRESSION_MIN_SIZE_BYTES key
[warning] 89-89: [UnorderedKey] The _APP_FUNCTIONS_RUNTIMES key should go before the _APP_FUNCTIONS_TIMEOUT key
[warning] 90-90: [UnorderedKey] The _APP_SITES_RUNTIMES key should go before the _APP_SITES_TIMEOUT key
🪛 PHPMD (2.15.0)
app/controllers/general.php
58-58: Avoid unused parameters such as '$utopia'. (Unused Code Rules)
(UnusedFormalParameter)
🔇 Additional comments (167)
.coderabbit.yaml (2)
8-12:1.8.xinclusion looks correct—double-check for unintended duplication.Good call extending
auto_review.base_branches.
Just confirm you don’t already list1.7.xelsewhere in the file; YAML will not complain but humans may overlook duplicates.
13-17: New review-output flags match the schema—indentation & scope are fine.Keys
high_level_summary,poem,sequence_diagrams,collapse_walkthrough, andchanged_files_summarysit directly underreviews, which is exactly where the schema expects them. No action needed. ✅README.md (1)
84-85: ✅ Docker tag 1.7.4 verified on Docker HubThe
appwrite/appwrite:1.7.4tag was found, so the README bump is safe and no further action is needed.Affected locations:
- README.md: lines 84-85, 96-97, 106-107
README-CN.md (1)
75-76: 同样确认appwrite/appwrite:1.7.4镜像已发布如果镜像尚未推送，新用户执行安装命令会失败。请在合并前确认 1.7.4 标签已存在。
Also applies to: 87-88, 97-98
.github/workflows/benchmark.yml (1)
67-67: Nice touch adding an explicit version check
Printingoha --versionhelps future debugging. 👍app/config/locale/translations/fr.json (1)
11-12: New button text keys look correct– Placeholders are preserved (
{{team}}).
– Apostrophe in “l'adresse” is properly encoded.
No further action needed.Also applies to: 24-25, 31-32
app/config/locale/translations/da.json (1)
11-12: Danish button text entries are accurateStrings are clear and placeholders intact. Good job keeping terminology consistent with existing translations.
Also applies to: 24-25, 31-32
app/config/locale/translations/mr.json (1)
11-31: Button-text placeholders look correct – no action requiredThe three new keys are added with appropriate Marathi strings, and the
{{team}}placeholder is preserved verbatim (case-sensitive) which is critical for later variable substitution.
LGTM.app/config/locale/translations/pl.json (1)
11-31: Polish button labels correctly addedStrings read naturally and the
{{team}}placeholder is intact.
No issues spotted.app/init/constants.php (1)
40-40: Cache-buster bump OK – ensure coordinated asset purgeIncrementing
APP_CACHE_BUSTERto 4320 is fine; just confirm any CDN or reverse-proxy rules use this value so stale assets are invalidated immediately after deploy.app/config/storage/inputs.php (1)
10-10: GIF input mapping looks good – matches existing pattern and keeps list alphabetised.app/config/storage/outputs.php (1)
11-11: Consistent GIF support – mirrors inputs.php; no issues.app/config/template-runtimes.php (1)
17-17: Verify runtime availability before mergeAdding versions is harmless config-wise, but CI/build images for
dart:3.8andflutter:3.32must already exist; otherwise deployments will fail. Please confirm and update docs / template selection tests.Also applies to: 41-41
app/views/general/error.phtml (1)
18-18: Confirmed: Console domain fallback is consistent across the codebase.All console-URL constructions now use
System::getEnv('_APP_CONSOLE_DOMAIN', System::getEnv('_APP_DOMAIN', ''))
and there are no remaining instances where only_APP_DOMAINis used for console routes. Key files checked:• docker-compose.yml
• src/Appwrite/Vcs/Comment.php
• src/Appwrite/Platform/Modules/Functions/Workers/Builds.php
• app/views/general/error.phtml
• app/controllers/general.php
• app/controllers/api/vcs.php
• app/controllers/api/account.phpNo further action required.
app/config/storage/resource_limits.php (1)
1-6: Clean implementation with proper resource management.The configuration correctly sets memory limits for image processing with a sensible default of 64MB. The use of environment variables allows for flexible deployment configuration.
.github/workflows/sdk-preview.yml (1)
50-50: Confirm console-web SDK workflow scopeNo other workflow in .github/workflows generates SDKs—sdk-preview.yml is the sole pipeline invoking
sdks --platform…. The hardcoded working-directory./app/sdks/console-webaligns with its exclusive focus on the console-web SDK preview. No further changes needed.app/config/console.php (1)
8-8: Approve migration: Platform::TYPE_WEB replaces inline origin values
- Verified
Platform::TYPE_WEBis defined as'web'insrc/Appwrite/Network/Platform.php- No
TYPE_WEBconstant existed in the oldOriginvalidator, so this change consolidates all platform identifiers into the newPlatformclass- Import and key rename (
$internalId→$sequence) follow the broader refactoring pattern and look correctapp/controllers/api/avatars.php (1)
672-672: Sequence-based tiering: confirm zero-sequence semanticsThe platinum / variation logic now keys off
$user->getSequence().
$sequence % 100 === 0and% 3mean that a user whose sequence happens to be0will be treated as platinum and pick variation1. If0is a valid value in your seeding logic, verify that this is intended; otherwise guard with> 0or start counting from1.No functional issue beyond that, just a sanity check.
Also applies to: 861-863, 961-962
app/config/locale/translations/ml.json (1)
11-11: New button-text keys LGTMKeys and placeholders follow existing localisation style.
Also applies to: 24-24, 31-31
app/config/locale/translations/bh.json (1)
11-11: Translation additions look goodStrings added match the new email-template variable.
Also applies to: 24-24, 31-31
app/config/locale/translations/ja.json (1)
11-11: Button text translations acceptedNo issues spotted with placeholder usage or wording.
Also applies to: 24-24, 31-31
app/config/locale/translations/sk.json (1)
11-11: Slovak button-text keys look correctStrings integrate cleanly with existing localisation set.
Also applies to: 24-24, 31-31
app/config/locale/translations/pt-br.json (1)
11-11: New button-text keys look goodKeys are present, JSON syntax is intact, and placeholders (
{{team}}) are preserved.
No further action required.Also applies to: 24-24, 31-31
app/config/locale/translations/hr.json (1)
11-11: Button-text additions are accurate and consistentTranslations are natural, placeholders remain untouched, and JSON stays valid.
Looks good to merge.Also applies to: 24-24, 31-31
app/config/locale/translations/ro.json (1)
11-11: Romanian labels read wellNo placeholder or grammatical issues spotted.
All good.Also applies to: 24-24, 31-31
app/config/locale/translations/ar-ma.json (1)
11-31: Localization keys added – looks goodTranslations render well and placeholders are kept intact. No further action needed.
app/config/locale/translations/zh-cn.json (1)
11-31: Chinese button text looks correctWording and placeholder usage are fine.
CONTRIBUTING.md (2)
418-420: Docs now mix…InternalId&…SequenceAfter switching examples to
getSequence(), audit the surrounding metric-name table (lines 346-360) to avoid inconsistent terminology.
466-468: Double-check metric key examplesConfirm that
{functionInternalId}placeholders should stay or be renamed to{functionSequence}for full consistency.app/init/database/filters.php (1)
74-77: Verify data migration to sequences
attributesfilter now queriescollectionInternalIdwith a sequence value. Ensure existing documents were back-filled; otherwise sub-queries will return empty results.app/controllers/api/messaging.php (1)
2600-2601: Missing index may cause full-collection scan
Query::equal('topicInternalId', [$topic->getSequence()]);looks fine logically, but check that thesubscriberscollection has an index ontopicInternalId.
Without it, paging through large subscriber sets will degrade linearly.app/config/locale/translations/fa.json (1)
11-12: New button-text keys added correctlyKeys, placeholders and trailing commas all look good, matching the template’s
{{buttonText}}usage.
No action required.Also applies to: 24-25, 31-32
app/config/locale/translations/sv.json (1)
11-12: Translations & placeholders look consistentAll three added keys are properly localised, include the expected
{{team}}placeholder, and preserve valid JSON syntax.
LGTM.Also applies to: 24-25, 31-32
app/config/locale/translations/vi.json (1)
11-12: Vietnamese strings added without issuesStrings are idiomatic, placeholders preserved, commas present. Ready to ship.
Also applies to: 24-25, 31-32
app/config/locale/translations/zh-tw.json (1)
11-12: Traditional-Chinese additions are accurateTranslations, placeholders and punctuation look correct; no further changes needed.
Also applies to: 24-25, 31-32
app/config/locale/translations/th.json (1)
11-11: Translations look correct and placeholders are preservedAll three newly-added button texts read naturally in Thai and correctly keep (or omit) placeholders where expected (e.g.
{{team}}).
No further action required.Also applies to: 24-24, 31-31
app/config/locale/translations/az.json (1)
11-11: Solid additions – wording + placeholder usage are on pointThe Azerbaijani strings are idiomatic and the
{{team}}placeholder is retained. No issues spotted.Also applies to: 24-24, 31-31
app/config/locale/translations/te.json (1)
11-11: Good Telugu phrasing and correct interpolationButton captions are fluent, and the invitation string starts with the
{{team}}placeholder exactly as required. Looks good.Also applies to: 24-24, 31-31
app/config/locale/translations/de.json (1)
11-11: Clean German wording & placeholder retentionThe three added keys read well and keep
{{team}}where needed. Implementation is fine.Also applies to: 24-24, 31-31
app/config/locale/translations/ca.json (1)
11-11: Catalan button labels look goodWording is natural and
{{team}}placeholder remains intact. No issues with the new lines.Also applies to: 24-24, 31-31
app/config/locale/translations/it.json (1)
11-11: LGTM – new button labels integrate correctlyKeys follow existing naming & placeholder conventions; no issues spotted.
Also applies to: 24-24, 31-31
app/config/locale/translations/es.json (1)
11-11: Translations look correctStrings read naturally and preserve
{{team}}placeholder where needed.Also applies to: 24-24, 31-31
app/config/locale/translations/ar.json (1)
11-11: Arabic additions are consistentTerminology matches existing style; placeholders untouched. No further action.
Also applies to: 24-24, 31-31
app/config/locale/translations/ga.json (1)
11-11: Irish translations approvedKeys and content follow current patterns; everything parses as valid JSON.
Also applies to: 24-24, 31-31
app/config/locale/translations/sa.json (3)
11-11: Translation key added correctlyKey and placeholder usage look consistent with other locales. No further action required.
24-24: Translation key added correctlySame as above – nothing to flag.
31-31: Translation key added correctlyPlaceholder
{{team}}retained – LGTM.app/config/locale/translations/ko.json (3)
11-11: Consistent button-text keyKey/value pair added in line with existing conventions. ✅
24-24: Consistent button-text keyNothing to change.
31-31: Consistent button-text keyPlaceholder preserved, no issues.
app/config/locale/translations/el.json (3)
11-11: Greek button text key OKKey and value added; matches pattern used elsewhere.
24-24: Greek recovery button text key OKLooks good.
31-31: Invitation button text key OK
{{team}}placeholder retained – no concerns.app/config/locale/translations/ta.json (3)
11-11: Tamil verification button text key OKConsistent with other locales.
24-24: Tamil recovery button text key OKNo issues.
31-31: Tamil invitation button text key OKPlaceholder
{{team}}intact – looks fine.app/config/locale/translations/ur.json (1)
11-11: Button-text keys look correctStrings are properly localized, placeholders (
{{team}},{{project}}) preserved, and JSON syntax fixed. No action needed.Also applies to: 24-24, 31-32
app/config/locale/translations/en.json (1)
11-11: Translations added correctlyKeys and placeholders are accurate; no further action required.
Also applies to: 49-49, 56-56
app/config/locale/translations/sn.json (1)
11-11: Button-text keys look goodPlaceholders maintained and JSON remains valid. 👍
Also applies to: 24-24, 31-31
app/config/locale/translations/tr.json (1)
11-11: Button-text keys added correctly – looks goodThe three new entries follow existing key naming, preserve placeholders, and respect Turkish grammar. JSON structure remains valid. No further action needed.
Also applies to: 24-24, 31-31
app/config/locale/translations/as.json (1)
11-11: Assamese button-text translations added without issuesKey names, placeholder usage (
{{team}}) and tone are consistent with the rest of the file. Syntax OK.Also applies to: 24-24, 31-31
app/config/locale/translations/lv.json (1)
11-11: Latvian additions are consistentEntries are accurately translated, placeholders unchanged, and JSON remains well-formed.
Also applies to: 24-24, 31-31
app/config/locale/translations/nl.json (1)
11-11: Dutch button-text keys look correctTranslations are imperative, retain
{{team}}placeholder, and conform to existing style. No concerns.Also applies to: 24-24, 31-31
app/config/locale/translations/id.json (1)
11-11: Indonesian button-text keys added successfullyStrings are concise, placeholders intact, and JSON syntax valid. All good.
Also applies to: 24-24, 31-31
app/config/locale/translations/sd.json (3)
11-11: Sindhi translations – looks good.Key is correctly added, placeholder usage is sound.
24-24: Sindhi translations – looks good.Key is correctly added, placeholder usage is sound.
31-31: Sindhi translations – looks good.Key is correctly added, placeholder usage is sound.
app/config/locale/translations/hi.json (3)
11-11: Hindi translations – looks good.Correct placeholder-free text; matches template expectations.
24-24: Hindi translations – looks good.Accurate wording and no placeholder errors.
31-31: Hindi translations – looks good.
{{team}}placeholder retained; string reads naturally.app/config/locale/translations/eo.json (3)
10-10: Esperanto translations – looks good.Correct placeholder usage and natural wording.
23-23: Esperanto translations – looks good.No issues detected.
30-30: Esperanto translations – looks good.Placeholder
{{team}}present and well-formed.app/config/locale/translations/si.json (3)
11-11: Sinhala translations – looks good.Key added with correct Sinhala copy.
24-24: Sinhala translations – looks good.No placeholder issues observed.
31-31: Sinhala translations – looks good.
{{team}}placeholder correctly retained.app/config/locale/translations/tl.json (1)
11-31: New button-text keys look consistent – no action required✓ Keys follow the existing naming pattern.
✓ Place-holders ({{team}}) preserved.
✓ Capitalisation matches surrounding strings.app/config/locale/translations/af.json (1)
11-31: Button-text translations added correctlyNothing to flag – the three new keys align with the existing style and keep the
{{team}}placeholder intact.app/config/locale/translations/gu.json (1)
11-31: Addition of button labels is soundAll three Gujarati strings are clear, correctly capitalised, and preserve template variables.
app/config/locale/translations/uk.json (1)
11-31: Ukrainian button-text keys verifiedStrings read naturally and contain the required
{{team}}placeholder.app/config/locale/translations/nn.json (1)
11-11: Translation keys added correctlyThe three new
buttonTextentries are syntactically valid JSON, follow the existing naming convention, include the correct{{team}}placeholder where needed, and don’t introduce trailing-comma issues.
No further action required.Also applies to: 24-24, 31-31
app/config/locale/translations/ru.json (1)
11-11: Button-text additions look goodKeys, placeholders, and comma placement are all consistent with the rest of the RU locale file.
Good to merge.Also applies to: 24-24, 31-31
app/config/locale/translations/be.json (1)
11-11: Belarusian translations added without issuesJSON remains valid, keys match the global pattern, and placeholders are intact.
Nothing else to flag.Also applies to: 24-24, 31-31
app/config/locale/translations/bn.json (1)
11-11: BengalibuttonTextentries verifiedEntries adhere to established structure and include the
{{team}}placeholder where appropriate.
Looks solid.Also applies to: 24-24, 31-31
app/config/locale/translations/lb.json (1)
11-11: Luxembourgish additions are consistentKey names, JSON syntax, and placeholder usage are correct.
No concerns from a code perspective.Also applies to: 24-24, 31-31
app/config/locale/translations/pt-pt.json (1)
11-12: Translations LGTMNew
buttonTextkeys are added with correct Portuguese phrasing and placeholders – no issues spotted.Also applies to: 24-25, 31-32
app/config/locale/translations/or.json (1)
11-11: Button-text translations look correct & placeholders preservedThe three new keys follow the existing naming convention, keep the required
{{team}}placeholder intact, and use the same quote style as the rest of the file. No action needed.Also applies to: 24-24, 31-31
app/config/locale/translations/ms.json (1)
11-11: Malay button-text strings are well-formedKeys, values, and placeholder usage (
{{team}}) are consistent with neighbouring entries and with the email template update. Good to merge.Also applies to: 24-24, 31-31
app/controllers/api/projects.php (2)
185-198: Sequence switch requires data migration & indexing
teamInternalIdis now populated withgetSequence().
All queries below (Query::equal('teamInternalId', …), etc.) will silently miss pre-existing rows created with the formergetInternalId()unless a migration/back-fill runs and proper DB indexes are added for the new column.Please confirm:
- A one-off migration script updates historical documents.
- Secondary indexes exist on
teamInternalIdfor every affected collection.
2271-2278: Locale key must exist forbuttonText
{{buttonText}}is injected unconditionally.
If a locale JSON is missingemails.<type>.buttonText, rendering will throw.
Guard with a default or fallback similar to the other keys.app/controllers/api/migrations.php (1)
348-350: Fallback for legacy buckets?The lookup now assumes every bucket already switched to the new
bucket_<sequence>collection name.
If projects created before the sequence-refactor still hold files inbucket_<internalId>,getDocument()will 404 and the whole import will fail.Consider adding a graceful fallback or a short migration note.
app/config/platforms.php (1)
14-15: Patch version downgrade – deliberate?
webSDK goes from18.1.1→18.1.0.
Downgrades can break consumers pulling the latest patch automatically. Double-check this wasn’t a typo.app/realtime.php (1)
108-115:setTenant((int)$project->getSequence())– zero allowed?If an older project does not yet have a sequence (null/0), passing
0tosetTenant()might leak data between tenants.
Guard with an explicit check or throw until the migration is complete..github/workflows/tests.yml (3)
181-181: Good security practice for test isolation.Setting
_APP_BROWSER_HOSTto an invalid URL prevents tests from making external browser connections, which is a good security practice in CI/CD environments.
202-202: Well-structured test organization.Excluding screenshot tests from the main test runs and moving them to dedicated jobs improves test organization, enables parallel execution, and allows for specific configuration (like keeping the original
_APP_BROWSER_HOSTvalue).Also applies to: 281-281
363-396: Well-implemented screenshot test jobs.The new screenshot test jobs are properly configured with:
- Abuse options enabled for Sites service functionality
- Original
_APP_BROWSER_HOSTpreserved for actual browser connections- Consistent structure with other test jobs
- Support for both dedicated and shared table modes
Also applies to: 397-443
app/controllers/api/health.php (1)
858-871: Excellent improvement to prevent race conditions.Using unique filenames for each health check prevents race conditions when multiple health checks run concurrently. This is a robust solution that improves reliability.
app/controllers/shared/api.php (2)
578-578: Consistent sequence ID migration.All references to bucket and file internal IDs have been properly updated to use sequence IDs, maintaining consistency with the project-wide refactoring.
Also applies to: 596-601
833-856: Good cache management improvements.The changes improve cache efficiency by:
- Reusing the cache instance instead of creating multiple instances
- Explicitly saving the cache file to update modification time for TTL checks
- Adding clear documentation about the behavior
app/controllers/api/storage.php (12)
21-21: LGTM!The import for
Utopia\CLI\Consoleis correctly added and properly grouped with other Utopia imports.
149-149: LGTM!The change from
getInternalId()togetSequence()is consistent with the codebase-wide migration to sequence IDs for collection naming.
319-330: LGTM!The bucket update implementation correctly sets all attributes and uses
getSequence()for the collection identifier, maintaining consistency with the sequence ID migration.
562-562: LGTM!All file operations correctly use
getSequence()for bucket collection references, and thebucketInternalIdattribute is properly added to store the bucket's sequence ID for future reference.Also applies to: 656-656, 675-675, 700-700, 708-708, 724-724, 745-745
830-850: LGTM!File listing operations correctly use
getSequence()for collection references while maintaining proper authorization checks.
906-909: LGTM!File retrieval correctly uses
getSequence()while preserving authorization checks.
957-1109: Performance monitoring enhancement looks great!The preview endpoint improvements include:
- Proper use of
getSequence()throughout- Added detailed timing measurements for download, decryption, decompression, and rendering phases
- Comprehensive logging with project, bucket, file, and URI context
- Using
APP_STORAGE_PREVIEW_LIMITenvironment variable for better configurabilityThe performance monitoring will help identify bottlenecks in the preview generation pipeline.
1162-1230: Excellent improvements to the download endpoint!Key improvements:
- Consistent use of
getSequence()for collection references- Using
APP_STORAGE_READ_BUFFERfor configurable buffer limits- Response headers are now properly set after range validation, preventing header pollution on invalid requests
The header ordering change is particularly good - it ensures headers are only set for valid requests.
1323-1401: LGTM!The view endpoint changes mirror the download endpoint improvements:
- Consistent
getSequence()usage- Configurable buffer limits with
APP_STORAGE_READ_BUFFER- Proper response header ordering after range validation
1503-1553: LGTM!Push notification endpoint follows the same improvement pattern with
getSequence()usage, configurable buffer limits, and proper header ordering.
1668-1668: LGTM!File update and delete operations correctly use
getSequence()for all collection references while maintaining proper authorization checks.Also applies to: 1714-1716, 1782-1782, 1812-1814
1962-1969: LGTM!Bucket usage metrics correctly use
getSequence()for metric identifiers, maintaining consistency with the sequence ID migration.app/cli.php (5)
128-134: LGTM!The database configuration correctly uses
getSequence()with appropriate type casting for tenant (int) and string concatenation for namespace. Both shared and non-shared table configurations are properly updated.Also applies to: 148-154
170-170: LGTM!Logs database configuration correctly uses
getSequence()with proper integer casting for the tenant identifier.Also applies to: 185-185
191-196: LGTM!The new
publisherandpublisherRedisresources are properly implemented. Thepublisherresource correctly returns aBrokerPoolinstance, whilepublisherRedisserves as a stub for the fallback mechanism used in health checks.
256-256: LGTM!The executor resource is correctly simplified to instantiate
Executorwithout parameters, indicating the class no longer requires external configuration.
289-289: Verify the impact of enabling TCP hooksThe change from
SWOOLE_HOOK_ALL ^ SWOOLE_HOOK_TCPtoSWOOLE_HOOK_ALLnow includes TCP hooks in the coroutine runtime. This could affect TCP socket operations and their coroutine behavior.Please ensure this change is intentional and that TCP-related operations have been tested with coroutines enabled.
app/controllers/api/account.php (1)
1193-1199: Standard ports are flagged as “custom” – strict type mismatch
$portcomes fromRequest::getPort()(integer).
Comparing it with the string literals'443'/'80'via strict!==always yieldstrue, so URLs likehttps://example.com:443are produced.-if ($protocol === 'https' && $port !== '443') { +if ($protocol === 'https' && $port !== 443) {or cast once:
$port = (int) $request->getPort();Please fix in every repeated block before this goes live.
Also applies to: 1228-1232, 1269-1276, 1306-1312, 1822-1828, 1856-1861, 2012-2019, 1353-1359
⛔ Skipped due to learnings
Learnt from: stnguyen90 PR: appwrite/appwrite#10119 File: app/controllers/api/account.php:1192-1199 Timestamp: 2025-07-08T01:20:00.913Z Learning: The `getPort()` method from Utopia\Http\Request returns a string, not an integer. When reviewing port comparison logic in PHP code using this library, string comparisons with '443' and '80' are correct.app/controllers/api/teams.php (13)
14-14: LGTM!The replacement of
Hostvalidator withRedirectvalidator aligns with the broader refactoring of validation logic across the codebase.
126-128: LGTM!Consistent replacement of
getInternalId()withgetSequence()for internal ID retrieval.
469-469: LGTM!Good security enhancement - the conditional validator properly restricts redirect URLs to platform-allowed domains when not using an API key, helping prevent open redirect attacks.
598-600: LGTM!Consistent use of
getSequence()in membership queries.
615-617: LGTM!Proper setting of sequence IDs in membership document creation.
846-846: LGTM!Consistent use of team sequence ID in membership queries.
669-669: LGTM!Addition of
{{buttonText}}parameter supports localized button text in email templates.
1187-1189: LGTM!Proper validation to ensure membership belongs to the correct team and user using sequence IDs.
Also applies to: 1204-1206
1236-1236: LGTM!Consistent addition of user sequence ID to session document.
1322-1327: LGTM!Proper injection of required dependencies for the new validation logic.
1351-1373: Good security enhancement consistent with the update endpoint.The logic correctly prevents the last owner from deleting their membership, ensuring organizations always have at least one owner.
1347-1349: LGTM!Consistent use of sequence IDs for validation and queries.
Also applies to: 1358-1358, 1366-1366
1382-1382: LGTM!Proper updates to use profile document and user ID from the fetched profile.
Also applies to: 1390-1390
app/controllers/general.php (12)
58-58: LGTM!Addition of the
Logparameter enables enhanced logging for custom domain function execution.Note: The static analysis hint about unused
$utopiaparameter appears to be a false positive, as$utopiais used throughout the function.
78-78: LGTM!Good configuration improvement - preferring
_APP_CONSOLE_DOMAINallows separate console domain configuration while maintaining backward compatibility.
120-125: LGTM!Good logging enhancement - adding the project ID tag helps track errors when executing custom domain functions where the x-appwrite-project header isn't available.
193-193: LGTM!Consistent incorporation of project region in console URLs supports multi-region deployments.
Also applies to: 331-332, 336-336, 341-341, 353-353
399-401: LGTM!Consistent replacement of internal IDs with sequence IDs in execution document.
699-703: LGTM!Proper use of sequence IDs in metrics tracking.
Also applies to: 726-728
519-538: Good refactoring of runtime entrypoint logic.The changes improve code maintainability by:
- Dynamically detecting source file extension
- Unifying start command assignment with framework adapter overrides
- Simplifying runtime entrypoint construction
812-814: LGTM!Good security improvement - centralizing and sanitizing HTTP referrer handling.
946-946: LGTM!Simplified and improved origin validation using sanitized referrer and merged platforms list.
Also applies to: 1020-1020, 1033-1042
1247-1252: LGTM!Good defensive programming - checking for existing projectId tag before adding to avoid duplication.
794-794: LGTM!Consistent addition of Log parameter across all route handlers and router calls.
Also applies to: 822-822, 1053-1053, 1066-1066, 1074-1074, 1370-1371, 1381-1381, 1389-1389, 1403-1404, 1414-1414, 1422-1422
918-918: LGTM!Consistent use of
getSequence()for console project internal ID.app/controllers/api/databases.php (8)
41-41: LGTM! New imports support the increment/decrement functionality.The addition of
TypeExceptionandNumericvalidator imports are necessary for the new increment/decrement endpoints to properly validate attribute types and numeric values.Also applies to: 66-66
112-112: Consistent migration from internal IDs to sequences.The systematic replacement of
getInternalId()withgetSequence()for database and collection references aligns with the project-wide refactoring for identifier consistency. This change affects document retrieval, ID generation, and cache management appropriately.Also applies to: 135-135, 143-148, 169-176
4466-4541: Well-implemented atomic increment operation.The new increment endpoint provides important benefits:
- Atomic operations prevent race conditions in concurrent scenarios
- Proper validation ensures only numeric attributes can be incremented
- The
maxparameter prevents overflow conditions- Comprehensive error handling covers all edge cases
The implementation correctly handles database lookups, metric tracking, and event queuing.
4543-4618: Complementary decrement operation with proper safeguards.The decrement endpoint mirrors the increment functionality with appropriate modifications:
- Uses
minparameter instead ofmaxto prevent underflow- Consistent error handling and validation approach
- Same comprehensive coverage for metrics and events
Both increment/decrement endpoints together provide a complete solution for atomic counter operations.
3224-3224: Enhanced security for bulk operations.Adding
AuthType::ADMINto bulk document operations (createDocuments, updateDocuments, upsertDocuments, deleteDocuments) is a good security enhancement. These operations already had internal checks for API keys and privileged users, so this change makes the authentication requirements explicit at the route level.Also applies to: 3245-3245, 4635-4635, 4745-4745, 4964-4964
1354-1356: Plan-based feature gating for encrypted attributes.The check for
databasesAllowEncryptin the plan ensures that encrypted string attributes are only available to users on appropriate pricing tiers. This is a standard approach for SaaS feature management.
4756-4756: Correct parameter requirement for upsert operation.Making the
documentsparameter required (removing thetrueflag that made it optional) is correct - an upsert operation without documents would be meaningless.
904-5272: Comprehensive and consistent sequence migration.The remaining changes throughout the file consistently replace
getInternalId()withgetSequence()across all database operations:
- Collection CRUD operations
- Attribute management
- Index operations
- Document operations
- Usage statistics
The migration maintains the same concatenation patterns and functionality while standardizing on the sequence-based identification system. This large-scale refactoring appears to be executed correctly with no inconsistencies detected.
app/init/resources.php (8)
131-174: LGTM! Platform-centric approach improves clarity.The refactoring from
clientstoplatformswith the newPlatformconstants provides better type safety and clearer semantics.
225-271: LGTM! Consistent user fetching across different modes.The updated logic properly handles user retrieval from the appropriate database based on the mode, maintaining consistency with the platform/project separation.
355-361: LGTM! Consistent sequence-based tenant identification.The systematic replacement of
getInternalId()with(int)getSequence()for tenant identification is implemented correctly. The integer cast ensures type safety for tenant IDs.Also applies to: 408-414, 438-453
462-473: LGTM! Clean telemetry integration for cache monitoring.The telemetry integration is well-implemented using dependency injection, allowing for easy monitoring and metrics collection.
496-513: LGTM! Consistent telemetry wrapper for all device types.The telemetry decorator pattern is consistently applied across all device resources, enabling unified monitoring of storage operations.
846-846: LGTM! Consistent sequence-based team queries.The query update aligns with the project-wide migration from internal IDs to sequences.
925-942: LGTM! Proper migration of resource tokens to sequences.The resource token handling correctly migrates to sequence-based identifiers while maintaining the existing token structure and validation logic.
952-999: LGTM! Comprehensive referrer validation with safe fallback.The HTTP referrer resources provide excellent security by validating against multiple sources (public routes, platforms, rules) and safely falling back to localhost. The implementation correctly uses sequences for project matching.
Consider adding logging when falling back to localhost for security monitoring:
// Unsafe; Localhost is always safe for ease of local development +Console::warning("Unsafe referrer '{$httpReferrer}' detected, falling back to localhost"); $origin = 'localhost';app/controllers/api/vcs.php (5)
81-85: LGTM! Consistent sequence usage for resource identification.All internal ID replacements with sequences are properly implemented throughout the deployment creation logic.
Also applies to: 162-162, 278-283
429-430: LGTM! Appropriate validator for redirect URLs.The
Redirectvalidator is the correct choice for validating redirect URLs, providing better security than the genericHostvalidator.
617-617: LGTM! Useful feature for flexible repository browsing.The addition of
providerReferenceparameter enables browsing repository contents at specific Git references (branches, tags, commits), enhancing the API's flexibility.Also applies to: 644-644
1018-1018: LGTM! Consistent sequence-based queries throughout VCS controller.All database queries have been properly updated to use sequences instead of internal IDs, maintaining consistency with the project-wide identifier migration.
Also applies to: 1369-1369, 1442-1442, 1525-1525
1228-1228: Good optimization to skip deployments on branch lifecycle events.The addition of
branchDeletedflag and the logic to skip deployment creation when branches are created or deleted prevents unnecessary builds, improving efficiency.Also applies to: 1250-1252
app/config/locale/translations/ne.json (2)
11-11: Button-text key added correctly
Key name, placeholder‐free value, comma separation, and overall JSON syntax all look good.
31-31: Invitation button-text looks good
Token placement ({{team}}) is preserved and the phrase reads naturally.app/config/locale/translations/nb.json (2)
11-11: Verification button-text added correctly
Key, value, and comma placement are all fine.
31-31: Invitation button-text LGTM
Placeholder order ({{team}}) and wording are correct.
| "emails.recovery.signature": "a {{project}} csapat", | ||
| "emails.invitation.subject": "Meghívó a(z) %s csapatba, a(z) %s projektbe", | ||
| "emails.invitation.hello": "Szia,", | ||
| "emails.invitation.body": "Ezt a levelet azért kaptad, mert {{owner}} meghívott, hogy légy a {{team}} csapat tagja a {{project}} projektben.", | ||
| "emails.invitation.footer": "Ha nem érdekel a lehetőség, nyugodtan hagyd figyelmen kívül ezt az üzenetet.", | ||
| "emails.invitation.thanks": "Köszönettel,", | ||
| "emails.invitation.buttonText": "Elfogadni meghívást a {{team}-re", |
There was a problem hiding this comment.
Placeholder syntax error breaks template rendering.
"Elfogadni meghívást a {{team}-re" has an opening {{ but is missing the closing }}, which will bust Mustache rendering and likely leak “{{team}-re” verbatim to users.
Suggested fix (also makes the wording more idiomatic):
-"emails.invitation.buttonText": "Elfogadni meghívást a {{team}-re",
+"emails.invitation.buttonText": "{{team}} csapat meghívásának elfogadása"Verify with any downstream placeholders before merging.
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In app/config/locale/translations/hu.json at line 31, the placeholder syntax is
incorrect because the opening {{ is not properly closed with }}. Fix this by
changing "{{team}-re" to "{{team}}-re" to correctly close the placeholder and
ensure proper Mustache template rendering. Also, review the wording to make it
more idiomatic as suggested before finalizing.
| $protocol = System::getEnv('_APP_OPTIONS_FORCE_HTTPS') === 'disabled' ? 'http' : 'https'; | ||
| $hostname = System::getEnv('_APP_DOMAIN', ''); |
There was a problem hiding this comment.
💡 Verification agent
❓ Verification inconclusive
Guard against empty _APP_DOMAIN after removing the localhost fallback
If _APP_DOMAIN isn’t set, $hostname becomes an empty string, producing URLs like https:///….
Either fail fast (\RuntimeException) or fall back to _APP_CONSOLE_DOMAIN / 'localhost' for dev environments to avoid broken asset links.
Enforce mandatory _APP_DOMAIN to avoid malformed URLs
Allowing an empty hostname leads to URLs like https:///… when _APP_DOMAIN is unset. Since _APP_DOMAIN is required, we should fail fast or explicitly handle missing values.
• File: app/config/templates/site.php (around lines 9–10)
• Replace the permissive default with a required check
Suggested patch:
- $hostname = System::getEnv('_APP_DOMAIN', '');
+ $hostname = System::getEnv('_APP_DOMAIN');
+ if (empty($hostname)) {
+ throw new \RuntimeException('Required environment variable _APP_DOMAIN is not set');
+ }This ensures misconfigurations are caught early.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| $protocol = System::getEnv('_APP_OPTIONS_FORCE_HTTPS') === 'disabled' ? 'http' : 'https'; | |
| $hostname = System::getEnv('_APP_DOMAIN', ''); | |
| $protocol = System::getEnv('_APP_OPTIONS_FORCE_HTTPS') === 'disabled' ? 'http' : 'https'; | |
| $hostname = System::getEnv('_APP_DOMAIN'); | |
| if (empty($hostname)) { | |
| throw new \RuntimeException('Required environment variable _APP_DOMAIN is not set'); | |
| } |
🤖 Prompt for AI Agents
In app/config/templates/site.php around lines 9 to 10, the code assigns an empty
string as the default for _APP_DOMAIN, which can cause malformed URLs if the
environment variable is missing. To fix this, remove the default empty string
and add a check that throws an error or halts execution if _APP_DOMAIN is not
set, ensuring the hostname is always defined and preventing invalid URLs.
| 'providerInternalId' => !empty($providerId) ? $provider->getSequence() : null, | ||
| 'providerType' => MESSAGE_TYPE_PUSH, | ||
| 'userId' => $user->getId(), | ||
| 'userInternalId' => $user->getInternalId(), | ||
| 'userInternalId' => $user->getSequence(), | ||
| 'sessionId' => $session->getId(), |
There was a problem hiding this comment.
providerInternalId may throw on missing provider
$provider->getSequence() is called when a providerId was supplied, but the preceding lookup can still return an empty document (e.g. wrong ID).
This yields a fatal error instead of a controlled Exception.
Add an existence check:
-'providerInternalId' => !empty($providerId) ? $provider->getSequence() : null,
+'providerInternalId' => (!empty($providerId) && !$provider->isEmpty())
+ ? $provider->getSequence()
+ : null,and consider returning Exception::PROVIDER_NOT_FOUND earlier.
🤖 Prompt for AI Agents
In app/controllers/api/account.php around lines 4682 to 4686, the code calls
$provider->getSequence() if $providerId is not empty, but $provider may be null
if the lookup failed, causing a fatal error. Fix this by adding a check to
confirm $provider is not null before calling getSequence(), and if it is null,
handle the error gracefully by returning Exception::PROVIDER_NOT_FOUND earlier
in the code flow.
| ->setTenant($sharedTablesV1 ? (int)$project->getSequence() : null) | ||
| ->setNamespace($dsn->getParam('namespace')); | ||
| } else { | ||
| $dbForProject | ||
| ->setSharedTables(false) | ||
| ->setTenant(null) | ||
| ->setNamespace('_' . $project->getInternalId()); | ||
| ->setNamespace('_' . $project->getSequence()); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Possible overflow / namespace collision
setTenant((int)$project->getSequence()) works while the sequence fits in 32-bit.
If the sequence ever exceeds INT_MAX (≈2^31-1 on many MySQL builds) it will truncate or throw.
Consider keeping it as string everywhere (already the case for setNamespace('_'.$project->getSequence())) or casting to bigint explicitly at DB level.
🤖 Prompt for AI Agents
In app/controllers/api/projects.php around lines 233 to 240, the code casts the
project sequence to an integer for setTenant, which risks overflow or truncation
if the sequence exceeds 32-bit integer limits. To fix this, avoid casting to int
and instead pass the sequence as a string consistently, matching the approach
used in setNamespace. Ensure the database schema supports storing this value as
a string or a bigint to prevent overflow issues.
| $protocol = System::getEnv('_APP_OPTIONS_FORCE_HTTPS') === 'disabled' ? 'http' : 'https'; | ||
| $hostname = System::getEnv('_APP_CONSOLE_DOMAIN', System::getEnv('_APP_DOMAIN', '')); | ||
|
|
||
| $authorizeUrl = $request->getProtocol() . '://' . $request->getHostname() . "/console/git/authorize-contributor?projectId={$projectId}&installationId={$installationId}&repositoryId={$repositoryId}&providerPullRequestId={$providerPullRequestId}"; | ||
| $authorizeUrl = $protocol . '://' . $hostname . "/console/git/authorize-contributor?projectId={$projectId}&installationId={$installationId}&repositoryId={$repositoryId}&providerPullRequestId={$providerPullRequestId}"; |
There was a problem hiding this comment.
💡 Verification agent
❓ Verification inconclusive
Verify environment variables are properly configured.
The URL construction now relies on environment variables for protocol and hostname determination, which is a security improvement. However, ensure these variables are documented and validated.
Also applies to: 443-451, 500-512
🏁 Script executed:
#!/bin/bash
# Description: Check if the new environment variables are documented and have defaults
# Check .env file for the new variables
fd -t f "\.env$" -x grep -l "_APP_CONSOLE_DOMAIN\|_APP_OPTIONS_FORCE_HTTPS" {} \;
# Check if there are any references to these variables in documentation
fd -t f "README\|INSTALL\|\.md$" -x grep -l "_APP_CONSOLE_DOMAIN\|_APP_OPTIONS_FORCE_HTTPS" {} \;Length of output: 189
Document and validate new environment variables
The newly introduced _APP_OPTIONS_FORCE_HTTPS and _APP_CONSOLE_DOMAIN are not referenced in any .env example or project documentation. Please add and validate them to ensure correct defaults and prevent misconfiguration.
• Add both variables to your .env.example (or equivalent) with sensible default values and comments on allowed values.
• Update README.md (or relevant docs) to describe their purpose, usage, and default behavior.
• In your bootstrap or configuration code, explicitly validate these variables (e.g. check that _APP_OPTIONS_FORCE_HTTPS is enabled/disabled, that _APP_CONSOLE_DOMAIN is a valid hostname) and fall back to safe defaults if validation fails.
🤖 Prompt for AI Agents
In app/controllers/api/vcs.php around lines 119 to 122, the new environment
variables _APP_OPTIONS_FORCE_HTTPS and _APP_CONSOLE_DOMAIN are used but not
documented or validated. Add these variables to the .env.example file with clear
default values and comments explaining their purpose and allowed values. Update
the README.md or relevant documentation to describe their usage and default
behavior. Implement validation logic in the bootstrap or configuration code to
ensure _APP_OPTIONS_FORCE_HTTPS only accepts 'enabled' or 'disabled' and that
_APP_CONSOLE_DOMAIN is a valid hostname, falling back to safe defaults if
validation fails.
| $dbForPlatform->createCollection('bucket_' . $bucket->getSequence(), $attributes, $indexes); | ||
| } |
There was a problem hiding this comment.
Potential data loss due to collection name change
Swapping from bucket_<internalId> to bucket_<sequence> creates new collections instead of re-using the originals.
After an upgrade, all existing file documents will remain in the old collection and will no longer be returned by the API, effectively hiding (orphaning) the data.
Before shipping, add an upgrade path, e.g.:
$legacy = 'bucket_' . $bucket->getInternalId();
$new = 'bucket_' . $bucket->getSequence();
if (!$db->getCollection($new)->isEmpty()) {
// nothing to do
} elseif (!$db->getCollection($legacy)->isEmpty()) {
$db->renameCollection($legacy, $new); // or copy & drop
}or keep reading from both names until a background migration finishes.
Without such guard rails, production users will perceive sudden data loss.
Also applies to: 360-361
🤖 Prompt for AI Agents
In app/http.php around lines 312-313 and also lines 360-361, the code changes
collection names from 'bucket_<internalId>' to 'bucket_<sequence>', which causes
new collections to be created and existing data to be orphaned. To fix this,
implement an upgrade path that checks if the new collection is empty and if the
legacy collection is not empty, then rename or migrate the legacy collection to
the new name before creating or using the new collection. This ensures existing
data remains accessible and prevents data loss after the upgrade.
What does this PR do?
(Provide a description of what this PR does and why it's needed.)
Test Plan
(Write your test plan here. If you changed any code, please provide us with clear instructions on how you verified your changes work. Screenshots may also be helpful.)
Related PRs and Issues
Checklist