app/config/templates/site.php (3)

1418-1436: Complete the template configuration for Formspree integration.

The Formspree template appears incomplete:

1. Wrong screenshot URLs: Using lynx gallery template URLs instead of Formspree-specific ones.
2. Empty variables array: Formspree typically requires configuration variables (e.g., form endpoint, API key).
3. Incomplete TODO items: Multiple TODO comments suggest this template needs further development.

Since this is a WIP template, consider adding typical Formspree environment variables:

-        'variables' => [] // TODO: Add env vars once added to template
+        'variables' => [
+            [
+                'name' => 'VITE_FORMSPREE_ENDPOINT',
+                'description' => 'Your Formspree form endpoint',
+                'value' => '',
+                'placeholder' => 'https://formspree.io/f/your-form-id',
+                'required' => true,
+                'type' => 'text'
+            ],
+        ]

And update the screenshot URLs:

-        'screenshotDark' => $url . '/images/sites/templates/gallery-for-lynx-dark.png', // TODO: Update this
-        'screenshotLight' => $url . '/images/sites/templates/gallery-for-lynx-light.png', // TODO: Update this
+        'screenshotDark' => $url . '/images/sites/templates/template-for-formspree-dark.png',
+        'screenshotLight' => $url . '/images/sites/templates/template-for-formspree-light.png',

---

1437-1455: Complete the template configuration for Clerk integration.

The Clerk template configuration is incomplete:

1. Wrong screenshot URLs: Using lynx gallery template URLs instead of Clerk-specific ones.
2. Missing environment variables: Clerk integration typically requires API keys and configuration.
3. Incomplete TODO items: Multiple TODO comments indicate this template needs completion.

Consider adding typical Clerk environment variables for Next.js:

-        'variables' => [] // TODO: Add the relevant variables for Clerk
+        'variables' => [
+            [
+                'name' => 'NEXT_PUBLIC_CLERK_PUBLISHABLE_KEY',
+                'description' => 'Your Clerk publishable key',
+                'value' => '',
+                'placeholder' => 'pk_test_...',
+                'required' => true,
+                'type' => 'text'
+            ],
+            [
+                'name' => 'CLERK_SECRET_KEY',
+                'description' => 'Your Clerk secret key',
+                'value' => '',
+                'placeholder' => 'sk_test_...',
+                'required' => true,
+                'type' => 'password'
+            ],
+        ]

And update the screenshot URLs:

-        'screenshotDark' => $url . '/images/sites/templates/gallery-for-lynx-dark.png', // TODO: Update this
-        'screenshotLight' => $url . '/images/sites/templates/gallery-for-lynx-light.png', // TODO: Update this
+        'screenshotDark' => $url . '/images/sites/templates/template-for-clerk-dark.png',
+        'screenshotLight' => $url . '/images/sites/templates/template-for-clerk-light.png',

---

1373-1455: Consider the use case categorization for these templates.

All three new templates use UseCases::STARTER with TODO comments questioning if they need new categories. Consider:

• React Admin Dashboard: Could fit better in a new ADMIN or DASHBOARD category
• Formspree: Could fit in a FORMS category or remain as STARTER
• Clerk: Could fit in an AUTH category or remain as STARTER

The current categorization works for now, but you might want to evaluate if new use case categories would provide better organization as more specialized templates are added.

Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro

app/config/templates/site.php (1)

1368-1412: Address the TODO comment and consider adjusting the score.

The template configuration looks correct, but:

1. Line 1373 has a TODO about checking if a new category is needed. Since this is an admin dashboard, consider whether a new use case category (e.g., UseCases::ADMIN or UseCases::DASHBOARD) would be more appropriate than STARTER.
2. The score is set to 1, which the guidelines suggest avoiding if possible (see line 175 comment).

Do you want me to help create a new use case category or suggest a more appropriate existing category for this admin dashboard template?

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

app/config/templates/site.php (1)

1475-1519: Resolve remaining TODOs and finalize metadata for React Admin template.

This entry looks structurally good (paths, screenshots, env vars), but there are still metadata gaps:

• Line 1479: score is set to 1, which goes against the inline guidance to avoid 1,2,3,8,9,10 if possible. Consider picking a more representative score.
• Line 1480: The TODO on useCases suggests the category is undecided. Before merge, either confirm UseCases::STARTER is correct or update it and remove the TODO so the config is “final”.

app/config/templates/site.php (1)

1521-1538: Finalize Formspree template configuration (score, useCases, env vars).

The Formspree entry is valid PHP, but still looks unfinished:

• Line 1524: score is 1, conflicting with the “avoid 1,2,3,8,9,10 if possible” guidance; choose a more realistic value once screenshots are final.
• Line 1525: TODO on useCases implies the category isn’t finalized; decide on the correct category and drop the TODO.
• Line 1537: variables is empty with a TODO. If the template truly needs no env vars, remove the comment; otherwise, add the required keys (e.g., Formspree IDs) so deployments aren’t missing configuration.

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

src/Appwrite/Platform/Tasks/Screenshot.php (2)

144-147: Consider using a clearer approach to get the last specification.

The \end() function modifies the array's internal pointer as a side effect. While this works, using \array_key_last() would be more explicit and avoid side effects.

