-
Notifications
You must be signed in to change notification settings - Fork 199
Switch to branch deployments in one-click deployments #2647
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
Console (appwrite/console)Project ID: Tip Silent mode disables those chatty PR comments if you prefer peace and quiet |
WalkthroughReplaced tag-based deployment with branch-based flows across console and public pages for functions and sites. Added Git helpers Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Areas requiring extra attention:
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
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. Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
src/routes/(console)/project-[region]-[project]/sites/create-site/deploy/+page.svelte (2)
379-387: Deploy button should be disabled when branches aren't ready.The button's
disabledcondition checksdomainIsValidanddomain, but does not account for branch loading state or selection. This allows users to click Deploy before branches are loaded.Apply this diff to add branch validation to the disabled condition:
<Button size="s" fullWidthMobile submissionLoader forceShowLoader={$isSubmitting} on:click={() => formComponent.triggerSubmit()} - disabled={$isSubmitting || !domainIsValid || !domain}> + disabled={$isSubmitting || !domainIsValid || !domain || !selectedBranch || loadingBranches}> Deploy </Button>
251-253: Potential null access in page title.
data.repository.nameis accessed without optional chaining. If repository information is missing (as handled in onMount lines 158-164), this could cause a runtime error.<svelte:head> - <title>Deploy {data.repository.name} - Appwrite</title> + <title>Deploy {data.repository?.name ?? 'Site'} - Appwrite</title> </svelte:head>src/routes/(console)/project-[region]-[project]/functions/create-function/deploy/+page.svelte (2)
35-35: Potential null access when initializingname.
data.repository.nameis accessed directly without optional chaining. Ifdata.repositoryis undefined, this will throw at component initialization.- let name = $state(data.repository.name); + let name = $state(data.repository?.name ?? '');
206-208: Potential null access in page title.Same issue as the sites page -
data.repository.nameaccessed without optional chaining.<svelte:head> - <title>Deploy {data.repository.name} - Appwrite</title> + <title>Deploy {data.repository?.name ?? 'Function'} - Appwrite</title> </svelte:head>
🧹 Nitpick comments (1)
src/lib/helpers/github.ts (1)
58-72: Consider pagination for repositories with many branches.The GitHub
/repos/{owner}/{name}/branchesendpoint returns a maximum of 30 branches per page by default (100 max withper_pageparameter). Repositories with more branches will have their list truncated, potentially hiding important branches from the user.For one-click deployments this is likely acceptable since most template repositories have few branches, but worth noting.
If needed, you could add pagination support or increase
per_page:const branchesResponse = await fetch( - `https://api.github.com/repos/${owner}/${name}/branches` + `https://api.github.com/repos/${owner}/${name}/branches?per_page=100` );
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/lib/helpers/github.ts(1 hunks)src/routes/(console)/project-[region]-[project]/functions/create-function/deploy/+page.svelte(7 hunks)src/routes/(console)/project-[region]-[project]/sites/create-site/deploy/+page.svelte(7 hunks)
🧰 Additional context used
📓 Path-based instructions (7)
**/*.{ts,tsx,js,jsx,svelte}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx,js,jsx,svelte}: Import reusable modules from the src/lib directory using the $lib alias
Use minimal comments in code; reserve comments for TODOs or complex logic explanations
Use $lib, $routes, and $themes aliases instead of relative paths for module imports
Files:
src/lib/helpers/github.tssrc/routes/(console)/project-[region]-[project]/functions/create-function/deploy/+page.sveltesrc/routes/(console)/project-[region]-[project]/sites/create-site/deploy/+page.svelte
src/lib/helpers/**/*.ts
📄 CodeRabbit inference engine (AGENTS.md)
Implement pure functions in src/lib/helpers/ directory and use camelCase for helper function names
Files:
src/lib/helpers/github.ts
**/*.ts
📄 CodeRabbit inference engine (AGENTS.md)
**/*.ts: Define types inline or in .d.ts files, avoid creating separate .types.ts files
Use TypeScript in non-strict mode; any type is tolerated in this project
Files:
src/lib/helpers/github.ts
**/*.{ts,tsx,js,jsx,svelte,json}
📄 CodeRabbit inference engine (AGENTS.md)
Use 4 spaces for indentation, single quotes, 100 character line width, and no trailing commas per Prettier configuration
Files:
src/lib/helpers/github.tssrc/routes/(console)/project-[region]-[project]/functions/create-function/deploy/+page.sveltesrc/routes/(console)/project-[region]-[project]/sites/create-site/deploy/+page.svelte
src/routes/**/*.svelte
📄 CodeRabbit inference engine (AGENTS.md)
Use SvelteKit file conventions: +page.svelte for components, +page.ts for data loaders, +layout.svelte for wrappers, +error.svelte for error handling, and dynamic route params in square brackets like [param]
Files:
src/routes/(console)/project-[region]-[project]/functions/create-function/deploy/+page.sveltesrc/routes/(console)/project-[region]-[project]/sites/create-site/deploy/+page.svelte
**/*.svelte
📄 CodeRabbit inference engine (AGENTS.md)
Use Svelte 5 + SvelteKit 2 syntax with TypeScript for component development
Files:
src/routes/(console)/project-[region]-[project]/functions/create-function/deploy/+page.sveltesrc/routes/(console)/project-[region]-[project]/sites/create-site/deploy/+page.svelte
src/routes/**
📄 CodeRabbit inference engine (AGENTS.md)
Configure dynamic routes using SvelteKit convention with [param] syntax in route directory names
Files:
src/routes/(console)/project-[region]-[project]/functions/create-function/deploy/+page.sveltesrc/routes/(console)/project-[region]-[project]/sites/create-site/deploy/+page.svelte
🧠 Learnings (4)
📓 Common learnings
Learnt from: ItzNotABug
Repo: appwrite/console PR: 2316
File: src/routes/(console)/project-[region]-[project]/functions/create-function/deploy/+page.svelte:95-103
Timestamp: 2025-09-08T13:21:53.793Z
Learning: In Appwrite's one-click deployment flows, Git tag validation should be lenient with fallbacks (like '1.0.0') rather than strict validation that blocks deployments, since the source code retrieval from the repository is more important than having proper version tags for the deployment to succeed.
📚 Learning: 2025-10-13T05:13:54.542Z
Learnt from: ItzNotABug
Repo: appwrite/console PR: 2413
File: src/routes/(console)/project-[region]-[project]/databases/table.svelte:33-39
Timestamp: 2025-10-13T05:13:54.542Z
Learning: In Svelte 5, `import { page } from '$app/state'` provides a reactive state proxy that can be accessed directly (e.g., `page.params`), unlike the older `import { page } from '$app/stores'` which returns a readable store requiring the `$page` syntax for auto-subscription in components.
Applied to files:
src/routes/(console)/project-[region]-[project]/sites/create-site/deploy/+page.svelte
📚 Learning: 2025-10-05T09:41:40.439Z
Learnt from: ItzNotABug
Repo: appwrite/console PR: 2398
File: src/routes/(console)/verify-email/+page.svelte:48-51
Timestamp: 2025-10-05T09:41:40.439Z
Learning: In SvelteKit 5, `page` imported from `$app/state` is a reactive state object (using runes), not a store. It should be accessed as `page.data` without the `$` prefix, unlike the store-based `$page` from `$app/stores` in earlier versions.
Applied to files:
src/routes/(console)/project-[region]-[project]/sites/create-site/deploy/+page.svelte
📚 Learning: 2025-10-13T05:16:07.656Z
Learnt from: ItzNotABug
Repo: appwrite/console PR: 2413
File: src/routes/(console)/project-[region]-[project]/databases/database-[database]/header.svelte:54-58
Timestamp: 2025-10-13T05:16:07.656Z
Learning: In SvelteKit apps, shared layout components (like headers) that use `$derived(page.data.*)` should use optional chaining when accessing properties that may not be present on all routes. During page transitions, reactive statements can briefly evaluate with different page.data structures, so optional chaining prevents runtime errors when navigating between routes with different data shapes (e.g., between `/databases` and `/databases/database-[database]`).
Applied to files:
src/routes/(console)/project-[region]-[project]/sites/create-site/deploy/+page.svelte
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: e2e
- GitHub Check: build
🔇 Additional comments (6)
src/routes/(console)/project-[region]-[project]/sites/create-site/deploy/+page.svelte (3)
124-164: LGTM on branch loading logic.Good use of
Promise.allfor parallel fetching, proper error handling with user notifications, and sensible default branch pre-selection fallback.
217-228: Verifydata.repositoryis defined before accessing properties.While
selectedBranchis validated at line 176,data.repository.nameanddata.repository.owneron lines 222-223 are accessed without null checks. If repository info is missing, this will throw.Consider adding a guard or using optional chaining with a fallback:
// Create deployment from GitHub repository using the selected branch + if (!data.repository?.owner || !data.repository?.name) { + addNotification({ + type: 'error', + message: 'Repository information is missing.' + }); + return; + } + const deployment = await sdk
305-315: LGTM on branch selection UI.The select input properly shows loading state, disables during fetch, and uses a clear placeholder. Good UX.
src/routes/(console)/project-[region]-[project]/functions/create-function/deploy/+page.svelte (3)
172-183: Add guard for repository info before deployment.Similar to the sites page,
data.repository.nameanddata.repository.ownerare accessed without null checks. The guard at line 128 only validates branch selection.Consider adding validation:
// Create deployment from GitHub repository using the selected branch + if (!data.repository?.owner || !data.repository?.name) { + addNotification({ + type: 'error', + message: 'Repository information is missing.' + }); + return; + } + await sdk
333-345: LGTM on Deploy button validation.The disabled condition properly includes branch readiness checks (
!selectedBranch || branches.length === 0), preventing submission before branches are loaded.
266-280: LGTM on Git configuration section.Clean implementation with proper loading state handling and disabled state during branch fetch.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/routes/(public)/functions/deploy/+page.svelte (1)
125-129:rootDirectoryfallback never used due to outer condition.The fallback
data.deploymentData.repository.rootDirectoryon line 128 will never be used because the outerif (rootDir)condition only executes whenrootDirfrom the URL is truthy. IfrootDiris null/undefined, the entire block is skipped.If the intent is to also forward the root directory extracted from the repository URL when no explicit
rootDirparam is provided, consider:- if (rootDir) - url.searchParams.set( - 'rootDir', - rootDir || data.deploymentData.repository.rootDirectory - ); + const effectiveRootDir = rootDir || data.deploymentData.repository.rootDirectory; + if (effectiveRootDir) url.searchParams.set('rootDir', effectiveRootDir);
♻️ Duplicate comments (1)
src/routes/(console)/project-[region]-[project]/sites/create-site/deploy/+page.ts (1)
26-40: Same duplicate envVars parsing as noted in functions loader.This parsing logic is identical to the functions deploy loader. As mentioned previously, consider extracting to a shared utility.
🧹 Nitpick comments (4)
src/routes/(public)/functions/deploy/+page.ts (1)
64-70: Type mismatch:rootDirectoryinitialized asnullbut typed asstring | undefined.The type declaration on line 57 defines
rootDirectory?: string, but the initialization assignsnull. While this works in non-strict TypeScript, it's inconsistent.deploymentData.repository = { url: repository!, - rootDirectory: null + rootDirectory: undefined },src/routes/(console)/project-[region]-[project]/sites/create-site/deploy/+page.svelte (1)
395-403: Consider disabling the Deploy button while branches are loading.The button is disabled when
$isSubmitting || !domainIsValid || !domain, but not whenloadingBranchesis true. Users could click Deploy before branches finish loading, which would trigger the error notification at line 192-198.disabled={$isSubmitting || !domainIsValid || !domain}> + disabled={$isSubmitting || !domainIsValid || !domain || loadingBranches || !selectedBranch}>src/routes/(console)/project-[region]-[project]/functions/create-function/deploy/+page.ts (1)
33-47: Environment variable parsing duplicated across loaders.The same
KEYorKEY=valueparsing logic is repeated in both the functions and sites deploy loaders. Consider extracting this to a shared helper in$lib/helpers/github.tsor a new utility file.// In $lib/helpers/env.ts or similar export function parseEnvParams(envParam: string | null): Array<{ key: string; value: string }> { if (!envParam) return []; return envParam.split(',').map((entry) => { const trimmed = entry.trim(); const eqIndex = trimmed.indexOf('='); if (eqIndex === -1) { return { key: trimmed, value: '' }; } return { key: trimmed.substring(0, eqIndex), value: trimmed.substring(eqIndex + 1) }; }); }src/routes/(console)/project-[region]-[project]/sites/create-site/deploy/+page.ts (1)
55-58: Frameworks fetched twice if quick mode fails.In quick mode,
listFrameworks()is called at line 55-57. If quick mode fails (catch block at line 141-148), the code falls through to line 151-157 wherelistFrameworks()is called again. Consider caching the result or restructuring to avoid the duplicate API call.+ let frameworks = null; + if (quickMode) { try { const { regionalConsoleVariables } = await parent(); const preset = url.searchParams.get('preset') || 'nextjs'; const framework = (Object.values(Framework) as string[]).includes(preset.toLowerCase()) ? (preset.toLowerCase() as Framework) : Framework.Nextjs; // Get framework defaults - const frameworks = await sdk + frameworks = await sdk .forProject(params.region, params.project) .sites.listFrameworks();Then at line 151:
- const [frameworks, installations] = await Promise.all([ - sdk.forProject(params.region, params.project).sites.listFrameworks(), + const [fetchedFrameworks, installations] = await Promise.all([ + frameworks ?? sdk.forProject(params.region, params.project).sites.listFrameworks(), sdk .forProject(params.region, params.project) .vcs.listInstallations() .catch(() => null) ]); + frameworks = fetchedFrameworks;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
src/lib/helpers/github.ts(2 hunks)src/routes/(console)/project-[region]-[project]/functions/create-function/deploy/+page.svelte(8 hunks)src/routes/(console)/project-[region]-[project]/functions/create-function/deploy/+page.ts(3 hunks)src/routes/(console)/project-[region]-[project]/sites/create-site/deploy/+page.svelte(7 hunks)src/routes/(console)/project-[region]-[project]/sites/create-site/deploy/+page.ts(3 hunks)src/routes/(public)/functions/deploy/+page.svelte(3 hunks)src/routes/(public)/functions/deploy/+page.ts(5 hunks)src/routes/(public)/sites/deploy/+page.svelte(2 hunks)src/routes/(public)/sites/deploy/+page.ts(4 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/routes/(public)/sites/deploy/+page.svelte
🧰 Additional context used
📓 Path-based instructions (7)
**/*.{ts,tsx,js,jsx,svelte}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx,js,jsx,svelte}: Import reusable modules from the src/lib directory using the $lib alias
Use minimal comments in code; reserve comments for TODOs or complex logic explanations
Use $lib, $routes, and $themes aliases instead of relative paths for module imports
Files:
src/routes/(public)/sites/deploy/+page.tssrc/routes/(console)/project-[region]-[project]/sites/create-site/deploy/+page.tssrc/routes/(console)/project-[region]-[project]/functions/create-function/deploy/+page.tssrc/lib/helpers/github.tssrc/routes/(console)/project-[region]-[project]/functions/create-function/deploy/+page.sveltesrc/routes/(public)/functions/deploy/+page.sveltesrc/routes/(console)/project-[region]-[project]/sites/create-site/deploy/+page.sveltesrc/routes/(public)/functions/deploy/+page.ts
**/*.ts
📄 CodeRabbit inference engine (AGENTS.md)
**/*.ts: Define types inline or in .d.ts files, avoid creating separate .types.ts files
Use TypeScript in non-strict mode; any type is tolerated in this project
Files:
src/routes/(public)/sites/deploy/+page.tssrc/routes/(console)/project-[region]-[project]/sites/create-site/deploy/+page.tssrc/routes/(console)/project-[region]-[project]/functions/create-function/deploy/+page.tssrc/lib/helpers/github.tssrc/routes/(public)/functions/deploy/+page.ts
**/*.{ts,tsx,js,jsx,svelte,json}
📄 CodeRabbit inference engine (AGENTS.md)
Use 4 spaces for indentation, single quotes, 100 character line width, and no trailing commas per Prettier configuration
Files:
src/routes/(public)/sites/deploy/+page.tssrc/routes/(console)/project-[region]-[project]/sites/create-site/deploy/+page.tssrc/routes/(console)/project-[region]-[project]/functions/create-function/deploy/+page.tssrc/lib/helpers/github.tssrc/routes/(console)/project-[region]-[project]/functions/create-function/deploy/+page.sveltesrc/routes/(public)/functions/deploy/+page.sveltesrc/routes/(console)/project-[region]-[project]/sites/create-site/deploy/+page.sveltesrc/routes/(public)/functions/deploy/+page.ts
src/routes/**
📄 CodeRabbit inference engine (AGENTS.md)
Configure dynamic routes using SvelteKit convention with [param] syntax in route directory names
Files:
src/routes/(public)/sites/deploy/+page.tssrc/routes/(console)/project-[region]-[project]/sites/create-site/deploy/+page.tssrc/routes/(console)/project-[region]-[project]/functions/create-function/deploy/+page.tssrc/routes/(console)/project-[region]-[project]/functions/create-function/deploy/+page.sveltesrc/routes/(public)/functions/deploy/+page.sveltesrc/routes/(console)/project-[region]-[project]/sites/create-site/deploy/+page.sveltesrc/routes/(public)/functions/deploy/+page.ts
src/lib/helpers/**/*.ts
📄 CodeRabbit inference engine (AGENTS.md)
Implement pure functions in src/lib/helpers/ directory and use camelCase for helper function names
Files:
src/lib/helpers/github.ts
src/routes/**/*.svelte
📄 CodeRabbit inference engine (AGENTS.md)
Use SvelteKit file conventions: +page.svelte for components, +page.ts for data loaders, +layout.svelte for wrappers, +error.svelte for error handling, and dynamic route params in square brackets like [param]
Files:
src/routes/(console)/project-[region]-[project]/functions/create-function/deploy/+page.sveltesrc/routes/(public)/functions/deploy/+page.sveltesrc/routes/(console)/project-[region]-[project]/sites/create-site/deploy/+page.svelte
**/*.svelte
📄 CodeRabbit inference engine (AGENTS.md)
Use Svelte 5 + SvelteKit 2 syntax with TypeScript for component development
Files:
src/routes/(console)/project-[region]-[project]/functions/create-function/deploy/+page.sveltesrc/routes/(public)/functions/deploy/+page.sveltesrc/routes/(console)/project-[region]-[project]/sites/create-site/deploy/+page.svelte
🧠 Learnings (7)
📓 Common learnings
Learnt from: ItzNotABug
Repo: appwrite/console PR: 2316
File: src/routes/(console)/project-[region]-[project]/functions/create-function/deploy/+page.svelte:95-103
Timestamp: 2025-09-08T13:21:53.793Z
Learning: In Appwrite's one-click deployment flows, Git tag validation should be lenient with fallbacks (like '1.0.0') rather than strict validation that blocks deployments, since the source code retrieval from the repository is more important than having proper version tags for the deployment to succeed.
📚 Learning: 2025-11-25T03:15:27.539Z
Learnt from: CR
Repo: appwrite/console PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-25T03:15:27.539Z
Learning: Applies to src/routes/**/*.svelte : Use SvelteKit file conventions: +page.svelte for components, +page.ts for data loaders, +layout.svelte for wrappers, +error.svelte for error handling, and dynamic route params in square brackets like [param]
Applied to files:
src/routes/(console)/project-[region]-[project]/sites/create-site/deploy/+page.tssrc/routes/(public)/functions/deploy/+page.svelte
📚 Learning: 2025-10-13T05:13:54.542Z
Learnt from: ItzNotABug
Repo: appwrite/console PR: 2413
File: src/routes/(console)/project-[region]-[project]/databases/table.svelte:33-39
Timestamp: 2025-10-13T05:13:54.542Z
Learning: In Svelte 5, `import { page } from '$app/state'` provides a reactive state proxy that can be accessed directly (e.g., `page.params`), unlike the older `import { page } from '$app/stores'` which returns a readable store requiring the `$page` syntax for auto-subscription in components.
Applied to files:
src/routes/(console)/project-[region]-[project]/sites/create-site/deploy/+page.tssrc/routes/(console)/project-[region]-[project]/functions/create-function/deploy/+page.tssrc/routes/(console)/project-[region]-[project]/sites/create-site/deploy/+page.svelte
📚 Learning: 2025-11-19T11:22:42.553Z
Learnt from: atharvadeosthale
Repo: appwrite/console PR: 2512
File: src/routes/(console)/project-[region]-[project]/overview/platforms/llmBanner.svelte:51-83
Timestamp: 2025-11-19T11:22:42.553Z
Learning: In src/routes/(console)/project-[region]-[project]/overview/platforms/llmBanner.svelte, the Lovable integration URL format `https://lovable.dev/` with `autosubmit` and `prompt` as query parameters (set via searchParams) is correct and functional.
Applied to files:
src/routes/(public)/functions/deploy/+page.svelte
📚 Learning: 2025-11-19T11:22:42.553Z
Learnt from: atharvadeosthale
Repo: appwrite/console PR: 2512
File: src/routes/(console)/project-[region]-[project]/overview/platforms/llmBanner.svelte:51-83
Timestamp: 2025-11-19T11:22:42.553Z
Learning: In src/routes/(console)/project-[region]-[project]/overview/platforms/llmBanner.svelte, the Cursor integration URL format `https://cursor.com/link/prompt` with the `text` query parameter is correct and functional.
Applied to files:
src/routes/(public)/functions/deploy/+page.svelte
📚 Learning: 2025-10-05T09:41:40.439Z
Learnt from: ItzNotABug
Repo: appwrite/console PR: 2398
File: src/routes/(console)/verify-email/+page.svelte:48-51
Timestamp: 2025-10-05T09:41:40.439Z
Learning: In SvelteKit 5, `page` imported from `$app/state` is a reactive state object (using runes), not a store. It should be accessed as `page.data` without the `$` prefix, unlike the store-based `$page` from `$app/stores` in earlier versions.
Applied to files:
src/routes/(console)/project-[region]-[project]/sites/create-site/deploy/+page.svelte
📚 Learning: 2025-10-13T05:16:07.656Z
Learnt from: ItzNotABug
Repo: appwrite/console PR: 2413
File: src/routes/(console)/project-[region]-[project]/databases/database-[database]/header.svelte:54-58
Timestamp: 2025-10-13T05:16:07.656Z
Learning: In SvelteKit apps, shared layout components (like headers) that use `$derived(page.data.*)` should use optional chaining when accessing properties that may not be present on all routes. During page transitions, reactive statements can briefly evaluate with different page.data structures, so optional chaining prevents runtime errors when navigating between routes with different data shapes (e.g., between `/databases` and `/databases/database-[database]`).
Applied to files:
src/routes/(console)/project-[region]-[project]/sites/create-site/deploy/+page.svelte
🧬 Code graph analysis (4)
src/routes/(public)/sites/deploy/+page.ts (1)
src/lib/helpers/github.ts (1)
getBranchFromUrl(6-9)
src/routes/(console)/project-[region]-[project]/sites/create-site/deploy/+page.ts (2)
src/lib/helpers/github.ts (3)
validateBranch(80-94)getDefaultBranch(47-61)getNestedRootDirectory(1-4)src/routes/(console)/project-[region]-[project]/store.ts (1)
regionalConsoleVariables(34-39)
src/routes/(console)/project-[region]-[project]/functions/create-function/deploy/+page.ts (3)
src/lib/helpers/github.ts (3)
getNestedRootDirectory(1-4)validateBranch(80-94)getDefaultBranch(47-61)src/lib/stores/sdk.ts (1)
sdk(173-196)src/routes/(console)/project-[region]-[project]/store.ts (1)
regionalConsoleVariables(34-39)
src/routes/(public)/functions/deploy/+page.ts (1)
src/lib/helpers/github.ts (1)
getBranchFromUrl(6-9)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: e2e
- GitHub Check: build
🔇 Additional comments (20)
src/lib/helpers/github.ts (2)
6-9: LGTM - Branch extraction helper is well-implemented.The regex correctly captures the branch segment from GitHub tree URLs and properly decodes URI components to handle special characters in branch names.
80-94: LGTM - Branch validation is correctly implemented.The function properly encodes the branch name in the URL path to handle special characters, and the error handling is consistent with other functions in this file.
src/routes/(public)/functions/deploy/+page.ts (2)
32-49: LGTM - Robust environment variable parsing.The parsing logic correctly handles both
KEYandKEY=valueformats, and usingindexOf('=')ensures values containing=characters are preserved correctly.
80-86: LGTM - Branch extraction integrated correctly.Branch is properly extracted from the URL and assigned to the deployment data, enabling branch-based deployments.
src/routes/(console)/project-[region]-[project]/functions/create-function/deploy/+page.svelte (4)
67-142: Well-structured branch loading with appropriate fallbacks.The implementation correctly:
- Validates URL-provided branch before using it
- Falls back to default branch, then first available branch
- Shows appropriate error notifications on failures
- Uses
Promise.allfor efficient parallel requestsThis aligns with the project's philosophy of lenient validation with fallbacks rather than strict blocking. Based on learnings, this approach ensures deployments succeed even when branch information isn't ideal.
144-151: LGTM - Appropriate guard clause for branch validation.The check ensures a branch is selected and branches are loaded before proceeding, with a clear user notification.
189-200: LGTM - Correct switch from tag-based to branch-based deployment.The deployment template now correctly uses
Type.Branchwithreference: selectedBranch, completing the migration from tag-based deployments.
283-297: LGTM - Clean branch selection UI.The fieldset properly handles loading state with placeholder text and disabled state, providing good UX during async branch loading.
src/routes/(public)/sites/deploy/+page.ts (2)
38-55: Consistent env parsing implementation.The env parsing logic is identical to
src/routes/(public)/functions/deploy/+page.ts. While this could be extracted to a shared helper, the duplication is acceptable given the limited scope.
87-101: LGTM - Branch extraction correctly integrated for repo-based deployments.Branch is properly extracted and assigned only for repository-based deployments (not templates), which is the correct behavior.
src/routes/(public)/functions/deploy/+page.svelte (2)
116-131: LGTM - Branch and quick parameter forwarding is correctly implemented.The branch priority (explicit URL param → extracted from repo URL) and conditional quick parameter forwarding are well-designed.
133-139: LGTM - Correct envVars serialization preserving values.The serialization correctly formats entries as
KEY=valuewhen a value exists, or justKEYotherwise, maintaining compatibility with the parsing logic.src/routes/(console)/project-[region]-[project]/sites/create-site/deploy/+page.svelte (3)
92-180: Branch loading and fallback logic looks good.The implementation properly handles:
- Parallel fetching of branch list, default branch, and validation
- Fallback to default branch or first branch when URL param is invalid
- User notifications on failure
Based on learnings, the lenient validation with fallbacks aligns with the preferred approach for one-click deployments.
241-242: Branch-based deployment correctly replaces tag-based approach.The deployment now uses
Type.Branchwithreference: selectedBranch, which aligns with the PR objective to switch from release-tag–based to branch-based deployments.
119-126: Type alignment verified—no issues found.The loader at
+page.ts(line 28) explicitly typesenvVarsasArray<{ key: string; value: string }>and returns it (line 160). The component correctly receives this shape and enriches it by adding thesecret: falseproperty during mapping. The type contract between loader and component is properly aligned.src/routes/(console)/project-[region]-[project]/functions/create-function/deploy/+page.ts (3)
68-77: Branch resolution logic is correct with proper fallbacks.The code validates the provided branch, falls back to the default branch, and ultimately to
'main'as a last resort. This aligns with the learnings indicating Git validation should be lenient with fallbacks.
96-99: Auto-generated domain usesID.unique()for subdomain.Using
ID.unique()generates a random identifier for the domain prefix. This is consistent with the sites implementation and provides unique domains automatically.
129-136: Quick mode error handling correctly re-throws redirects.Using
isRedirect(e)to distinguish between SvelteKit redirects and actual errors prevents swallowing the redirect and correctly falls through to the wizard on real failures.src/routes/(console)/project-[region]-[project]/sites/create-site/deploy/+page.ts (2)
76-85: Branch resolution consistent with functions loader.The branch validation and fallback logic mirrors the functions loader implementation, ensuring consistent behavior across both deployment types.
141-148: Error handling preserves redirect behavior.The
isRedirect(e)check properly re-throws SvelteKit redirects while allowing actual errors to fall through to the wizard UI. The console.error provides visibility into quick deploy failures.
| export async function getBranches(owner: string, name: string): Promise<string[] | null> { | ||
| try { | ||
| const branchesResponse = await fetch( | ||
| `https://api.github.com/repos/${owner}/${name}/branches` | ||
| ); | ||
| if (!branchesResponse.ok) { | ||
| return null; | ||
| } | ||
|
|
||
| const branches = await branchesResponse.json(); | ||
| return branches.map((branch) => branch.name); | ||
| } catch (error) { | ||
| console.error('Failed to fetch branches from GitHub:', error); | ||
| return null; | ||
| } | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🌐 Web query:
GitHub API branches endpoint pagination default limit
💡 Result:
The List Branches REST endpoint returns 30 results per page by default (per_page default = 30, max = 100); page defaults to 1. [1][2]
Sources:
[1] GitHub REST API — Branches (List branches) documentation.
[2] GitHub Docs — REST API endpoints for branches.
🏁 Script executed:
#!/bin/bash
# First, let's examine the complete getBranches function and its usage context
cat -n src/lib/helpers/github.ts | head -100Repository: appwrite/console
Length of output: 3534
🏁 Script executed:
#!/bin/bash
# Search for all usages of getBranches function to understand the context
rg -n "getBranches" --type ts --type tsxRepository: appwrite/console
Length of output: 87
🏁 Script executed:
#!/bin/bash
# Search for usages of getBranches
rg -n "getBranches" --type ts --type jsRepository: appwrite/console
Length of output: 179
🏁 Script executed:
#!/bin/bash
# Search for pagination patterns in the codebase
rg -n "per_page|pagination|page=" --type ts --type js -C 2Repository: appwrite/console
Length of output: 1074
🏁 Script executed:
#!/bin/bash
# Check if there are any Link headers or pagination handling elsewhere
rg -n "Link|headers" --type ts --type js src/lib/helpers/ -C 2Repository: appwrite/console
Length of output: 42
Based on my verification, I can now provide the rewritten review comment.
Add GitHub API pagination to handle repositories with more than 30 branches.
The GitHub Branches API defaults to 30 results per page, so repositories with many branches will have incomplete results. The suggested fix of adding ?per_page=100 improves this (GitHub's max), but note that repositories with more than 100 branches would still require full pagination support via the Link header or looping through pages.
For most repositories, increasing to per_page=100 is sufficient:
export async function getBranches(owner: string, name: string): Promise<string[] | null> {
try {
const branchesResponse = await fetch(
- `https://api.github.com/repos/${owner}/${name}/branches`
+ `https://api.github.com/repos/${owner}/${name}/branches?per_page=100`
);
if (!branchesResponse.ok) {
return null;
}
const branches = await branchesResponse.json();
return branches.map((branch) => branch.name);
} catch (error) {
console.error('Failed to fetch branches from GitHub:', error);
return null;
}
}🤖 Prompt for AI Agents
In src/lib/helpers/github.ts around lines 63 to 78, the current function fetches
only the first page of branches (GitHub defaults to 30 per page) causing missing
branches for large repos; update the function to request up to per_page=100 and
implement full pagination by looping requests and following the Link response
header (or incrementing page until an empty array) to accumulate all branch
names, deduplicate if needed, and return the combined list; ensure error
handling remains and stop when there is no next page.
| // Get first available specification | ||
| const specification = | ||
| specificationsList?.specifications?.[0]?.slug || 's-0.5vcpu-512mb'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check if this specification slug is used elsewhere or defined as a constant
rg -n 's-0.5vcpu-512mb' --type tsRepository: appwrite/console
Length of output: 236
🏁 Script executed:
#!/bin/bash
# Read the file to see the context around lines 79-81
cat -n src/routes/\(console\)/project-\[region\]-\[project\]/functions/create-function/deploy/+page.ts | sed -n '60,100p'Repository: appwrite/console
Length of output: 2099
🏁 Script executed:
#!/bin/bash
# Read the top of the file to see imports and variable definitions
cat -n src/routes/\(console\)/project-\[region\]-\[project\]/functions/create-function/deploy/+page.ts | head -80Repository: appwrite/console
Length of output: 3522
🏁 Script executed:
#!/bin/bash
# Find the parent layout file - should be in src/routes/(console)
find src/routes -name '+layout.ts' -o -name '+layout.svelte' | head -20Repository: appwrite/console
Length of output: 1394
🏁 Script executed:
#!/bin/bash
# Read the project-level layout file
cat -n src/routes/\(console\)/project-\[region\]-\[project\]/+layout.ts | head -150Repository: appwrite/console
Length of output: 3874
🏁 Script executed:
#!/bin/bash
# Read the console-level layout file and search for specificationsList
rg -n 'specificationsList' src/routes/\(console\)/+layout.ts -A 5 -B 5Repository: appwrite/console
Length of output: 42
🏁 Script executed:
#!/bin/bash
# Search for specificationsList definition across the codebase
rg -n 'specificationsList' --type ts -B 3 -A 3Repository: appwrite/console
Length of output: 13283
🏁 Script executed:
#!/bin/bash
# Read the functions layout file to see the full context
cat -n src/routes/\(console\)/project-\[region\]-\[project\]/functions/+layout.ts | head -35Repository: appwrite/console
Length of output: 1165
🏁 Script executed:
#!/bin/bash
# Search for specification constants or defaults
rg -n 'specification' --type ts -i | grep -i 'const\|default\|fallback' | head -20Repository: appwrite/console
Length of output: 1481
🏁 Script executed:
#!/bin/bash
# Search for constants files and any specification-related constants
find src -name '*const*' -type f | head -20Repository: appwrite/console
Length of output: 80
🏁 Script executed:
#!/bin/bash
# Read the constants file and search for specification-related content
rg -n 'specification' src/lib/constants.ts -i -A 2 -B 2Repository: appwrite/console
Length of output: 42
🏁 Script executed:
#!/bin/bash
# Search for other hardcoded values in the deploy page
rg -n 'node-18\|runtime\|slug' src/routes/\(console\)/project-\[region\]-\[project\]/functions/create-function/deploy/+page.ts -A 1 -B 1Repository: appwrite/console
Length of output: 42
🏁 Script executed:
#!/bin/bash
# Look at the runtime fallback on line 56
sed -n '54,58p' src/routes/\(console\)/project-\[region\]-\[project\]/functions/create-function/deploy/+page.tsRepository: appwrite/console
Length of output: 213
🏁 Script executed:
#!/bin/bash
# Search for types or documentation about specifications
rg -n 'SpecificationList|specification.*slug' --type ts -A 2 -B 2 | head -40Repository: appwrite/console
Length of output: 1653
Based on my verification, I've found:
- The hardcoded slug is used only once in the codebase at line 81 of the target file
- It's a valid fallback pattern - the code tries to get the first available specification and falls back to
's-0.5vcpu-512mb'when none are available - No constant defined - the slug is not extracted as a named constant anywhere in the codebase
- Consistent with the codebase pattern - similar fallbacks exist (e.g.,
'node-18.0'for runtimes) - The slug format appears valid - it follows Appwrite's specification naming convention and represents a 0.5 vCPU / 512MB specification
Minor Fixes Required
The hardcoded specification slug 's-0.5vcpu-512mb' should be extracted as a named constant for maintainability and easier future updates. While the slug appears valid and the fallback behavior is sound, this aligns with best practices for configuration values that might need centralized management.
🤖 Prompt for AI Agents
In
src/routes/(console)/project-[region]-[project]/functions/create-function/deploy/+page.ts
around lines 79 to 81, the fallback spec slug 's-0.5vcpu-512mb' is hardcoded;
extract it to a named constant (e.g., DEFAULT_SPEC_SLUG) placed either at the
top of this file or in a shared constants module, export it if placed in a
shared module, then replace the inline string with that constant; ensure any
imports are updated and run linters to confirm no unused-vars or export issues.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@atharvadeosthale agreee, see if we have proper constant for this. I think there's a RuntimeSpecification type 🤔
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag @coderabbitai in a new comment, and I'll come hopping out!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think we have that yet
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (3)
src/routes/(public)/functions/deploy/+page.svelte (1)
66-74: Consider tracking billing fetch errors.The silent error handling ensures the deployment flow isn't blocked by billing API issues. However, tracking these errors could help identify systematic problems.
Consider adding error tracking:
} catch { + trackError(error, 'billing.getOrganizationPlan'); currentPlan = null; }src/routes/(public)/sites/deploy/+page.svelte (2)
49-63: Consider adding explanatory comments for billing logic.The billing state logic is sound but complex. The conditions distinguish between addon territory (pricing info) and hard limits (block creation).
One edge case to verify: Line 54 checks
currentPlan?.projects > 0, which means ifprojectsis explicitly0, the limit check is bypassed (unlimited projects). Confirm this behavior is intentional.Consider adding inline comments explaining when each derived state applies:
let currentPlan = $state<Plan>(null); +// Block project creation when hard limit is reached let projectsLimited = $derived( isCloud && currentPlan?.projects > 0 && projects?.total !== undefined && projects.total >= currentPlan.projects ); +// Show addon pricing when using slots beyond plan-included count let isAddonProject = $derived( isCloud && currentPlan?.addons?.projects?.supported && projects?.total !== undefined && projects.total >= (currentPlan?.addons?.projects?.planIncluded ?? 0) );
429-431: LGTM! Billing-aware UI correctly gates project creation.The disabled states and alerts work together to:
- Show addon pricing when using extra project slots (lines 464-473)
- Block creation when at the hard limit (lines 474-481)
- Disable inputs appropriately (lines 429, 454)
The mutually exclusive alert conditions are correct.
For improved clarity, consider adding a comment above the alerts explaining the three UI states (under limit, in addon range, at limit).
Also applies to: 454-454, 464-481
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/routes/(public)/functions/deploy/+page.svelte(10 hunks)src/routes/(public)/sites/deploy/+page.svelte(8 hunks)
🧰 Additional context used
📓 Path-based instructions (5)
**/*.{ts,tsx,js,jsx,svelte}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx,js,jsx,svelte}: Import reusable modules from the src/lib directory using the $lib alias
Use minimal comments in code; reserve comments for TODOs or complex logic explanations
Use $lib, $routes, and $themes aliases instead of relative paths for module imports
Files:
src/routes/(public)/sites/deploy/+page.sveltesrc/routes/(public)/functions/deploy/+page.svelte
src/routes/**/*.svelte
📄 CodeRabbit inference engine (AGENTS.md)
Use SvelteKit file conventions: +page.svelte for components, +page.ts for data loaders, +layout.svelte for wrappers, +error.svelte for error handling, and dynamic route params in square brackets like [param]
Files:
src/routes/(public)/sites/deploy/+page.sveltesrc/routes/(public)/functions/deploy/+page.svelte
**/*.{ts,tsx,js,jsx,svelte,json}
📄 CodeRabbit inference engine (AGENTS.md)
Use 4 spaces for indentation, single quotes, 100 character line width, and no trailing commas per Prettier configuration
Files:
src/routes/(public)/sites/deploy/+page.sveltesrc/routes/(public)/functions/deploy/+page.svelte
**/*.svelte
📄 CodeRabbit inference engine (AGENTS.md)
Use Svelte 5 + SvelteKit 2 syntax with TypeScript for component development
Files:
src/routes/(public)/sites/deploy/+page.sveltesrc/routes/(public)/functions/deploy/+page.svelte
src/routes/**
📄 CodeRabbit inference engine (AGENTS.md)
Configure dynamic routes using SvelteKit convention with [param] syntax in route directory names
Files:
src/routes/(public)/sites/deploy/+page.sveltesrc/routes/(public)/functions/deploy/+page.svelte
🧠 Learnings (2)
📓 Common learnings
Learnt from: ItzNotABug
Repo: appwrite/console PR: 2316
File: src/routes/(console)/project-[region]-[project]/functions/create-function/deploy/+page.svelte:95-103
Timestamp: 2025-09-08T13:21:53.793Z
Learning: In Appwrite's one-click deployment flows, Git tag validation should be lenient with fallbacks (like '1.0.0') rather than strict validation that blocks deployments, since the source code retrieval from the repository is more important than having proper version tags for the deployment to succeed.
📚 Learning: 2025-11-19T11:22:42.553Z
Learnt from: atharvadeosthale
Repo: appwrite/console PR: 2512
File: src/routes/(console)/project-[region]-[project]/overview/platforms/llmBanner.svelte:51-83
Timestamp: 2025-11-19T11:22:42.553Z
Learning: In src/routes/(console)/project-[region]-[project]/overview/platforms/llmBanner.svelte, the Lovable integration URL format `https://lovable.dev/` with `autosubmit` and `prompt` as query parameters (set via searchParams) is correct and functional.
Applied to files:
src/routes/(public)/functions/deploy/+page.svelte
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: build
- GitHub Check: e2e
🔇 Additional comments (15)
src/routes/(public)/functions/deploy/+page.svelte (9)
8-8: LGTM! Billing imports added appropriately.The new imports for
Plan,Alert, andformatCurrencyalign with the billing-aware features introduced in this PR.Also applies to: 15-15, 28-28
43-57: LGTM! Billing state logic is sound.The derived values correctly implement project limit checks and addon eligibility. Optional chaining is appropriately used for nested addon properties.
122-122: LGTM! Region handling is correct.The logic properly handles cloud vs non-cloud modes with an appropriate fallback to 'default'.
273-273: LGTM! Consistent UX for billing limits.Disabling inputs when
projectsLimitedprevents users from completing forms they cannot submit, providing clear feedback aligned with the billing constraints.Also applies to: 275-275, 298-298
308-326: LGTM! Clear billing communication to users.The alerts provide appropriate feedback about addon costs and project limits. The mutually exclusive logic ensures users see the most relevant message.
332-336: LGTM! Button disable logic correctly handles billing limits.The updated condition appropriately prevents submission when project creation is blocked by billing constraints, maintaining consistency with the disabled inputs and alert messages.
146-147: LGTM! Strict boolean flag handling.The strict check
quick === 'true'ensures only the explicit 'true' value is forwarded, which is appropriate for a boolean flag parameter.Also applies to: 157-157
159-165: The dual-tracking is intentional and correct—envKeysis properly derived fromenvVars.The data loader (line 49 of +page.ts) explicitly derives
envKeysfromenvVarsvia.map((v) => v.key), with a comment stating "Keep envKeys for backward compatibility." SinceenvKeysis computed directly fromenvVarson every load, both arrays remain synchronized automatically. The code is working as designed.
143-145: Branch extraction logic is safe and properly handles null values.The data loader populates
repository.branchviagetBranchFromUrl(), which returnsstring | null. When the repository URL doesn't contain a branch pattern (e.g.,/tree/branch), it returnsnull. The fallback pattern in lines 143-145 is sound: it prefers the explicit URL parameter but safely falls back to the data field. Line 156 verifies the branch exists before setting it with the conditional checkif (branch), which prevents null values from propagating. No issues identified.src/routes/(public)/sites/deploy/+page.svelte (6)
9-9: LGTM! Imports support billing features.The new imports for
Plan,Alert, andformatCurrencyare appropriate for the billing-aware UI enhancements.Also applies to: 17-17, 32-32
71-78: LGTM! Plan fetching with graceful degradation.The plan fetch is properly guarded for cloud deployments and includes error handling. Silent failure (setting
null) is appropriate here since billing checks are enhancements that degrade gracefully if the plan info is unavailable.
126-127: LGTM! Proper region handling for cloud and self-hosted.The region resolution correctly uses the project's actual region for cloud deployments and falls back to 'default' appropriately. The inline comment clearly explains the logic.
489-489: LGTM! Submit button correctly prevents creation when at project limit.The additional condition properly disables the button when attempting to create a new project while at the organization's project limit, consistent with the disabled inputs and warning alert.
163-169: Environment variable format handling is correctly implemented and fully verified.The page loader (
+page.ts) at lines 40-52 properly parses theenvsearch parameter and handles bothKEYandKEY=valueformats:
- Splits entries by comma
- Detects
=delimiter to extract key and value- Returns
{key, value: ''}for key-only entries- Returns
{key, value}for key=value entriesThe
data.envVarsis correctly populated at line 148 and passed to the component, supporting the pre-filling behavior shown in the snippet at lines 163-169 of+page.svelte.
149-160: Branch handling verified - implementation is correct.The loader properly populates
data.deploymentData.repository.branchusing thegetBranchFromUrlhelper (line 88), extracting the branch from the repository URL structure and assigning it at line 96. The +page.svelte snippet correctly uses this value with URL parameter override, maintaining proper precedence.
src/lib/helpers/github.ts
Outdated
| } | ||
|
|
||
| export function getBranchFromUrl(repository: string): string | null { | ||
| const match = repository.match(/\/tree\/([^/]+)/); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| const match = repository.match(/\/tree\/([^/]+)/); | |
| const match = repository.match(/\/tree\/([^/?#]+)/); |
| }); | ||
| } | ||
| } catch (error) { | ||
| console.error('Failed to load branches:', error); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
leftover.
|
|
||
| if (quickMode) { | ||
| try { | ||
| const runtimeParam = runtime || 'node-18.0'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
mm, not sure about the default value of node-18. IIRC self-hosted has 22 enabled by default? whatever the value on SH side can be used here.
| // Get first available specification | ||
| const specification = | ||
| specificationsList?.specifications?.[0]?.slug || 's-0.5vcpu-512mb'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@atharvadeosthale agreee, see if we have proper constant for this. I think there's a RuntimeSpecification type 🤔
| envVars.map((variable) => | ||
| sdk.forProject(params.region, params.project).functions.createVariable({ | ||
| functionId: func.$id, | ||
| key: variable.key, | ||
| value: variable.value, | ||
| secret: false | ||
| }) | ||
| ) | ||
| ); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
unrelated but this has a very bad performance, we need a batch method for this.
| // Redirect to function page | ||
| redirect( | ||
| 302, | ||
| `${base}/project-${params.region}-${params.project}/functions/function-${func.$id}` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lets use resolve for all url building. type safety!
| const preset = url.searchParams.get('preset') || 'nextjs'; | ||
| const framework = (Object.values(Framework) as string[]).includes(preset.toLowerCase()) | ||
| ? (preset.toLowerCase() as Framework) | ||
| : Framework.Nextjs; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is this default intended? don't we have Other?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Most people would deploy Next.js. I want it so that it would work for most people without any configuration. Default being other means the end user will need to fill additional details which is not really a good experience imo
| throw e; | ||
| } | ||
| // On error, fall through to show the wizard | ||
| console.error('Quick deploy failed:', e); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
same here.
| // Redirect to deploying page | ||
| redirect( | ||
| 302, | ||
| `${base}/project-${params.region}-${params.project}/sites/create-site/deploying?site=${site.$id}&deployment=${deployment.$id}` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
resolve
| // Fetch plan info for billing checks | ||
| if (isCloud && selectedOrg) { | ||
| try { | ||
| currentPlan = await sdk.forConsole.billing.getOrganizationPlan(selectedOrg); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
there's a plansInfo derived store that has cached plan details, we could use that to avoid a request. If a plan isn't found in there, we should fallback to making the API call.
| const envKeys = envParam ? envParam.split(',').map((key: string) => key.trim()) : []; | ||
|
|
||
| // Parse env vars - supports KEY or KEY=value format | ||
| const envVars: Array<{ key: string; value: string }> = envParam |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lets add an helper of sorts, duplicated in quite a few places.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
♻️ Duplicate comments (2)
src/routes/(console)/project-[region]-[project]/sites/create-site/deploy/+page.svelte (1)
92-180: Branch-loading flow is robust with good fallbacks; consider loosening coupling togetDefaultBranchand deduping logicThe async
onMountflow looks solid overall:
- Handles optional
branchURL param and validates it.- Falls back to the repository’s default branch, then to the first branch in the list, which matches the “lenient Git selection with fallbacks” approach we want for one‑click flows. Based on learnings.
- Surfaces clear notifications when branches cannot be loaded or when repository info is missing.
Two non-blocking improvement ideas:
Decouple default-branch failure from branch-list failure
Right now, a failure in
getDefaultBranchwill cause the wholePromise.allto reject even ifgetBranchessucceeded. You could make the branch list the primary requirement and treat default-branch resolution as best-effort, e.g.:
const [branchList, defaultBranch, isBranchValid] = await Promise.all([getBranches(data.repository.owner, data.repository.name),getDefaultBranch(data.repository.owner, data.repository.name),branchParam? validateBranch(data.repository.owner, data.repository.name, branchParam): Promise.resolve(false)]);
const branchList = await getBranches(data.repository.owner,data.repository.name);let defaultBranch: string | undefined;try {defaultBranch = await getDefaultBranch(data.repository.owner,data.repository.name);} catch {// Ignore default branch failures; we'll fall back to the first branch}const isBranchValid = branchParam? await validateBranch(data.repository.owner,data.repository.name,branchParam): false;This way, a transient failure just in default-branch lookup doesn’t block use of the fetched branch list.
Deduplicate shared branch-init logic across one-click flows
If the functions deploy flow and other one-click pages are performing the same
branchParam/default/first-branch selection with identical notifications, consider extracting a small helper (e.g.initBranchSelection({ repo, searchParams })) or Svelte utility to avoid drift between screens.src/routes/(console)/project-[region]-[project]/functions/create-function/deploy/+page.ts (1)
66-69: Extract hardcoded specification slug into a named constantThe fallback
's-0.5vcpu-512mb'is still inlined; extracting it into a shared or top‑level constant (e.g.DEFAULT_FUNCTION_SPEC_SLUG) will make future spec changes safer and more discoverable. This was already raised earlier and the concern still applies here.- const specification = - specificationsList?.specifications?.[0]?.slug || 's-0.5vcpu-512mb'; + const specification = + specificationsList?.specifications?.[0]?.slug || DEFAULT_FUNCTION_SPEC_SLUG;
🧹 Nitpick comments (8)
src/routes/(public)/sites/deploy/+page.svelte (3)
73-90: Plan lookup viaplansInfocache then billing API is soundUsing the cached
plansInfomap whenorg.billingPlanis present and only falling back tosdk.forConsole.billing.getOrganizationPlanis a good optimization and keeps billing checks in sync with the org-level metadata.One small robustness tweak you might consider (optional): if
plansInfocan ever be empty or partially loaded, you could explicitly default to an emptyMapin the getter to avoid any surprises fromundefined:- const cachedPlans = get(plansInfo); + const cachedPlans = get(plansInfo) ?? new Map();Not required if
plansInfois guaranteed to always be aMap.
175-180: Guard againstdata.envVarsbeing undefined (or enforce loader default)
buildDeployUrlassumesdata.envVarsis always defined:if (data.envVars.length > 0) { url.searchParams.set( 'env', data.envVars.map((v) => (v.value ? `${v.key}=${v.value}` : v.key)).join(',') ); }If any loader path forgets to initialize
envVars(e.g.envVars: []), this will throw at runtime.Either ensure all loaders for this page set
envVars: []at minimum, or make this code defensive:- if (data.envVars.length > 0) { + if (data.envVars && data.envVars.length > 0) {Given the number of call sites, I’d lean toward enforcing a default
envVars: []in the load function and treating this as a contract, with the guard as optional extra safety.
441-444: Project-limit UI behavior is coherent; minor cleanup of unused sentinelThe new gating behaves as expected:
- Name and Region fields are disabled when
projectsLimited.- Add‑on pricing info (
isAddonProject && !projectsLimited) only shows when the user is in the paid add‑on range.- A clear warning is shown when the hard cap is reached (
projectsLimited).- The Continue button is disabled when trying to create a new project while limited.
One small nit: the Continue button condition still checks for a
'create-new'sentinel:(selectedProject === 'create-new' && (!projectName || (isCloud && !region)))but the
InputSelectusesnullas the “Create project” value, so this branch is effectively dead.You can safely drop the
'create-new'clause (or align the option value with it) to reduce confusion:- disabled={!selectedOrg || - (selectedProject === null && projectsLimited) || - (selectedProject === 'create-new' && - (!projectName || (isCloud && !region)))} + disabled={ + !selectedOrg || + (selectedProject === null && projectsLimited) + }Form-level
requiredattributes already enforceprojectName/region, so this simplification shouldn’t change behavior.Also applies to: 466-467, 476-493, 499-502
src/routes/(console)/project-[region]-[project]/sites/create-site/deploy/+page.svelte (1)
119-127: Guard againstdata.envVarsbeing missing, depending onPageDatacontractIf
PageData['envVars']is ever omitted orundefined,data.envVars.lengthwill throw at runtime. If the load function always provides at least an empty array, this is fine; otherwise, consider a small defensive tweak:- // Initialize environment variables from query params (with prefilled values if provided) - if (data.envVars.length > 0) { - variables = data.envVars.map((env) => ({ + // Initialize environment variables from loader data (with prefilled values if provided) + const envVars = data.envVars ?? []; + if (envVars.length > 0) { + variables = envVars.map((env) => ({ key: env.key, value: env.value, secret: false })); }src/routes/(public)/sites/deploy/+page.ts (1)
125-127: Silent error swallowing loses debugging context.The empty catch block with just
// ignoreprovides no visibility into organization creation failures. Consider logging the error for debugging purposes while still allowing the flow to continue.} catch (e) { - // ignore + // Organization creation failed - continue with available orgs + console.warn('Failed to create default organization:', e); }src/lib/helpers/env.ts (1)
7-19: Consider filtering out empty keys from malformed input.The function correctly handles
KEYandKEY=valueformats. However, if the input contains trailing commas or empty segments (e.g.,"KEY1,,KEY2"), this will produce entries with empty keys.export function parseEnvParam(envParam: string | null): Array<{ key: string; value: string }> { if (!envParam) return []; - return envParam.split(',').map((entry: string) => { + return envParam.split(',').filter(Boolean).map((entry: string) => { const trimmed = entry.trim(); + if (!trimmed) return null; const eqIndex = trimmed.indexOf('='); if (eqIndex === -1) { return { key: trimmed, value: '' }; } return { key: trimmed.substring(0, eqIndex), value: trimmed.substring(eqIndex + 1) }; - }); + }).filter((v): v is { key: string; value: string } => v !== null && v.key !== ''); }src/routes/(public)/functions/deploy/+page.ts (1)
108-110: Inconsistent error handling with sites deploy loader.This file logs the error with
console.error, whilesrc/routes/(public)/sites/deploy/+page.tssilently ignores it with just// ignore. Consider making these consistent—either both log or both silently continue.src/routes/(console)/project-[region]-[project]/sites/create-site/deploy/+page.ts (1)
123-137: Consider surfacing quick‑mode failures instead of silently falling backIn quick mode, any non‑redirect error is swallowed and the loader just continues into the normal flow. That’s safe, but from a UX/debuggability standpoint it might be helpful to at least log the failure (similar to the functions loader) or surface a lightweight error hint so it’s clear why the auto‑deploy didn’t happen.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
-
src/lib/helpers/env.ts(1 hunks) -
src/lib/helpers/github.ts(2 hunks) -
src/routes/(console)/project-[region]-[project]/functions/create-function/deploy/+page.svelte(8 hunks) -
src/routes/(console)/project-[region]-[project]/functions/create-function/deploy/+page.ts(3 hunks) -
src/routes/(console)/project-[region]-[project]/sites/create-site/deploy/+page.svelte(7 hunks) -
src/routes/(console)/project-[region]-[project]/sites/create-site/deploy/+page.ts(3 hunks) -
src/routes/(public)/functions/deploy/+page.svelte(10 hunks) -
src/routes/(public)/functions/deploy/+page.ts(6 hunks) -
src/routes/(public)/sites/deploy/+page.svelte(8 hunks) -
src/routes/(public)/sites/deploy/+page.ts(4 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/lib/helpers/github.ts
🧰 Additional context used
📓 Path-based instructions (7)
**/*.{ts,tsx,js,jsx,svelte}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx,js,jsx,svelte}: Import reusable modules from the src/lib directory using the $lib alias
Use minimal comments in code; reserve comments for TODOs or complex logic explanations
Use $lib, $routes, and $themes aliases instead of relative paths for module imports
Files:
src/routes/(public)/sites/deploy/+page.tssrc/routes/(public)/functions/deploy/+page.sveltesrc/routes/(public)/sites/deploy/+page.sveltesrc/routes/(console)/project-[region]-[project]/functions/create-function/deploy/+page.sveltesrc/routes/(console)/project-[region]-[project]/sites/create-site/deploy/+page.sveltesrc/routes/(console)/project-[region]-[project]/sites/create-site/deploy/+page.tssrc/routes/(console)/project-[region]-[project]/functions/create-function/deploy/+page.tssrc/lib/helpers/env.tssrc/routes/(public)/functions/deploy/+page.ts
**/*.ts
📄 CodeRabbit inference engine (AGENTS.md)
**/*.ts: Define types inline or in .d.ts files, avoid creating separate .types.ts files
Use TypeScript in non-strict mode; any type is tolerated in this project
Files:
src/routes/(public)/sites/deploy/+page.tssrc/routes/(console)/project-[region]-[project]/sites/create-site/deploy/+page.tssrc/routes/(console)/project-[region]-[project]/functions/create-function/deploy/+page.tssrc/lib/helpers/env.tssrc/routes/(public)/functions/deploy/+page.ts
**/*.{ts,tsx,js,jsx,svelte,json}
📄 CodeRabbit inference engine (AGENTS.md)
Use 4 spaces for indentation, single quotes, 100 character line width, and no trailing commas per Prettier configuration
Files:
src/routes/(public)/sites/deploy/+page.tssrc/routes/(public)/functions/deploy/+page.sveltesrc/routes/(public)/sites/deploy/+page.sveltesrc/routes/(console)/project-[region]-[project]/functions/create-function/deploy/+page.sveltesrc/routes/(console)/project-[region]-[project]/sites/create-site/deploy/+page.sveltesrc/routes/(console)/project-[region]-[project]/sites/create-site/deploy/+page.tssrc/routes/(console)/project-[region]-[project]/functions/create-function/deploy/+page.tssrc/lib/helpers/env.tssrc/routes/(public)/functions/deploy/+page.ts
src/routes/**
📄 CodeRabbit inference engine (AGENTS.md)
Configure dynamic routes using SvelteKit convention with [param] syntax in route directory names
Files:
src/routes/(public)/sites/deploy/+page.tssrc/routes/(public)/functions/deploy/+page.sveltesrc/routes/(public)/sites/deploy/+page.sveltesrc/routes/(console)/project-[region]-[project]/functions/create-function/deploy/+page.sveltesrc/routes/(console)/project-[region]-[project]/sites/create-site/deploy/+page.sveltesrc/routes/(console)/project-[region]-[project]/sites/create-site/deploy/+page.tssrc/routes/(console)/project-[region]-[project]/functions/create-function/deploy/+page.tssrc/routes/(public)/functions/deploy/+page.ts
src/routes/**/*.svelte
📄 CodeRabbit inference engine (AGENTS.md)
Use SvelteKit file conventions: +page.svelte for components, +page.ts for data loaders, +layout.svelte for wrappers, +error.svelte for error handling, and dynamic route params in square brackets like [param]
Files:
src/routes/(public)/functions/deploy/+page.sveltesrc/routes/(public)/sites/deploy/+page.sveltesrc/routes/(console)/project-[region]-[project]/functions/create-function/deploy/+page.sveltesrc/routes/(console)/project-[region]-[project]/sites/create-site/deploy/+page.svelte
**/*.svelte
📄 CodeRabbit inference engine (AGENTS.md)
Use Svelte 5 + SvelteKit 2 syntax with TypeScript for component development
Files:
src/routes/(public)/functions/deploy/+page.sveltesrc/routes/(public)/sites/deploy/+page.sveltesrc/routes/(console)/project-[region]-[project]/functions/create-function/deploy/+page.sveltesrc/routes/(console)/project-[region]-[project]/sites/create-site/deploy/+page.svelte
src/lib/helpers/**/*.ts
📄 CodeRabbit inference engine (AGENTS.md)
Implement pure functions in src/lib/helpers/ directory and use camelCase for helper function names
Files:
src/lib/helpers/env.ts
🧠 Learnings (10)
📓 Common learnings
Learnt from: ItzNotABug
Repo: appwrite/console PR: 2316
File: src/routes/(console)/project-[region]-[project]/functions/create-function/deploy/+page.svelte:95-103
Timestamp: 2025-09-08T13:21:53.793Z
Learning: In Appwrite's one-click deployment flows, Git tag validation should be lenient with fallbacks (like '1.0.0') rather than strict validation that blocks deployments, since the source code retrieval from the repository is more important than having proper version tags for the deployment to succeed.
📚 Learning: 2025-11-25T03:15:27.539Z
Learnt from: CR
Repo: appwrite/console PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-25T03:15:27.539Z
Learning: Applies to src/routes/**/*.svelte : Use SvelteKit file conventions: +page.svelte for components, +page.ts for data loaders, +layout.svelte for wrappers, +error.svelte for error handling, and dynamic route params in square brackets like [param]
Applied to files:
src/routes/(public)/functions/deploy/+page.sveltesrc/routes/(public)/sites/deploy/+page.sveltesrc/routes/(console)/project-[region]-[project]/sites/create-site/deploy/+page.ts
📚 Learning: 2025-11-19T11:22:42.553Z
Learnt from: atharvadeosthale
Repo: appwrite/console PR: 2512
File: src/routes/(console)/project-[region]-[project]/overview/platforms/llmBanner.svelte:51-83
Timestamp: 2025-11-19T11:22:42.553Z
Learning: In src/routes/(console)/project-[region]-[project]/overview/platforms/llmBanner.svelte, the Lovable integration URL format `https://lovable.dev/` with `autosubmit` and `prompt` as query parameters (set via searchParams) is correct and functional.
Applied to files:
src/routes/(public)/functions/deploy/+page.sveltesrc/routes/(public)/sites/deploy/+page.svelte
📚 Learning: 2025-10-13T05:13:54.542Z
Learnt from: ItzNotABug
Repo: appwrite/console PR: 2413
File: src/routes/(console)/project-[region]-[project]/databases/table.svelte:33-39
Timestamp: 2025-10-13T05:13:54.542Z
Learning: In Svelte 5, `import { page } from '$app/state'` provides a reactive state proxy that can be accessed directly (e.g., `page.params`), unlike the older `import { page } from '$app/stores'` which returns a readable store requiring the `$page` syntax for auto-subscription in components.
Applied to files:
src/routes/(public)/functions/deploy/+page.sveltesrc/routes/(public)/sites/deploy/+page.sveltesrc/routes/(console)/project-[region]-[project]/sites/create-site/deploy/+page.sveltesrc/routes/(console)/project-[region]-[project]/sites/create-site/deploy/+page.tssrc/routes/(console)/project-[region]-[project]/functions/create-function/deploy/+page.ts
📚 Learning: 2025-11-25T03:15:27.539Z
Learnt from: CR
Repo: appwrite/console PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-25T03:15:27.539Z
Learning: Applies to **/*.svelte : Use Svelte 5 + SvelteKit 2 syntax with TypeScript for component development
Applied to files:
src/routes/(public)/functions/deploy/+page.sveltesrc/routes/(public)/sites/deploy/+page.svelte
📚 Learning: 2025-09-08T13:20:47.308Z
Learnt from: ItzNotABug
Repo: appwrite/console PR: 2316
File: src/routes/(console)/project-[region]-[project]/functions/create-function/deploy/+page.svelte:29-29
Timestamp: 2025-09-08T13:20:47.308Z
Learning: The Form.svelte component in the Appwrite console creates a FormContext with isSubmitting as writable(false) and expects consumers to work with Svelte writable stores, not plain booleans.
Applied to files:
src/routes/(public)/functions/deploy/+page.sveltesrc/routes/(public)/sites/deploy/+page.svelte
📚 Learning: 2025-11-19T11:22:42.553Z
Learnt from: atharvadeosthale
Repo: appwrite/console PR: 2512
File: src/routes/(console)/project-[region]-[project]/overview/platforms/llmBanner.svelte:51-83
Timestamp: 2025-11-19T11:22:42.553Z
Learning: In src/routes/(console)/project-[region]-[project]/overview/platforms/llmBanner.svelte, the Cursor integration URL format `https://cursor.com/link/prompt` with the `text` query parameter is correct and functional.
Applied to files:
src/routes/(public)/functions/deploy/+page.sveltesrc/routes/(public)/sites/deploy/+page.svelte
📚 Learning: 2025-09-08T13:21:53.793Z
Learnt from: ItzNotABug
Repo: appwrite/console PR: 2316
File: src/routes/(console)/project-[region]-[project]/functions/create-function/deploy/+page.svelte:95-103
Timestamp: 2025-09-08T13:21:53.793Z
Learning: In Appwrite's one-click deployment flows, Git tag validation should be lenient with fallbacks (like '1.0.0') rather than strict validation that blocks deployments, since the source code retrieval from the repository is more important than having proper version tags for the deployment to succeed.
Applied to files:
src/routes/(console)/project-[region]-[project]/functions/create-function/deploy/+page.sveltesrc/routes/(console)/project-[region]-[project]/sites/create-site/deploy/+page.svelte
📚 Learning: 2025-10-05T09:41:40.439Z
Learnt from: ItzNotABug
Repo: appwrite/console PR: 2398
File: src/routes/(console)/verify-email/+page.svelte:48-51
Timestamp: 2025-10-05T09:41:40.439Z
Learning: In SvelteKit 5, `page` imported from `$app/state` is a reactive state object (using runes), not a store. It should be accessed as `page.data` without the `$` prefix, unlike the store-based `$page` from `$app/stores` in earlier versions.
Applied to files:
src/routes/(console)/project-[region]-[project]/sites/create-site/deploy/+page.sveltesrc/routes/(console)/project-[region]-[project]/sites/create-site/deploy/+page.ts
📚 Learning: 2025-10-13T05:16:07.656Z
Learnt from: ItzNotABug
Repo: appwrite/console PR: 2413
File: src/routes/(console)/project-[region]-[project]/databases/database-[database]/header.svelte:54-58
Timestamp: 2025-10-13T05:16:07.656Z
Learning: In SvelteKit apps, shared layout components (like headers) that use `$derived(page.data.*)` should use optional chaining when accessing properties that may not be present on all routes. During page transitions, reactive statements can briefly evaluate with different page.data structures, so optional chaining prevents runtime errors when navigating between routes with different data shapes (e.g., between `/databases` and `/databases/database-[database]`).
Applied to files:
src/routes/(console)/project-[region]-[project]/sites/create-site/deploy/+page.svelte
🧬 Code graph analysis (3)
src/routes/(public)/sites/deploy/+page.ts (2)
src/lib/helpers/env.ts (1)
parseEnvParam(7-20)src/lib/helpers/github.ts (1)
getBranchFromUrl(6-9)
src/routes/(console)/project-[region]-[project]/sites/create-site/deploy/+page.ts (3)
src/lib/helpers/env.ts (1)
parseEnvParam(7-20)src/lib/helpers/github.ts (3)
validateBranch(77-90)getDefaultBranch(46-59)getNestedRootDirectory(1-4)src/routes/(console)/project-[region]-[project]/store.ts (1)
regionalConsoleVariables(34-39)
src/routes/(public)/functions/deploy/+page.ts (2)
src/lib/helpers/env.ts (1)
parseEnvParam(7-20)src/lib/helpers/github.ts (1)
getBranchFromUrl(6-9)
🔇 Additional comments (25)
src/routes/(public)/sites/deploy/+page.svelte (2)
53-65: Billing limit / add‑on derivations look consistentThe
projectsLimitedandisAddonProjectderivations correctly:
- Only apply in cloud (
isCloud && ...).- Treat
projects === 0as “unlimited” via thecurrentPlan?.projects > 0guard.- Distinguish between “within included projects”, “paid add‑on range”, and “hard project cap”.
This matches the intended gating (add‑on info when between
planIncludedandprojects, hard-limit warning at/aboveprojects).
138-140: Region & branch propagation inbuildDeployUrlaligns with branch-based deploy flowUsing
project.region ?? 'default'for cloud and falling back to'default'for self-hosted keeps URLs consistent with project regions. The branch handling:const branch = currentUrl.searchParams.get('branch') || data.deploymentData.repository.branch; if (branch) url.searchParams.set('branch', branch);correctly prefers an explicit
branchquery param while still propagating a repository-derived branch when present, which fits the PR goal of branch-based one-click deployments.Also, restricting
quickto'true'avoids leaking arbitrary values into the query string.Also applies to: 161-166, 171-173
src/routes/(console)/project-[region]-[project]/sites/create-site/deploy/+page.svelte (5)
22-22: GitHub helpers import is correct and follows alias guidelinesImporting
getDefaultBranch,getBranches, andvalidateBranchfrom$lib/helpers/githubcentralizes GitHub logic behind the$libalias as expected; no issues here.
47-49: Branch-related state is clearly defined and matches later usage
branches,selectedBranch, andloadingBranchesare initialized with sensible defaults and are used consistently in the onMount loader, validation guard, and<Input.Select>, which keeps the flow easy to follow.
191-197: Guard onselectedBranchprevents deploying before branches are readyThe early return when
!selectedBranch || branches.length === 0cleanly prevents deployments from proceeding while branches are still loading or failed to load, and gives a clear user-facing error message.
232-242: Template deployment correctly switches to branch-based referenceUsing
type: Type.Branchtogether withreference: selectedBranchandrootDirectory: rootDir || '.'ensures the deployment is tied to the user-selected branch with a sensible root-directory fallback; the rest of the payload matches the created site and looks consistent.
320-330: Branch<Input.Select>integrates cleanly with state and loading UXThe branch selector wiring (placeholder tied to
loadingBranches, disabled while loading, options derived frombranches, andbind:value={selectedBranch}) is straightforward and keeps the UI in sync with the branch-loading logic.src/routes/(console)/project-[region]-[project]/functions/create-function/deploy/+page.svelte (4)
67-141: Well-structured branch loading with appropriate fallbacks.The async
onMountimplementation properly handles branch loading with:
- URL parameter priority for branch selection
- Fallback to default branch, then first available branch
- Error notifications for failure cases
This aligns with the lenient validation approach for one-click deployments. Based on learnings, this fallback behavior is appropriate since source code retrieval is more important than strict branch validation.
143-150: Guard clause provides good protection, but consider UX timing.The guard correctly prevents submission when branches aren't loaded. However, the button is already disabled via the
disabledprop on lines 354-359, so this guard acts as a secondary safeguard.
282-296: Git configuration section is well implemented.The branch selector properly:
- Shows loading state with placeholder text
- Disables during loading
- Maps branches to options correctly
188-198: Type.Branch enum exists and is correctly used.The code properly imports
Typefrom@appwrite.io/consoleand usesType.Branchconsistently with other deployment flows in the codebase (e.g., sites). No issues found.src/routes/(public)/sites/deploy/+page.ts (2)
39-42: Good backward compatibility approach for environment variables.Deriving
envKeysfromenvVarsmaintains backward compatibility while introducing the richerenvVarsstructure. This allows gradual migration of consumers.
74-83: Branch extraction is correctly integrated.The branch is properly extracted from the repository URL and assigned to the
deploymentData.repository.branchproperty, enabling branch-based deployments for sites.src/routes/(public)/functions/deploy/+page.ts (2)
56-56: Good default runtime update to Node 22.Using
Runtime.Node22as the default is appropriate for new deployments, providing users with a modern runtime version.
67-73: Branch extraction is consistent with other deploy flows.The pattern of extracting branch from URL and assigning to
repository.branchis consistent with the sites deploy loader, maintaining code consistency across deployment paths.src/routes/(public)/functions/deploy/+page.svelte (5)
68-85: Good implementation of cached plan lookup.The code correctly uses the cached
plansInfostore before falling back to an API call. This addresses the previous review feedback about avoiding unnecessary requests.
48-59: Billing gating logic is well-structured.The derived
projectsLimitedandisAddonProjectstates correctly gate project creation based on plan limits. The conditions properly check for cloud mode and compare project counts against plan limits.
154-169: Branch and quick parameter forwarding is correctly implemented.The logic properly sources the branch from either the explicit URL parameter or the repository data (extracted from
/tree/branchin the URL), with the explicit parameter taking precedence.
171-177: Environment variable serialization preserves format correctly.The serialization logic
v.value ? \${v.key}=${v.value}` : v.keycorrectly preserves theKEY=value` format for prefilled values while maintaining backward compatibility for key-only entries.
320-337: Billing alerts provide clear user guidance.The conditional alerts for addon projects and project limits give users clear information about pricing and limitations, improving the UX when limits are reached.
src/routes/(console)/project-[region]-[project]/functions/create-function/deploy/+page.ts (2)
51-64: Branch validation + default fallback aligns with lenient one‑click behaviorThe
validateBranch+getDefaultBranchflow with a final'main'fallback gives a non‑blocking, lenient resolution for invalid/missing branches, which is exactly what we want for one‑click flows (avoid hard failures, fall back to something sensible). Looks good to me. Based on learnings, this matches the desired “lenient validation with fallback” behavior.
33-35: Verify env query usage and non‑secret handling for one‑click variablesHere
envis parsed from the query string and used to create variables withsecret: false. That’s fine for public build‑time config (e.g.NEXT_PUBLIC_*), but would be problematic if any one‑click templates ever pass actual secrets this way (URL + non‑secret storage). Please double‑check that:
envis only used for non‑secret values in templates, and- anything secret continues to be collected in the UI and stored with
secret: true.If that assumption doesn’t hold, we should adjust this path (no secret values in the URL, mark as
secret: true, or require user input).Also applies to: 88-98, 141-143
src/routes/(console)/project-[region]-[project]/sites/create-site/deploy/+page.ts (3)
36-57: Framework preset + adapter defaults make quick‑mode sensible by defaultThe
preset→Frameworkmapping and use of the first adapter’s install/build/output when none are provided should give a good “it just works” experience for common cases like Next.js, while still allowing overrides via query params. This looks reasonable and matches the intended quick‑deploy behavior.
59-78: Consistent, lenient branch resolution for site deploymentsThe branch handling here mirrors the functions flow: validate an explicit branch when present, otherwise fall back to the repo’s default branch or
'main'. This is consistent and should prevent one‑click deployments from failing hard on bad/missing branch refs, which aligns with the lenient behavior we want for these flows.
26-28: Confirm env URL semantics and non‑secret usage for site variablesSame concern as in the functions deploy loader:
envfrom the query string is parsed intoenvVarsand used to create variables withsecret: false. Please confirm that:
- one‑click site templates only use this
envchannel for non‑secret config, and- any secrets are still gathered through the UI and stored as secret variables.
If secrets can flow through
env, we should avoid putting them in the URL and/or mark these variables assecret: trueinstead.Also applies to: 98-108, 148-150
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/routes/(public)/functions/deploy/+page.ts (1)
45-45: Avoid persistingnullforrepository.branch
getBranchFromUrlcan returnnull, butdeploymentData.repository.branchis modeled as optional. To keep the shape clean and avoid sending explicitnulldownstream, only set the field when a branch was actually parsed.- // Extract branch from URL if present (e.g., github.com/owner/repo/tree/branch) - const branchFromUrl = getBranchFromUrl(repository); + // Extract branch from URL if present (e.g., github.com/owner/repo/tree/branch) + const branchFromUrl = getBranchFromUrl(repository); @@ - deploymentData.repository.branch = branchFromUrl; + if (branchFromUrl) { + deploymentData.repository.branch = branchFromUrl; + }Also applies to: 67-69, 73-73
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/routes/(public)/functions/deploy/+page.ts(6 hunks)src/routes/(public)/sites/deploy/+page.ts(4 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/routes/(public)/sites/deploy/+page.ts
🧰 Additional context used
📓 Path-based instructions (4)
**/*.{ts,tsx,js,jsx,svelte}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx,js,jsx,svelte}: Import reusable modules from the src/lib directory using the $lib alias
Use minimal comments in code; reserve comments for TODOs or complex logic explanations
Use $lib, $routes, and $themes aliases instead of relative paths for module imports
Files:
src/routes/(public)/functions/deploy/+page.ts
**/*.ts
📄 CodeRabbit inference engine (AGENTS.md)
**/*.ts: Define types inline or in .d.ts files, avoid creating separate .types.ts files
Use TypeScript in non-strict mode; any type is tolerated in this project
Files:
src/routes/(public)/functions/deploy/+page.ts
**/*.{ts,tsx,js,jsx,svelte,json}
📄 CodeRabbit inference engine (AGENTS.md)
Use 4 spaces for indentation, single quotes, 100 character line width, and no trailing commas per Prettier configuration
Files:
src/routes/(public)/functions/deploy/+page.ts
src/routes/**
📄 CodeRabbit inference engine (AGENTS.md)
Configure dynamic routes using SvelteKit convention with [param] syntax in route directory names
Files:
src/routes/(public)/functions/deploy/+page.ts
🧠 Learnings (1)
📓 Common learnings
Learnt from: ItzNotABug
Repo: appwrite/console PR: 2316
File: src/routes/(console)/project-[region]-[project]/functions/create-function/deploy/+page.svelte:95-103
Timestamp: 2025-09-08T13:21:53.793Z
Learning: In Appwrite's one-click deployment flows, Git tag validation should be lenient with fallbacks (like '1.0.0') rather than strict validation that blocks deployments, since the source code retrieval from the repository is more important than having proper version tags for the deployment to succeed.
🧬 Code graph analysis (1)
src/routes/(public)/functions/deploy/+page.ts (2)
src/lib/helpers/env.ts (1)
parseEnvParam(7-20)src/lib/helpers/github.ts (1)
getBranchFromUrl(6-9)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: e2e
- GitHub Check: build
🔇 Additional comments (2)
src/routes/(public)/functions/deploy/+page.ts (2)
10-11: Env parsing helper + envVars/envKeys wiring look solidUsing
parseEnvParamwith a structuredenvVarsarray and then derivingenvKeysfrom it keeps backward compatibility while giving the UI richer data. The helper’s behavior (splitting on commas, handlingKEY=VALUEand bareKEYcases) matches howenvis typically encoded and should be safe even whenenvis absent (null→[]). No issues here.Also applies to: 34-36, 118-118
6-6: Runtime.Node22 is valid and already validated in the code
Runtime.Node22is a valid constant exported from@appwrite.io/consoleand is already used consistently across the codebase (e.g., in the create-function deploy flow). The code immediately fetcheslistRuntimes()on line 60, which validates the runtime against available runtimes on the target Appwrite instance. This pattern is already established and working elsewhere in the codebase. The only caveat is that older Appwrite server versions may not support Node 22; ensure the target Appwrite instance is version 1.6.1 or later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/routes/(public)/sites/deploy/+page.svelte(8 hunks)
🧰 Additional context used
📓 Path-based instructions (5)
**/*.{ts,tsx,js,jsx,svelte}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx,js,jsx,svelte}: Import reusable modules from the src/lib directory using the $lib alias
Use minimal comments in code; reserve comments for TODOs or complex logic explanations
Use $lib, $routes, and $themes aliases instead of relative paths for module imports
Files:
src/routes/(public)/sites/deploy/+page.svelte
src/routes/**/*.svelte
📄 CodeRabbit inference engine (AGENTS.md)
Use SvelteKit file conventions: +page.svelte for components, +page.ts for data loaders, +layout.svelte for wrappers, +error.svelte for error handling, and dynamic route params in square brackets like [param]
Files:
src/routes/(public)/sites/deploy/+page.svelte
**/*.{ts,tsx,js,jsx,svelte,json}
📄 CodeRabbit inference engine (AGENTS.md)
Use 4 spaces for indentation, single quotes, 100 character line width, and no trailing commas per Prettier configuration
Files:
src/routes/(public)/sites/deploy/+page.svelte
**/*.svelte
📄 CodeRabbit inference engine (AGENTS.md)
Use Svelte 5 + SvelteKit 2 syntax with TypeScript for component development
Files:
src/routes/(public)/sites/deploy/+page.svelte
src/routes/**
📄 CodeRabbit inference engine (AGENTS.md)
Configure dynamic routes using SvelteKit convention with [param] syntax in route directory names
Files:
src/routes/(public)/sites/deploy/+page.svelte
🧠 Learnings (7)
📓 Common learnings
Learnt from: ItzNotABug
Repo: appwrite/console PR: 2316
File: src/routes/(console)/project-[region]-[project]/functions/create-function/deploy/+page.svelte:95-103
Timestamp: 2025-09-08T13:21:53.793Z
Learning: In Appwrite's one-click deployment flows, Git tag validation should be lenient with fallbacks (like '1.0.0') rather than strict validation that blocks deployments, since the source code retrieval from the repository is more important than having proper version tags for the deployment to succeed.
📚 Learning: 2025-11-25T03:15:27.539Z
Learnt from: CR
Repo: appwrite/console PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-25T03:15:27.539Z
Learning: Applies to src/routes/**/*.svelte : Use SvelteKit file conventions: +page.svelte for components, +page.ts for data loaders, +layout.svelte for wrappers, +error.svelte for error handling, and dynamic route params in square brackets like [param]
Applied to files:
src/routes/(public)/sites/deploy/+page.svelte
📚 Learning: 2025-11-19T11:22:42.553Z
Learnt from: atharvadeosthale
Repo: appwrite/console PR: 2512
File: src/routes/(console)/project-[region]-[project]/overview/platforms/llmBanner.svelte:51-83
Timestamp: 2025-11-19T11:22:42.553Z
Learning: In src/routes/(console)/project-[region]-[project]/overview/platforms/llmBanner.svelte, the Lovable integration URL format `https://lovable.dev/` with `autosubmit` and `prompt` as query parameters (set via searchParams) is correct and functional.
Applied to files:
src/routes/(public)/sites/deploy/+page.svelte
📚 Learning: 2025-10-13T05:13:54.542Z
Learnt from: ItzNotABug
Repo: appwrite/console PR: 2413
File: src/routes/(console)/project-[region]-[project]/databases/table.svelte:33-39
Timestamp: 2025-10-13T05:13:54.542Z
Learning: In Svelte 5, `import { page } from '$app/state'` provides a reactive state proxy that can be accessed directly (e.g., `page.params`), unlike the older `import { page } from '$app/stores'` which returns a readable store requiring the `$page` syntax for auto-subscription in components.
Applied to files:
src/routes/(public)/sites/deploy/+page.svelte
📚 Learning: 2025-11-25T03:15:27.539Z
Learnt from: CR
Repo: appwrite/console PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-25T03:15:27.539Z
Learning: Applies to **/*.svelte : Use Svelte 5 + SvelteKit 2 syntax with TypeScript for component development
Applied to files:
src/routes/(public)/sites/deploy/+page.svelte
📚 Learning: 2025-11-19T11:22:42.553Z
Learnt from: atharvadeosthale
Repo: appwrite/console PR: 2512
File: src/routes/(console)/project-[region]-[project]/overview/platforms/llmBanner.svelte:51-83
Timestamp: 2025-11-19T11:22:42.553Z
Learning: In src/routes/(console)/project-[region]-[project]/overview/platforms/llmBanner.svelte, the Cursor integration URL format `https://cursor.com/link/prompt` with the `text` query parameter is correct and functional.
Applied to files:
src/routes/(public)/sites/deploy/+page.svelte
📚 Learning: 2025-09-08T13:20:47.308Z
Learnt from: ItzNotABug
Repo: appwrite/console PR: 2316
File: src/routes/(console)/project-[region]-[project]/functions/create-function/deploy/+page.svelte:29-29
Timestamp: 2025-09-08T13:20:47.308Z
Learning: The Form.svelte component in the Appwrite console creates a FormContext with isSubmitting as writable(false) and expects consumers to work with Svelte writable stores, not plain booleans.
Applied to files:
src/routes/(public)/sites/deploy/+page.svelte
🔇 Additional comments (6)
src/routes/(public)/sites/deploy/+page.svelte (6)
4-34: LGTM!The new imports are well-organized and correctly use the
$libalias. They appropriately support the billing checks and branch-based deployment functionality.
51-65: LGTM!The type fix for
currentPlancorrectly addresses the previous review feedback. The derived billing flags use proper optional chaining and defensive checks for undefined values.
73-90: LGTM!The plan fetching logic correctly uses the cached
plansInfostore with an API fallback. The silent error handling provides graceful degradation, which aligns with the lenient fallback approach for one-click deployments. Based on learnings, this ensures deployments proceed even when billing info is unavailable.
161-172: LGTM!The branch parameter handling correctly prioritizes an explicit URL parameter over the branch extracted from the repository URL (via
getBranchFromUrl). This properly supports the switch from tag-based to branch-based deployments while maintaining backward compatibility with explicit branch parameters.
175-181: LGTM!The environment variable handling correctly preserves the
KEY=valueformat for prefilled values while passing keys without values as justKEY. This allows downstream pages to prompt for missing values while preserving any pre-configured defaults.
476-493: LGTM!The billing-related alerts provide clear user messaging. The addon project info correctly uses
formatCurrencywith a sensible fallback price, and the project limit warning clearly communicates the constraint and upsell opportunity.
| disabled={!selectedOrg || | ||
| (selectedProject === null && projectsLimited) || | ||
| (selectedProject === 'create-new' && | ||
| (!projectName || (isCloud && !region)))} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pre-existing bug: Button disable logic checks wrong value for new project validation.
The new projectsLimited check on line 501 is correct. However, the existing condition on line 502 checks selectedProject === 'create-new', but the "Create project" option is defined with value: null (line 430). This means the validation for empty projectName or missing region never triggers via the disabled attribute.
The required attributes on the inputs provide browser-level validation as a fallback, but the disable logic should be consistent.
Consider fixing the pre-existing issue while adding the new billing check:
disabled={!selectedOrg ||
(selectedProject === null && projectsLimited) ||
- (selectedProject === 'create-new' &&
+ (selectedProject === null &&
(!projectName || (isCloud && !region)))}📝 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.
| disabled={!selectedOrg || | |
| (selectedProject === null && projectsLimited) || | |
| (selectedProject === 'create-new' && | |
| (!projectName || (isCloud && !region)))} | |
| disabled={!selectedOrg || | |
| (selectedProject === null && projectsLimited) || | |
| (selectedProject === null && | |
| (!projectName || (isCloud && !region)))} |
🤖 Prompt for AI Agents
In src/routes/(public)/sites/deploy/+page.svelte around lines 500-503, the
button disabled logic currently checks selectedProject === 'create-new' which is
incorrect because the "Create project" option uses value null; change that
clause to check selectedProject === null (or selectedProject == null) so the
empty projectName/region validation runs when creating a new project, and also
include the new billing check by OR-ing a condition like (billingRequired &&
!orgHasBilling) — i.e., disable the button if billing is required for this org
and the org does not have billing set up.

Ditches the release tag based deployments on one-click deployments, and rather uses branches instead. This means target repository doesn't need to have releases anymore.
Summary by CodeRabbit
New Features
Bug Fixes / Improvements
✏️ Tip: You can customize this high-level summary in your review settings.