composer.json (1)

89-89: Review the decision to require a development branch for sdk-generator
Switching from a stable "0.40.*" constraint to "dev-fix-devkeys-header" may introduce instability and reproducibility issues.

• Verify that the fix-devkeys-header branch is published and accessible via Packagist or your custom repository.
• Ensure your Composer configuration (minimum-stability / prefer-stable) allows dev requirements.
• Consider pinning to a specific commit SHA or creating a tagged prerelease once the fix is merged upstream for more deterministic installs.

Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge Base: Disabled due to data retention organization setting

.github/workflows/sdk-preview.yml (1)

50-50: Optional: abstract SDK path into a reusable output
You could compute the combined SDK directory once and reuse it across steps. For example:

- name: Set SDK path
  id: sdk-path
  run: |
    echo "sdk_path=app/sdks/${{ steps.set-sdk.outputs.platform }}-${{ steps.set-sdk.outputs.sdk_type }}" >> $GITHUB_OUTPUT

- name: Build and Publish SDK
  working-directory: ${{ steps.sdk-path.outputs.sdk_path }}
  run: |
    npm install
    npm run build
    npx pkg-pr-new publish --comment=update

This centralizes the directory logic and makes future updates simpler.

Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge Base: Disabled due to data retention organization setting

app/http.php (1)

175-190: ⚠️ Potential issue

Potential connection-pool leak on repeated retry attempts

while (true) acquires a new adapter on every failed attempt without returning the previous one to the pool.
If the root cause is credentials/privileges (not network) the loop may exhaust the pool before hitting $max and start throwing unrelated “too many connections” errors.

Two possible mitigations:

-    $resource = $app->getResource($resourceKey);
+    try {
+        $resource = $app->getResource($resourceKey);
+    } finally {
+        // ensure connection is returned when the adapter constructor fails mid-handshake
+        $pools->reclaim(); // or adapter->close() depending on pool API
+    }

or re-use the same adapter instance across retries rather than re-allocating.

app/controllers/api/health.php (1)

103-113: ⚠️ Potential issue

ping latency calculation is wrong – divide vs multiply

(\microtime(true) - $checkStart) / 1000 converts seconds to seconds ÷ 1000 (i.e., milliseconds⁄1000).
If you intend to output milliseconds multiply by 1000, otherwise remove the division. Same issue exists for cache & pubsub sections.

- 'ping' => round((microtime(true) - $checkStart) / 1000)
+ 'ping' => round((microtime(true) - $checkStart) * 1000) // ms

app/controllers/general.php (1)

1050-1051: Header allow-list duplicated & growing – centralize to avoid drift

The expanded Access-Control-Allow-Headers string now appears twice with only minor differences. As more SDK headers are introduced this list keeps lengthening, increasing the chance that the two occurrences diverge.

Recommend placing the header list in a constant (e.g., CORS_ALLOW_HEADERS) and referencing it in both locations:

$response->addHeader('Access-Control-Allow-Headers', CORS_ALLOW_HEADERS);

Besides preventing inconsistencies it becomes easier to audit permitted headers for security reviews.

Also applies to: 1113-1114

app/http.php (2)

372-378: Wrapper allocation inside tight loop – cache adapter instance

new DatabasePool($pools->get($hostname)) is executed for every shared-table host on every cold-start.
Because DatabasePool already caches the base connection internally you can safely move the wrapper construction outside the loop or memoise it per $hostname to reduce object churn.

---

523-523: Stack trace logged in production – watch for PII & log noise

Dumping the full trace string helps during debugging but may expose sensitive file paths or secrets in production aggregators.
Consider guarding the extra Trace: line behind App::isDevelopment() or redacting passwords/tokens before emitting.

app/controllers/api/health.php (1)

92-124: Three nearly identical health-check blocks – extract helper

The DB, cache and pubsub checks share the same structure (loop pools → ping → collect failures).
Duplicated logic means three places to patch the ping fix and harder testability.

Consider a small reusable closure/helper:

$checkPools = function(array $names, callable $factory): array {
    $statuses = []; $fail = [];
    foreach ($names as $n) {
        try {
            $adapter = $factory($n);
            $ok = $adapter->ping();
        } catch (\Throwable) { $ok = false; }
        if ($ok) {
            $statuses[] = /*build doc*/;
        } else { $fail[] = $n; }
    }
    return [$statuses, $fail];
};

This reduces duplication and keeps the health file easier to reason about.

Also applies to: 154-185, 214-245

app/init/resources.php (1)

364-366: Duplicate logic – extract common builder

