Conversation
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📝 WalkthroughWalkthroughAdds Cloud Workspaces end-to-end: DB migrations and schema, TRPC cloudWorkspace router, web dashboard pages/components for listing/creating workspaces and detail, desktop UI to link/unlink local workspaces to cloud, and git helpers for remote detection and parsing. Changes
Sequence Diagram(s)sequenceDiagram
participant Desktop as Desktop UI
participant Web as Web UI
participant TRPCCloud as trpc.cloudWorkspace
participant TRPCLocal as trpc.workspaces.status
participant DB as Database
Desktop->>TRPCCloud: create({orgId, repoOwner, repoName, repoUrl, name, branch})
TRPCCloud->>DB: transaction -> find or create repository
DB-->>TRPCCloud: repository
TRPCCloud->>DB: insert cloud_workspaces(repositoryId, orgId, name, branch)
DB-->>TRPCCloud: cloudWorkspace
TRPCCloud-->>Desktop: {cloudWorkspace, repository}
Desktop->>TRPCLocal: linkToCloud(localWorkspaceId, cloudWorkspaceId)
TRPCLocal->>DB: update workspaces set cloud_workspace_id = cloudWorkspaceId
DB-->>TRPCLocal: updated workspace
TRPCLocal-->>Desktop: success
Web->>TRPCCloud: cloudWorkspace.all / byOrganization / byId
TRPCCloud->>DB: query cloud_workspaces (+ repo/org)
DB-->>TRPCCloud: results
TRPCCloud-->>Web: results
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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 |
🧹 Preview Cleanup CompleteThe following preview resources have been cleaned up:
Thank you for your contribution! 🎉 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@apps/web/src/app/`(dashboard)/cloud/new/page.tsx:
- Around line 46-54: The parseGitHubUrl function's regex can produce false
positives for URLs like https://example.com/github.com/owner/repo; update
parseGitHubUrl to parse using the URL constructor, validate the hostname is
exactly "github.com" or "www.github.com" (reject other hosts), then extract
owner and repo from the pathname segments, strip a trailing ".git" from the
repo, and return null on any parse/validation failure; ensure the function still
returns { owner, name } | null.
In `@packages/trpc/src/router/cloudWorkspace/cloudWorkspace.ts`:
- Around line 88-100: After inserting the cloud workspace with const
[cloudWorkspace] = await tx.insert(cloudWorkspaces).values({...}).returning(),
add a null/undefined check similar to the repository check and throw an error if
creation failed (e.g., throw new TRPCError({ code: 'INTERNAL', message: 'Failed
to create cloud workspace' })) so you never return an undefined cloudWorkspace;
reference the cloudWorkspace variable, the
tx.insert(cloudWorkspaces).values(...).returning() call, and the surrounding
return { cloudWorkspace, repository } to locate where to add the check.
🧹 Nitpick comments (5)
apps/desktop/src/lib/trpc/routers/workspaces/utils/git.ts (1)
649-659: Log origin-remote lookup failures instead of swallowing.The empty catch hides Git errors (e.g., missing git, repo access issues). Please log with a context prefix and then return null so the UI still degrades gracefully. As per coding guidelines, avoid silently swallowing errors.
♻️ Proposed change
export async function getOriginRemoteUrl( repoPath: string, ): Promise<string | null> { try { const git = simpleGit(repoPath); const remotes = await git.getRemotes(true); const origin = remotes.find((r) => r.name === "origin"); return origin?.refs?.fetch ?? origin?.refs?.push ?? null; - } catch { + } catch (error) { + console.warn( + `[git/get-origin-remote] Failed to read origin remote for ${repoPath}: ${ + error instanceof Error ? error.message : String(error) + }`, + ); return null; } }apps/desktop/src/renderer/screens/main/components/TopBar/CloudWorkspaceButton.tsx (1)
33-36: Log repo-info query failures for visibility.If
getRepoInfofails, the button disappears with no diagnostics. Add anonErrorhandler to log the failure with a prefixed context. As per coding guidelines, avoid silently swallowing errors.♻️ Proposed change
- const { data: repoInfo } = electronTrpc.workspaces.getRepoInfo.useQuery( - { id: workspaceId }, - { enabled: !cloudWorkspaceId }, - ); + const { data: repoInfo } = electronTrpc.workspaces.getRepoInfo.useQuery( + { id: workspaceId }, + { + enabled: !cloudWorkspaceId, + onError: (err) => { + console.error( + "[cloud-workspace/get-repo-info] Failed to load repo info:", + err, + ); + }, + }, + );packages/trpc/src/router/cloudWorkspace/cloudWorkspace.ts (3)
77-77: Extract hardcoded branch name to a named constant.The hardcoded
"main"string should be extracted to a named constant at the module level for maintainability and consistency. As per coding guidelines, magic values should be extracted to named constants.♻️ Proposed refactor
+const DEFAULT_BRANCH = "main"; + export const cloudWorkspaceRouter = {Then update line 77:
- defaultBranch: "main", + defaultBranch: DEFAULT_BRANCH,
9-17: Consider adding pagination for theallquery.This query returns all cloud workspaces without any limit. As the dataset grows, this could cause performance issues. Consider adding pagination support.
♻️ Example pagination pattern
- all: protectedProcedure.query(() => { - return db.query.cloudWorkspaces.findMany({ - orderBy: desc(cloudWorkspaces.createdAt), - with: { - organization: true, - repository: true, - }, - }); - }), + all: protectedProcedure + .input( + z.object({ + limit: z.number().min(1).max(100).default(50), + offset: z.number().min(0).default(0), + }).optional(), + ) + .query(({ input }) => { + const { limit = 50, offset = 0 } = input ?? {}; + return db.query.cloudWorkspaces.findMany({ + orderBy: desc(cloudWorkspaces.createdAt), + limit, + offset, + with: { + organization: true, + repository: true, + }, + }); + }),
66-79: Consider handling potential slug collisions.The slug is generated as
${repoOwner}-${repoName}.toLowerCase()which works for the find-or-create pattern here. However, ifrepoOwnerorrepoNamecontain special characters or dashes, the slug could become ambiguous (e.g.,foo-bar+bazvsfoo+bar-bazboth producefoo-bar-baz`).Since the unique constraint on repositories likely uses
(organizationId, repoOwner, repoName)rather than slug, this may not cause data integrity issues, but slugs could be confusing for URL routing.
| const [cloudWorkspace] = await tx | ||
| .insert(cloudWorkspaces) | ||
| .values({ | ||
| organizationId, | ||
| repositoryId: repository.id, | ||
| name, | ||
| branch, | ||
| }) | ||
| .returning(); | ||
|
|
||
| return { | ||
| cloudWorkspace, | ||
| repository, |
There was a problem hiding this comment.
Missing error check after cloud workspace creation.
The repository creation (lines 81-83) includes a null check and throws an error if it fails, but the cloud workspace creation lacks the same safeguard. If the insert fails, cloudWorkspace will be undefined and silently returned in the response object.
🐛 Proposed fix to add error handling
// Create the cloud workspace
const [cloudWorkspace] = await tx
.insert(cloudWorkspaces)
.values({
organizationId,
repositoryId: repository.id,
name,
branch,
})
.returning();
+ if (!cloudWorkspace) {
+ throw new Error("Failed to create cloud workspace");
+ }
+
return {
cloudWorkspace,
repository,
};🤖 Prompt for AI Agents
In `@packages/trpc/src/router/cloudWorkspace/cloudWorkspace.ts` around lines 88 -
100, After inserting the cloud workspace with const [cloudWorkspace] = await
tx.insert(cloudWorkspaces).values({...}).returning(), add a null/undefined check
similar to the repository check and throw an error if creation failed (e.g.,
throw new TRPCError({ code: 'INTERNAL', message: 'Failed to create cloud
workspace' })) so you never return an undefined cloudWorkspace; reference the
cloudWorkspace variable, the tx.insert(cloudWorkspaces).values(...).returning()
call, and the surrounding return { cloudWorkspace, repository } to locate where
to add the check.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@apps/web/src/app/`(dashboard)/cloud/workspace/[id]/page.tsx:
- Around line 11-12: params.id is being unsafely cast to string. Change the use
of useParams()/params.id so you validate its type: check for undefined and
handle string[] by selecting the first element (or reject arrays), and handle
missing/invalid cases explicitly (e.g., render a NotFound/InvalidId UI, throw,
or redirect). Update the declaration currently written as "const id = params.id
as string" to perform these runtime checks before using id in this module/page.
🧹 Nitpick comments (3)
apps/web/src/app/(dashboard)/cloud/components/WorkspaceList/WorkspaceList.tsx (1)
26-32: Error details are not logged for debugging.Per coding guidelines, errors should not be swallowed silently. While the user sees a friendly message, the actual error is lost, making debugging production issues difficult.
Consider logging the error with context:
Suggested improvement
+import { useEffect } from "react"; const { data: workspaces, isLoading, isError, + error, } = useQuery(trpc.cloudWorkspace.all.queryOptions()); +useEffect(() => { + if (isError && error) { + console.error("[cloud/WorkspaceList] Failed to fetch workspaces:", error); + } +}, [isError, error]);apps/web/src/app/(dashboard)/cloud/workspace/[id]/page.tsx (2)
30-45: Error details are not logged for debugging.Same as in
WorkspaceList, the error is shown to the user but not logged. Consider capturing and logging the error for observability.Suggested improvement
const { data: workspace, isLoading, isError, + error, } = useQuery(trpc.cloudWorkspace.byId.queryOptions(id)); +useEffect(() => { + if (isError && error) { + console.error("[cloud/WorkspaceDetail] Failed to fetch workspace:", { id, error }); + } +}, [isError, error, id]);
106-115: "View on GitHub" assumes GitHub hosting.The button text hardcodes "GitHub" but
repoUrlcould point to GitLab, Bitbucket, or other providers. Consider a more generic label like "View Repository" or derive the provider name from the URL.
| const params = useParams(); | ||
| const id = params.id as string; |
There was a problem hiding this comment.
Unsafe type assertion on route parameter.
params.id in Next.js App Router can be string | string[] | undefined. The direct cast as string bypasses type safety and could cause runtime issues if the param is missing or malformed.
Suggested fix with validation
const params = useParams();
-const id = params.id as string;
+const id = typeof params.id === "string" ? params.id : "";Alternatively, handle the invalid case explicitly:
const params = useParams();
-const id = params.id as string;
+const id = typeof params.id === "string" ? params.id : null;
+
+if (!id) {
+ return (
+ <div className="py-8 text-center text-muted-foreground">
+ Invalid workspace ID.
+ </div>
+ );
+}🤖 Prompt for AI Agents
In `@apps/web/src/app/`(dashboard)/cloud/workspace/[id]/page.tsx around lines 11 -
12, params.id is being unsafely cast to string. Change the use of
useParams()/params.id so you validate its type: check for undefined and handle
string[] by selecting the first element (or reject arrays), and handle
missing/invalid cases explicitly (e.g., render a NotFound/InvalidId UI, throw,
or redirect). Update the declaration currently written as "const id = params.id
as string" to perform these runtime checks before using id in this module/page.
Hide cloud workspace features behind the `cloud-workspace-enabled` PostHog feature flag. When disabled, the CloudWorkspaceButton is hidden from the TopBar and workspace list items show regular folder icons instead of cloud icons.
This reverts commit 1803365.
Add ENABLE_CLOUD_WORKSPACES constant (defaults to false) to gate cloud workspace features until ready: - CloudWorkspaceButton in TopBar - Cloud icons in WorkspaceListItem - Cloud tab in NewWorkspaceModal Also fixes lint warnings in CloudWorkspaceButton.
Kitenite
force-pushed
the
cloud-ws-plan-decomposition
branch
from
ab3481e to
314a472
Compare
…sition # Conflicts: # apps/desktop/src/lib/trpc/routers/workspaces/procedures/status.ts # apps/desktop/src/renderer/components/NewWorkspaceModal/NewWorkspaceModal.tsx # packages/db/drizzle/meta/0013_snapshot.json # packages/db/drizzle/meta/_journal.json # packages/db/src/schema/relations.ts # packages/db/src/schema/schema.ts # packages/local-db/drizzle/meta/0013_snapshot.json # packages/local-db/drizzle/meta/_journal.json # packages/trpc/src/root.ts
1 participant
Description
Related Issues
Type of Change
Testing
Screenshots (if applicable)
Additional Notes
Summary by CodeRabbit
New Features
Improvements
✏️ Tip: You can customize this high-level summary in your review settings.