Apply this diff for improved clarity:

         // Use best specifications to prevent out-of-memory during build
         $specifications = Config::getParam('specifications', []);
         $specifications = array_keys($specifications);
-        $specification = \end($specifications);
+        $specification = \array_key_last($specifications) ? $specifications[\array_key_last($specifications)] : \end($specifications);

Or even simpler:

         $specifications = Config::getParam('specifications', []);
-        $specifications = array_keys($specifications);
-        $specification = \end($specifications);
+        $specificationKeys = array_keys($specifications);
+        $specification = \end($specificationKeys);

---

54-54: Consider adding a clarifying comment for the hardcoded endpoint.

The hardcoded http://localhost/v1 is correct for the CLI client running inside the Docker container, while line 208 dynamically constructs the apiEndpoint for deployed sites. Adding a comment would make this distinction clearer for future maintainers.

+        // Internal endpoint for CLI client communication (within Docker)
         $client->setEndpoint('http://localhost/v1');

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

src/Appwrite/Platform/Tasks/Screenshot.php (2)

14-15: Validate _APP_DOMAIN before constructing {apiEndpoint} placeholder

The {apiEndpoint} placeholder is expanded using:

$value = \str_replace('{apiEndpoint}', 'http://' . System::getEnv('_APP_DOMAIN', '') . '/v1', $value);

If _APP_DOMAIN is unset or empty, this yields http:///v1, which will break templates relying on apiEndpoint and produce hard-to-debug failures. Adding an explicit validation here would make misconfiguration obvious, e.g.:

+                $appDomain = System::getEnv('_APP_DOMAIN', '');
+                if ($appDomain === '') {
+                    throw new \Exception('_APP_DOMAIN environment variable is not set; required for {apiEndpoint} replacement in screenshot task.');
+                }
+
-                $value = \str_replace('{apiEndpoint}', 'http://' . System::getEnv('_APP_DOMAIN', '') . '/v1', $value);
+                $value = \str_replace('{apiEndpoint}', 'http://' . $appDomain . '/v1', $value);

Based on learnings, _APP_DOMAIN is expected to always be set, so this should only trip in misconfigured environments.

Also applies to: 213-215

---

186-205: Avoid creating API keys with all available scopes

The screenshot flow currently creates a project API key with every defined scope:

'_scopes' => \array_keys(Config::getParam('scopes', []))

This violates least-privilege and risks exposing an overpowered key via site env vars (build logs, runtime introspection, etc.). Prefer a minimal, template-specific scope set, e.g.:

• Allow templates to declare required scopes (e.g. $template['apiScopes']) and intersect them with the global list.
• Fall back to a conservative default set if none are declared.

Example:

-        $response = $client->call(..., [
-            'name' => 'Screenshot API key',
-            'scopes' => \array_keys(Config::getParam('scopes', []))
-        ]);
+        $allScopes = \array_keys(Config::getParam('scopes', []));
+        $templateScopes = $template['apiScopes'] ?? [];
+        $scopes = empty($templateScopes)
+            ? ['databases.read', 'documents.read'] // or another minimal safe set
+            : \array_values(\array_intersect($templateScopes, $allScopes));
+
+        if (empty($scopes)) {
+            throw new \Exception('No valid scopes configured for Screenshot API key.');
+        }
+
+        $response = $client->call(..., [
+            'name' => 'Screenshot API key',
+            'scopes' => $scopes,
+        ]);

The rest of the key handling and injection into $variables['APPWRITE_API_KEY'] looks good.

src/Appwrite/Platform/Tasks/Screenshot.php (2)

5-7: CLI --variables usage and parsing look correct; consider minor clarity tweaks

The example --variables usage, new CLI param, and parsing logic together correctly support optional JSON env vars and fix the prior empty-string decode issue. You might still want to:

• Rename the $variables parameter to e.g. $variablesJson and introduce a separate $variables array to avoid type confusion (parameter is declared string but later holds an array).
• Drop the now-redundant if ($variables === null) check, since empty() short-circuits the empty-string case and non-empty invalid JSON already fails the \is_array() check.

These are just cleanup/clarity improvements; behavior is fine as-is.

Also applies to: 29-30, 33-45

---

210-221: Variable interpolation/overrides work; be aware of placeholder and empty() semantics

The interpolation and override flow is generally sound, but there are a couple of behavioral nuances to be aware of:

• CLI overrides skip placeholder replacement: You first expand {projectName}, {projectId}, and {apiEndpoint} in the template default, then override with $variables[$name] when present. Any placeholders inside a CLI-provided value will remain literal. If you want CLI-provided values to also support placeholders, you’d need to either:

  • Override first, then apply str_replace, or
  • Re-run the replacements after the override.

• empty($value) treats some valid strings as missing: Values like '0' will be considered empty and either skipped or trigger the “Missing required variable” exception. If you need to support such values, consider a stricter check such as $value === '' instead of empty($value).

Neither is necessarily wrong, but clarifying the intended behavior now will avoid surprises for template authors.

Also applies to: 223-223

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Learnt from: stnguyen90
Repo: appwrite/appwrite 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.