Identical adapter-construction logic now lives in three places (dbForProject, dbForPlatform, getProjectDB). Centralising it in a makeDatabaseFromPool(string $key): Database helper eliminates duplication and keeps future timeout / metadata tweaks in one spot.

app/controllers/general.php (1)

455-479: 🛠️ Refactor suggestion

DRY violation – duplicated env-var scaffolding for site & function

The two array_merge blocks build nearly identical lists that differ only in the prefix (APPWRITE_FUNCTION_* vs APPWRITE_SITE_*). Maintaining these manually-expanded arrays in lock-step is error-prone: any future addition for one resource type risks being forgotten for the other.

Consider extracting the common keys and computing the prefix dynamically, e.g.:

$baseVars = [
    'API_ENDPOINT'     => $endpoint,
    'ID'               => $resource->getId(),
    'NAME'             => $resource->getAttribute('name'),
    'DEPLOYMENT'       => $deployment->getId(),
    'PROJECT_ID'       => $project->getId(),
    'RUNTIME_NAME'     => $runtime['name'] ?? '',
    'RUNTIME_VERSION'  => $runtime['version'] ?? '',
    'CPUS'             => $spec['cpus'] ?? APP_COMPUTE_CPUS_DEFAULT,
    'MEMORY'           => $spec['memory'] ?? APP_COMPUTE_MEMORY_DEFAULT,
];

$prefix = $type === 'function' ? 'APPWRITE_FUNCTION_' : 'APPWRITE_SITE_';
$vars = array_merge(
    $vars,
    array_combine(
        array_map(fn($k) => $prefix.$k, array_keys($baseVars)),
        $baseVars
    )
);

This reduces duplication, eases future maintenance, and guarantees parity between the two resource types.

🤖 Prompt for AI Agents

In app/controllers/general.php around lines 455 to 479, the environment variable
arrays for 'function' and 'site' types are duplicated with only the prefix
differing. To fix this, extract the common key-value pairs into a single base
array without prefixes, then dynamically compute the prefix based on the type
('APPWRITE_FUNCTION_' or 'APPWRITE_SITE_'). Use array_map to prepend the prefix
to each key and array_combine to merge these prefixed keys with the base values,
then merge this result into $vars. This eliminates duplication and ensures
consistent environment variable sets for both types.

app/init/resources.php (5)

420-422: 🛠️ Refactor suggestion

Unbounded in-memory cache can leak connections

$databases[$dsn->getHost()] permanently stores Database objects for every host ever seen. In long-running PHP-FPM or Swoole workers this grows without limit.

Option A – LRU cache with a max size:

