Conversation
Console (appwrite/console)Project ID: Tip HTTPS and SSL certificates are handled automatically for all your Sites |
Contributor
WalkthroughAdds VCS authorization checks to src/lib/components/git/deploymentSource.svelte: new optional props Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes 🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Tip Try Coding Plans. Let us write the prompt for your AI agent so you can ship faster (with fewer bugs). 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 |
Contributor
Greptile SummaryAdded VCS authorization checking for deployments to warn users when their repository integration isn't authorized for auto-deployments. The warning appears as an icon next to the GitHub source link with a tooltip explaining how to fix it. Key changes:
Critical issue:
Confidence Score: 2/5
Important Files Changed
Sequence DiagramsequenceDiagram
participant Page as Site/Function Page
participant Card as DeploymentCard/SiteCard
participant DS as DeploymentSource
participant API as VCS API
Page->>Card: Pass deployment + resource + region + project
Card->>DS: Pass all props
Note over DS: onMount()
DS->>DS: Check if resource has installationId & providerRepositoryId
alt Has required data
DS->>API: getRepository(installationId, providerRepositoryId)
API-->>DS: { authorized: boolean }
DS->>DS: Set authorized state
alt authorized === false
DS->>DS: Render warning icon with tooltip
end
else Missing data
DS->>DS: authorized stays null (no warning shown)
end
Last reviewed commit: ba7e9ce |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
src/routes/(console)/project-[region]-[project]/functions/function-[function]/+page.svelte
Outdated
Show resolved
Hide resolved
src/routes/(console)/project-[region]-[project]/sites/site-[site]/+page.svelte
Outdated
Show resolved
Hide resolved
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
src/routes/(console)/project-[region]-[project]/sites/site-[site]/+page.ts (1)
103-110: Avoid swallowing all fetch errors silently.At Line 108 and Line 129, returning
nullfor every exception hides operational failures. Consider at least logging unexpected errors so production issues remain debuggable.Suggested pattern
async function loadDeployment({ @@ }): Promise<Models.Deployment | null> { try { return await sdk.forProject(region, project).sites.getDeployment({ siteId, deploymentId }); } catch (error) { + console.error('Failed to load site deployment', { siteId, deploymentId, error }); return null; } } @@ }): Promise<Models.ProviderRepository | null> { try { return await sdk.forProject(region, project).vcs.getRepository({ installationId, providerRepositoryId }); } catch (error) { + console.error('Failed to load site repository', { + installationId, + providerRepositoryId, + error + }); return null; } }Also applies to: 124-131
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/routes/`(console)/project-[region]-[project]/sites/site-[site]/+page.ts around lines 103 - 110, The catch block that wraps sdk.forProject(...).sites.getDeployment currently swallows all errors and returns null; change it to detect and handle expected "not found" responses (e.g., check error.status === 404 or SDK-specific NotFound error) returning null only in that case, but for all other exceptions log the error with context (include region, project, siteId, deploymentId) using the project's logger (or console.error if no logger available) and rethrow or return a meaningful error; update the catch around sdk.forProject(...).sites.getDeployment (and the similar block calling the same method) accordingly so operational failures are not silently hidden.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@src/routes/`(console)/project-[region]-[project]/functions/function-[function]/+page.ts:
- Around line 69-70: The load function currently mutates the module-scoped
queries store by calling queries.set(parsedQueries) after computing
parsedQueries with queryParamToMap(query || '[]'); remove that mutation from the
load function and instead return parsedQueries as part of the load data payload
(e.g., return { parsedQueries }). Then initialize the queries store on the
client: subscribe/ set it inside a client-only context such as a +page.svelte
onMount or a script tag with context="module" avoided—use onMount to call
queries.set(data.parsedQueries) or otherwise subscribe to load data; update the
+page.svelte to read the returned parsedQueries and set the queries store there
so SSR does not mutate shared module state.
In
`@src/routes/`(console)/project-[region]-[project]/sites/site-[site]/+page.svelte:
- Around line 39-44: Replace the generic link text "here" in the Link component
with descriptive copy that conveys the destination (e.g., "Manage GitHub App
installation" or "View GitHub installation settings") and update the Link
(component named Link, href constructed with
data.repository.providerInstallationId) to include an accessible label if
additional context is needed (aria-label) so screen readers see the destination
out of context; ensure the text mentions GitHub and installation/settings to
match the URL.
---
Nitpick comments:
In `@src/routes/`(console)/project-[region]-[project]/sites/site-[site]/+page.ts:
- Around line 103-110: The catch block that wraps
sdk.forProject(...).sites.getDeployment currently swallows all errors and
returns null; change it to detect and handle expected "not found" responses
(e.g., check error.status === 404 or SDK-specific NotFound error) returning null
only in that case, but for all other exceptions log the error with context
(include region, project, siteId, deploymentId) using the project's logger (or
console.error if no logger available) and rethrow or return a meaningful error;
update the catch around sdk.forProject(...).sites.getDeployment (and the similar
block calling the same method) accordingly so operational failures are not
silently hidden.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
src/routes/(console)/project-[region]-[project]/functions/function-[function]/+page.sveltesrc/routes/(console)/project-[region]-[project]/functions/function-[function]/+page.tssrc/routes/(console)/project-[region]-[project]/sites/site-[site]/+page.sveltesrc/routes/(console)/project-[region]-[project]/sites/site-[site]/+page.ts
src/routes/(console)/project-[region]-[project]/functions/function-[function]/+page.ts
Outdated
Show resolved
Hide resolved
src/routes/(console)/project-[region]-[project]/sites/site-[site]/+page.svelte
Outdated
Show resolved
Hide resolved
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/lib/components/git/deploymentSource.svelte (1)
29-41: Optional: Add explicit type toauthorizedfor consistency.TypeScript strict null checks are currently disabled in this project, so the current declaration works without issues. However, for consistency with similar state declarations elsewhere (e.g.,
multiSelectTable.svelte), consider explicitly typing the state:♻️ Proposed fix
- let authorized = $state(null); + let authorized = $state<boolean | null>(null);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/components/git/deploymentSource.svelte` around lines 29 - 41, The local state variable authorized is declared without an explicit type; update the declaration of authorized to include an explicit type (matching other components like multiSelectTable.svelte) such as boolean | null (or the specific Writable/State type you use) so its intended shape is clear and consistent, leaving the existing initializer ($state(null)) and keeping loadAuthorized and its assignment (authorized = repository.authorized) unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src/lib/components/git/deploymentSource.svelte`:
- Around line 29-41: The local state variable authorized is declared without an
explicit type; update the declaration of authorized to include an explicit type
(matching other components like multiSelectTable.svelte) such as boolean | null
(or the specific Writable/State type you use) so its intended shape is clear and
consistent, leaving the existing initializer ($state(null)) and keeping
loadAuthorized and its assignment (authorized = repository.authorized)
unchanged.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
src/lib/components/git/deploymentSource.sveltesrc/routes/(console)/project-[region]-[project]/functions/function-[function]/(components)/deploymentCard.sveltesrc/routes/(console)/project-[region]-[project]/sites/(components)/siteCard.sveltesrc/routes/(console)/project-[region]-[project]/sites/site-[site]/+page.svelte
src/routes/(console)/project-[region]-[project]/sites/(components)/siteCard.svelte
Outdated
Show resolved
Hide resolved
Meldiron
reviewed
Mar 2, 2026
Meldiron
reviewed
Mar 2, 2026
| <Tooltip> | ||
| <Icon icon={IconExclamation} size="s" color="--bgcolor-warning" /> | ||
| <span slot="tooltip"> | ||
| Integration not authorized for auto deployments.<br /> |
Contributor
There was a problem hiding this comment.
Lets consider CTA, dont use "here"
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/lib/components/git/deploymentSource.svelte`:
- Around line 30-44: The template is calling loadAuthorized() inside a {`#await`
...} which recreates the Promise on every render; move the auth check out of the
template and invoke loadAuthorized imperatively only when inputs change (e.g.,
in an onMount and/or a reactive statement that watches resource.installationId,
resource.providerRepositoryId, region, and project) so it only calls
sdk.forProject(...).vcs.getRepository once per actual dependency change; keep
the function loadAuthorized and its try/catch as-is but stop calling it from the
template and ensure authorized is updated only by that controlled invocation.
In
`@src/routes/`(console)/project-[region]-[project]/sites/(components)/siteCard.svelte:
- Around line 32-39: The SiteCard currently declares the prop "site" as optional
but the VCS warning (which relies on DeploymentSource needing
resource.installationId and resource.providerRepositoryId) requires those
identifiers; either make "site" required by changing the prop type from site?:
Models.Site to site: Models.Site in SiteCard (and also update the other prop
declaration block around lines 213-217) and update the two callers in
sites/site-[site]/deployments/.../+page.svelte and
sites/create-site/finish/+page.svelte to pass the site object, OR implement a
fallback inside SiteCard to compute installationId/providerRepositoryId from
other available data (e.g., deployment.resource or proxyRuleList) and use that
when site is undefined; reference the prop declaration in siteCard.svelte and
ensure all callsites that previously omitted site are updated if you choose the
required-route.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 194dfd33-46f9-476c-9fb4-547ded63a6c6
📒 Files selected for processing (2)
src/lib/components/git/deploymentSource.sveltesrc/routes/(console)/project-[region]-[project]/sites/(components)/siteCard.svelte
src/routes/(console)/project-[region]-[project]/sites/(components)/siteCard.svelte
Show resolved
Hide resolved
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Towards SER-1129
Summary by CodeRabbit
New Features
Improvements
Chores