-if (isset($databases[$dsn->getHost()])) {
+if (array_key_exists($dsn->getHost(), $databases)) {
     ...
 }
-...
-$databases[$dsn->getHost()] = $database;
+// Cap at 50 hosts
+if (count($databases) >= 50) {
+    array_shift($databases); // Removes oldest
+}
+$databases[$dsn->getHost()] = $database;

Option B – rely on the upstream DatabasePool to multiplex and drop the manual array entirely.

Committable suggestion skipped: line range outside the PR's diff.

🤖 Prompt for AI Agents

In app/init/resources.php around lines 420 to 422, the $databases array stores
Database objects keyed by host indefinitely, causing unbounded memory growth in
long-running processes. To fix this, remove the manual $databases array entirely
and rely on the upstream DatabasePool to manage connection multiplexing and
lifecycle. This avoids retaining Database instances manually and prevents memory
leaks.

---

337-339: ⚠️ Potential issue

Missing pool-key guard can raise hard-to-trace null errors

$pools->get($dsn->getHost()) is assumed to exist. If the DSN host is mis-typed or the connection was not registered, Group::get() will likely return null leading to a fatal when DatabasePool expects a valid adapter.

Recommend:

$pool = $pools->get($dsn->getHost());
-if (!$pool) {
-    throw new Exception(Exception::GENERAL_SERVER_ERROR, "Database pool '{$dsn->getHost()}' not configured");
-}
-$adapter = new DatabasePool($pool);
+$adapter = new DatabasePool($pool ?? throw new Exception(
+    Exception::GENERAL_SERVER_ERROR,
+    "Database pool '{$dsn->getHost()}' not configured"
+));

This fails fast with a clear message instead of an opaque type error down-stream.

Committable suggestion skipped: line range outside the PR's diff.

🤖 Prompt for AI Agents

In app/init/resources.php around lines 337 to 339, the code assumes that
$pools->get($dsn->getHost()) always returns a valid adapter, but if the DSN host
is incorrect or missing, it returns null causing a fatal error later. Add a
guard to check if the adapter is null immediately after calling $pools->get(),
and if so, throw an exception or trigger an error with a clear message
indicating the missing pool key. This will fail fast and provide a clear
diagnostic instead of an obscure downstream error.

---

462-466: 🛠️ Refactor suggestion

Sharding with zero adapters will error on first call

If pools-cache env/config is empty the loop yields $adapters = [].
new Sharding([]) will later throw when pick() is called.

Add a fallback:

-    foreach ($list as $value) {
-        $adapters[] = new CachePool($pools->get($value));
-    }
-    return new Cache(new Sharding($adapters));
+    foreach ($list as $value) {
+        $adapter = $pools->get($value);
+        if ($adapter) {
+            $adapters[] = new CachePool($adapter);
+        }
+    }
+    return new Cache(
+        empty($adapters)
+            ? new \Utopia\Cache\Adapter\Memory()   // or NullAdapter
+            : new Sharding($adapters)
+    );

Prevents boot failure on minimal installations.

📝 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.

    foreach ($list as $value) {
        $adapter = $pools->get($value);
        if ($adapter) {
            $adapters[] = new CachePool($adapter);
        }
    }

    return new Cache(
        empty($adapters)
            ? new \Utopia\Cache\Adapter\Memory()   // or NullAdapter
            : new Sharding($adapters)
    );

🤖 Prompt for AI Agents

In app/init/resources.php around lines 462 to 466, the code creates a Sharding
instance with an empty adapters array if the pools-cache config is empty, which
causes an error when pick() is called. To fix this, add a check after the loop
to verify if $adapters is empty, and if so, provide a fallback adapter or handle
the case gracefully before creating the Sharding instance. This prevents boot
failure on minimal installations.

---

80-84: 🛠️ Refactor suggestion

Named-argument coupling may break on future SDKs

new BrokerPool(publisher: $pools->get('publisher')) and the matching call for consumer rely on parameter names instead of position.
If BrokerPool::__construct() changes its parameter list (common in evolving SDKs) the call will throw a Named parameter $publisher does not exist error.

Safer pattern:

-    return new BrokerPool(publisher: $pools->get('publisher'));
+    // Order-based to avoid tight coupling with parameter names
+    return new BrokerPool($pools->get('publisher'));

—or add an adapting factory so the surface area stays in one place.

🤖 Prompt for AI Agents

In app/init/resources.php around lines 80 to 84, the use of named arguments in
the BrokerPool constructor (e.g., publisher: $pools->get('publisher')) risks
breaking if the constructor's parameter names change in future SDK versions. To
fix this, replace the named arguments with positional arguments by passing the
parameters in the correct order without specifying names, or create an adapter
factory function that maps the current call to the constructor, isolating the
dependency on parameter names in one place.

---

432-440: ⚠️ Potential issue

Shared $database instance crossed by reference ⇒ race condition

The outer $database is reused for every call to getLogsDB().
Under Swoole coroutines simultaneous requests may:

1. Mutate tenant with setTenant().
2. Continue executing while another coroutine overwrites the tenant pointer.

Result: request A might suddenly read/write logs under request B’s tenant.

Safer approach – clone per call:

-        if ($database !== null && $project !== null ...
-            $database->setTenant($project->getInternalId());
-            return $database;
+        if ($database !== null) {
+            $instance = clone $database;            // cheap shallow clone
+            if ($project !== null && !$project->isEmpty() && $project->getId() !== 'console') {
+                $instance->setTenant($project->getInternalId());
+            }
+            return $instance;
         }

Or simply drop the shared static and let DatabasePool handle pooling.

📝 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.

    return function (?Document $project = null) use ($pools, $cache, &$database) {
        if ($database !== null) {
            $instance = clone $database;            // cheap shallow clone
            if ($project !== null && !$project->isEmpty() && $project->getId() !== 'console') {
                $instance->setTenant($project->getInternalId());
            }
            return $instance;
        }

        $adapter = new DatabasePool($pools->get('logs'));
        $database = new Database($adapter, $cache);

🤖 Prompt for AI Agents

In app/init/resources.php around lines 432 to 440, the shared $database variable
is passed by reference and reused across calls, causing potential race
conditions in concurrent environments like Swoole. To fix this, avoid reusing
the shared $database instance by removing the reference and instead create a new
Database instance for each call, either by cloning the existing one or by
instantiating a fresh Database object using the DatabasePool. This ensures each
request gets an isolated database connection without tenant data leakage.