feat(desktop): add non worktree tabs for different branches that arent worktrees#359
feat(desktop): add non worktree tabs for different branches that arent worktrees#359
Conversation
WalkthroughAdds branch-type workspaces and related plumbing: schema and DB migrations, git branch utilities and safety checks, workspace path resolution, TRPC routes (createBranchWorkspace/getBranches/killByWorkspaceId), project upsert changes to create/activate a main workspace, and renderer UI/hooks for branch discovery and creation. Changes
Sequence DiagramsequenceDiagram
participant User
participant UI as Renderer UI
participant TRPC as TRPC Router
participant Git as Git Utilities
participant DB as Database
participant Terminal as TerminalManager
User->>UI: Open "Add existing branch..." dialog
UI->>TRPC: getBranches({ projectId })
TRPC->>Git: listBranches(mainRepoPath)
Git-->>TRPC: { local, remote }
TRPC-->>UI: branches list
User->>UI: Select branch & "Create Workspace"
UI->>TRPC: createBranchWorkspace({ projectId, branch })
TRPC->>Git: safeCheckoutBranch(mainRepoPath, branch)
Git-->>TRPC: checkout verified
TRPC->>DB: insert workspace (type: "branch", branch), update project.lastActiveWorkspaceId/tabOrder
DB-->>TRPC: workspace created
TRPC-->>UI: { workspace, worktreePath?, projectId }
UI->>UI: invalidate queries, add workspace tab
TRPC->>Terminal: refreshPromptsForWorkspace(workspace.id)
Terminal-->>TRPC: refreshed count
UI-->>User: new branch workspace active
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes
Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (1 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
apps/desktop/src/lib/trpc/routers/terminal/terminal.ts (1)
21-21: Update comment to reflect workspace path generalization.The comment still references "worktree path" but should now reference "workspace path" since it applies to both worktree and branch workspaces.
Apply this diff:
- * - SUPERSET_WORKSPACE_PATH: The worktree path (used by setup/teardown scripts) + * - SUPERSET_WORKSPACE_PATH: The workspace path (used by setup/teardown scripts)apps/desktop/src/lib/trpc/routers/workspaces/workspaces.ts (1)
763-794: Inconsistent caching: Branch workspaces don't cache GitHub status.For worktree-type workspaces, the GitHub status is cached in
worktree.githubStatus(lines 784-791), but branch workspaces fetch fresh data without caching. This could cause unnecessary API calls on repeated queries.Consider storing GitHub status at the workspace level for branch workspaces, or document that branch workspaces intentionally skip caching.
🧹 Nitpick comments (6)
apps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/WorkspaceHoverCard/WorkspaceHoverCard.tsx (1)
37-37: Consider explicit undefined handling for type safety.The optional chaining handles
worktreeInfobeing undefined, but the comparison implicitly treatsundefinedas not equal to"branch", which works but may be unclear. Consider making this explicit:- const isBranchType = worktreeInfo?.workspaceType === "branch"; + const isBranchType = worktreeInfo?.workspaceType === "branch" ?? false;This makes the default behavior explicit when
worktreeInfois undefined.apps/desktop/src/main/lib/db/schemas.ts (1)
58-60: Consider adding runtime validation for workspace type invariants.The schema makes
worktreeIdoptional but doesn't enforce thattype="worktree"requires aworktreeId. This could lead to runtime errors when code assumes worktree-type workspaces have aworktreeId.Consider adding a Zod schema or runtime validation to enforce this constraint, or document this invariant clearly in comments.
Example validation approach:
// Add a discriminated union type for stronger type safety export type Workspace = | { id: string; projectId: string; type: "worktree"; worktreeId: string; // Required for worktree type branch: string; name: string; tabOrder: number; createdAt: number; updatedAt: number; lastOpenedAt: number; } | { id: string; projectId: string; type: "branch"; worktreeId?: never; // Explicitly exclude for branch type branch: string; name: string; tabOrder: number; createdAt: number; updatedAt: number; lastOpenedAt: number; };apps/desktop/src/lib/trpc/routers/workspaces/utils/git.ts (1)
448-476: Add clarifying comments and consider multi-remote support.The implementation is solid, but a few improvements would enhance maintainability:
Document the symbolic ref filter: Line 472's
!b.includes("->")filters symbolic refs likeorigin/HEAD -> origin/main. This deserves a comment.Document the origin-only assumption: The function only handles branches from the
originremote. If repos with multiple remotes are supported, this should be documented or extended.Apply these clarifying comments:
// Get remote branches (strip "origin/" prefix) const remoteResult = await git.branch(["-r"]); const remote = remoteResult.all + // Filter symbolic refs (e.g., "origin/HEAD -> origin/main") and non-origin remotes .filter((b) => b.startsWith("origin/") && !b.includes("->")) .map((b) => b.replace("origin/", ""));apps/desktop/src/lib/trpc/routers/projects/projects.ts (1)
86-108: Good auto-activation logic, but consider edge case for concurrent project creation.The tabOrder calculation iterates existing active projects to find max tabOrder. In concurrent scenarios (rare but possible), two simultaneous project creations could receive the same tabOrder. This is a minor concern given typical usage patterns.
apps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/AddBranchDialog/AddBranchDialog.tsx (1)
96-102: Consider using theInputcomponent from@superset/uifor consistency.The rest of the codebase (including
WorkspaceItem.tsx) uses@superset/ui/inputfor form inputs. Using a plain HTML input here creates visual inconsistency.+import { Input } from "@superset/ui/input"; ... - <input - type="text" + <Input value={search} onChange={(e) => setSearch(e.target.value)} placeholder="Search branches..." - className="w-full rounded-md border border-border bg-muted/50 pl-9 pr-3 py-2 text-sm outline-none focus:border-primary focus:bg-background" + className="w-full pl-9" />apps/desktop/src/lib/trpc/routers/workspaces/workspaces.ts (1)
618-626: Consider user-friendly error handling for branch checkout failures.When switching to a branch workspace,
checkoutBranchcan fail if the main repo has uncommitted changes. The raw git error will propagate to the user.// For branch workspaces, checkout the branch in the main repo if (workspace.type === "branch" && workspace.branch) { const project = db.data.projects.find( (p) => p.id === workspace.projectId, ); if (project) { - await checkoutBranch(project.mainRepoPath, workspace.branch); + try { + await checkoutBranch(project.mainRepoPath, workspace.branch); + } catch (error) { + const msg = error instanceof Error ? error.message : String(error); + throw new Error( + `Cannot switch to branch "${workspace.branch}": ${msg}. Commit or stash your changes first.`, + ); + } } }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (14)
apps/desktop/src/lib/trpc/routers/projects/projects.ts(6 hunks)apps/desktop/src/lib/trpc/routers/terminal/terminal.ts(4 hunks)apps/desktop/src/lib/trpc/routers/workspaces/utils/git.ts(1 hunks)apps/desktop/src/lib/trpc/routers/workspaces/utils/worktree.ts(1 hunks)apps/desktop/src/lib/trpc/routers/workspaces/workspaces.ts(13 hunks)apps/desktop/src/main/lib/db/schemas.ts(1 hunks)apps/desktop/src/renderer/react-query/workspaces/index.ts(1 hunks)apps/desktop/src/renderer/react-query/workspaces/useCreateBranchWorkspace.ts(1 hunks)apps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/AddBranchDialog/AddBranchDialog.tsx(1 hunks)apps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/AddBranchDialog/index.ts(1 hunks)apps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/WorkspaceGroup.tsx(2 hunks)apps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/WorkspaceGroupContextMenu.tsx(3 hunks)apps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/WorkspaceHoverCard/WorkspaceHoverCard.tsx(2 hunks)apps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/WorkspaceItem.tsx(4 hunks)
🧰 Additional context used
📓 Path-based instructions (6)
apps/desktop/**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (apps/desktop/AGENTS.md)
For Electron interprocess communication, ALWAYS use tRPC as defined in
src/lib/trpc
Files:
apps/desktop/src/lib/trpc/routers/workspaces/utils/git.tsapps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/AddBranchDialog/index.tsapps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/WorkspaceItem.tsxapps/desktop/src/renderer/react-query/workspaces/useCreateBranchWorkspace.tsapps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/WorkspaceGroupContextMenu.tsxapps/desktop/src/renderer/react-query/workspaces/index.tsapps/desktop/src/lib/trpc/routers/workspaces/utils/worktree.tsapps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/WorkspaceGroup.tsxapps/desktop/src/main/lib/db/schemas.tsapps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/AddBranchDialog/AddBranchDialog.tsxapps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/WorkspaceHoverCard/WorkspaceHoverCard.tsxapps/desktop/src/lib/trpc/routers/workspaces/workspaces.tsapps/desktop/src/lib/trpc/routers/terminal/terminal.tsapps/desktop/src/lib/trpc/routers/projects/projects.ts
apps/desktop/**/*.{ts,tsx}
📄 CodeRabbit inference engine (apps/desktop/AGENTS.md)
apps/desktop/**/*.{ts,tsx}: Please use alias as defined intsconfig.jsonwhen possible
Prefer zustand for state management if it makes sense. Do not use effect unless absolutely necessary
Files:
apps/desktop/src/lib/trpc/routers/workspaces/utils/git.tsapps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/AddBranchDialog/index.tsapps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/WorkspaceItem.tsxapps/desktop/src/renderer/react-query/workspaces/useCreateBranchWorkspace.tsapps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/WorkspaceGroupContextMenu.tsxapps/desktop/src/renderer/react-query/workspaces/index.tsapps/desktop/src/lib/trpc/routers/workspaces/utils/worktree.tsapps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/WorkspaceGroup.tsxapps/desktop/src/main/lib/db/schemas.tsapps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/AddBranchDialog/AddBranchDialog.tsxapps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/WorkspaceHoverCard/WorkspaceHoverCard.tsxapps/desktop/src/lib/trpc/routers/workspaces/workspaces.tsapps/desktop/src/lib/trpc/routers/terminal/terminal.tsapps/desktop/src/lib/trpc/routers/projects/projects.ts
**/*.{ts,tsx,js,jsx,json}
📄 CodeRabbit inference engine (AGENTS.md)
Use Biome for code formatting and linting, running at root level for speed
Files:
apps/desktop/src/lib/trpc/routers/workspaces/utils/git.tsapps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/AddBranchDialog/index.tsapps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/WorkspaceItem.tsxapps/desktop/src/renderer/react-query/workspaces/useCreateBranchWorkspace.tsapps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/WorkspaceGroupContextMenu.tsxapps/desktop/src/renderer/react-query/workspaces/index.tsapps/desktop/src/lib/trpc/routers/workspaces/utils/worktree.tsapps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/WorkspaceGroup.tsxapps/desktop/src/main/lib/db/schemas.tsapps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/AddBranchDialog/AddBranchDialog.tsxapps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/WorkspaceHoverCard/WorkspaceHoverCard.tsxapps/desktop/src/lib/trpc/routers/workspaces/workspaces.tsapps/desktop/src/lib/trpc/routers/terminal/terminal.tsapps/desktop/src/lib/trpc/routers/projects/projects.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Avoid
anytype and prioritize type safety in TypeScript code
Files:
apps/desktop/src/lib/trpc/routers/workspaces/utils/git.tsapps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/AddBranchDialog/index.tsapps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/WorkspaceItem.tsxapps/desktop/src/renderer/react-query/workspaces/useCreateBranchWorkspace.tsapps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/WorkspaceGroupContextMenu.tsxapps/desktop/src/renderer/react-query/workspaces/index.tsapps/desktop/src/lib/trpc/routers/workspaces/utils/worktree.tsapps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/WorkspaceGroup.tsxapps/desktop/src/main/lib/db/schemas.tsapps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/AddBranchDialog/AddBranchDialog.tsxapps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/WorkspaceHoverCard/WorkspaceHoverCard.tsxapps/desktop/src/lib/trpc/routers/workspaces/workspaces.tsapps/desktop/src/lib/trpc/routers/terminal/terminal.tsapps/desktop/src/lib/trpc/routers/projects/projects.ts
**/components/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/components/**/*.{ts,tsx}: Structure project folders as one folder per component with PascalCase naming (ComponentName/ComponentName.tsx + index.ts barrel export)
Co-locate component dependencies (utils, hooks, constants, config, tests, stories) next to the file using them
Use one component per file (no multi-component files)
Files:
apps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/AddBranchDialog/index.tsapps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/WorkspaceItem.tsxapps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/WorkspaceGroupContextMenu.tsxapps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/WorkspaceGroup.tsxapps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/AddBranchDialog/AddBranchDialog.tsxapps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/WorkspaceHoverCard/WorkspaceHoverCard.tsx
apps/desktop/src/renderer/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Call IPC methods from renderer process using window.ipcRenderer.invoke with type-safe object parameters
Files:
apps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/AddBranchDialog/index.tsapps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/WorkspaceItem.tsxapps/desktop/src/renderer/react-query/workspaces/useCreateBranchWorkspace.tsapps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/WorkspaceGroupContextMenu.tsxapps/desktop/src/renderer/react-query/workspaces/index.tsapps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/WorkspaceGroup.tsxapps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/AddBranchDialog/AddBranchDialog.tsxapps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/WorkspaceHoverCard/WorkspaceHoverCard.tsx
🧠 Learnings (4)
📚 Learning: 2025-12-12T05:45:09.672Z
Learnt from: CR
Repo: superset-sh/superset PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-12T05:45:09.672Z
Learning: Applies to **/components/**/*.{ts,tsx} : Structure project folders as one folder per component with PascalCase naming (ComponentName/ComponentName.tsx + index.ts barrel export)
Applied to files:
apps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/AddBranchDialog/index.ts
📚 Learning: 2025-11-24T21:33:13.267Z
Learnt from: CR
Repo: superset-sh/superset PR: 0
File: apps/desktop/AGENTS.md:0-0
Timestamp: 2025-11-24T21:33:13.267Z
Learning: Applies to apps/desktop/**/*.{ts,tsx} : Please use alias as defined in `tsconfig.json` when possible
Applied to files:
apps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/AddBranchDialog/index.tsapps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/WorkspaceItem.tsxapps/desktop/src/renderer/react-query/workspaces/index.tsapps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/WorkspaceGroup.tsxapps/desktop/src/lib/trpc/routers/terminal/terminal.ts
📚 Learning: 2025-11-24T21:33:13.267Z
Learnt from: CR
Repo: superset-sh/superset PR: 0
File: apps/desktop/AGENTS.md:0-0
Timestamp: 2025-11-24T21:33:13.267Z
Learning: Applies to apps/desktop/**/*.{ts,tsx,js,jsx} : For Electron interprocess communication, ALWAYS use tRPC as defined in `src/lib/trpc`
Applied to files:
apps/desktop/src/lib/trpc/routers/terminal/terminal.tsapps/desktop/src/lib/trpc/routers/projects/projects.ts
📚 Learning: 2025-12-12T05:45:09.672Z
Learnt from: CR
Repo: superset-sh/superset PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-12T05:45:09.672Z
Learning: Applies to apps/desktop/src/renderer/**/*.{ts,tsx,js,jsx} apps/desktop/src/lib/**/*.{ts,tsx,js,jsx} apps/desktop/src/shared/**/*.{ts,tsx,js,tsx} : Never import Node.js modules in renderer process or shared code, only in main process
Applied to files:
apps/desktop/src/lib/trpc/routers/projects/projects.ts
🧬 Code graph analysis (6)
apps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/WorkspaceGroupContextMenu.tsx (3)
packages/ui/src/components/ui/context-menu.tsx (4)
ContextMenu(237-237)ContextMenuTrigger(238-238)ContextMenuContent(239-239)ContextMenuSeparator(244-244)apps/desktop/src/shared/constants/project-colors.ts (1)
PROJECT_COLORS(1-10)apps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/AddBranchDialog/AddBranchDialog.tsx (1)
AddBranchDialog(23-149)
apps/desktop/src/lib/trpc/routers/workspaces/utils/worktree.ts (1)
apps/desktop/src/main/lib/db/schemas.ts (1)
Workspace(55-66)
apps/desktop/src/main/lib/db/schemas.ts (1)
apps/desktop/src/shared/types.ts (1)
Workspace(70-82)
apps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/AddBranchDialog/AddBranchDialog.tsx (3)
apps/desktop/src/renderer/react-query/workspaces/useCreateBranchWorkspace.ts (1)
useCreateBranchWorkspace(9-30)packages/ui/src/components/ui/sonner.tsx (1)
toast(40-40)packages/ui/src/components/ui/dialog.tsx (6)
Dialog(134-134)DialogContent(136-136)DialogHeader(139-139)DialogTitle(142-142)DialogDescription(137-137)DialogFooter(138-138)
apps/desktop/src/lib/trpc/routers/workspaces/workspaces.ts (4)
apps/desktop/src/lib/trpc/routers/workspaces/utils/git.ts (4)
checkoutBranch(483-489)listBranches(453-476)worktreeExists(264-281)removeWorktree(225-246)apps/desktop/src/lib/trpc/routers/workspaces/utils/teardown.ts (1)
runTeardown(37-75)apps/desktop/src/lib/trpc/routers/workspaces/utils/github/github.ts (1)
fetchGitHubPRStatus(22-67)apps/desktop/src/lib/trpc/routers/workspaces/utils/github/index.ts (1)
fetchGitHubPRStatus(1-1)
apps/desktop/src/lib/trpc/routers/terminal/terminal.ts (2)
apps/desktop/src/lib/trpc/routers/workspaces/utils/worktree.ts (1)
getWorkspacePath(18-28)apps/desktop/src/lib/trpc/routers/terminal/utils/resolve-cwd.ts (1)
resolveCwd(14-54)
⏰ 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). (6)
- GitHub Check: Deploy Marketing
- GitHub Check: Deploy API
- GitHub Check: Deploy Docs
- GitHub Check: Deploy Admin
- GitHub Check: Deploy Web
- GitHub Check: Build
🔇 Additional comments (31)
apps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/AddBranchDialog/index.ts (1)
1-1: LGTM!Barrel export follows the component structure guidelines correctly.
apps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/WorkspaceHoverCard/WorkspaceHoverCard.tsx (1)
81-86: LGTM!The branch workspace indicator is clearly presented and appropriately styled.
apps/desktop/src/renderer/react-query/workspaces/index.ts (1)
1-1: LGTM!Export follows the established pattern for workspace hooks.
apps/desktop/src/lib/trpc/routers/terminal/terminal.ts (2)
8-8: LGTM!The import change from
getWorktreePathtogetWorkspacePathcorrectly reflects the refactoring to support both worktree and branch workspaces.
50-55: LGTM!The refactoring from
worktreePathtoworkspacePathis correctly applied and integrates well with the newgetWorkspacePathutility.apps/desktop/src/lib/trpc/routers/workspaces/utils/git.ts (1)
478-489: LGTM!The
checkoutBranchfunction is straightforward and correctly delegates to simple-git's checkout API.apps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/WorkspaceGroup.tsx (2)
10-11: LGTM!The local
Workspaceinterface correctly reflects the schema changes with the newtypeandbranchfields.
78-78: LGTM!Correctly passes the workspace type to
WorkspaceItemfor proper rendering differentiation.apps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/WorkspaceGroupContextMenu.tsx (4)
9-9: LGTM!Imports are correctly added for the new branch workspace creation feature.
Also applies to: 12-12
28-28: LGTM!Dialog state management follows React best practices with a boolean flag.
152-161: LGTM!The new menu item is well-integrated with clear labeling and appropriate iconography. The handler correctly toggles the dialog state.
178-183: LGTM!The
AddBranchDialogis correctly rendered outside theContextMenuto avoid portal/focus issues. Props are properly passed for controlled dialog behavior.apps/desktop/src/main/lib/db/schemas.ts (1)
53-66: Verify type consistency across the codebase.The schema changes introduce a new
Workspacestructure, but previous context indicates a differentWorkspaceinterface inapps/desktop/src/shared/types.tswith fields likeworktrees: Worktree[]andactiveWorktreeId. This suggests potential type conflicts.Please ensure:
- All
Workspacetype references are updated consistently- Any migration logic exists to handle the schema change
- The old
Workspaceinterface inshared/types.tsis either removed or renamed to avoid confusionapps/desktop/src/lib/trpc/routers/workspaces/utils/worktree.ts (2)
18-28: LGTM! Clean polymorphic path resolution.The function correctly handles both workspace types with clear branching logic. The deprecation notice on
getWorktreePathis helpful for guiding future refactoring.
33-37: LGTM!Clean convenience wrapper that properly handles the not-found case.
apps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/WorkspaceItem.tsx (2)
23-34: LGTM!Clean prop extension with a sensible default value that maintains backward compatibility.
204-206: LGTM!Good visual differentiation for branch-type workspaces. The icon is appropriately styled and positioned.
apps/desktop/src/renderer/react-query/workspaces/useCreateBranchWorkspace.ts (1)
1-30: LGTM!Well-structured mutation hook following established patterns. The cache invalidation and tab integration are properly handled.
apps/desktop/src/lib/trpc/routers/projects/projects.ts (2)
34-50: LGTM!Clean helper function that creates a properly structured branch workspace with all required fields.
114-179: LGTM! Robust URL parsing with good security considerations.The
extractRepoNameutility handles multiple URL formats, decodes percent-encoded characters, and validates against path traversal attacks viaSAFE_REPO_NAME_REGEX.apps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/AddBranchDialog/AddBranchDialog.tsx (2)
139-144: LGTM!Good UX with proper button disabling during pending state and when no branch is selected.
39-47: Remove the suggested normalization — remote branches are already normalized in the backend.The
listBranchesutility already strips theorigin/prefix from remote branches (seegit.tsline 514:.map((b) => b.replace("origin/", ""))), so both local and remote branches in the combined array use consistent naming. The current deduplication logic is correct and does not need modification.apps/desktop/src/lib/trpc/routers/workspaces/workspaces.ts (9)
9-26: LGTM!The new imports for
checkoutBranch,listBranches, andgetWorkspacePathare appropriate for the branch workspace functionality and follow the alias conventions defined in tsconfig.json.
110-121: LGTM!The workspace object now includes
typeandbranchfields, correctly usingas constfor type narrowing. This establishes the worktree workspace type properly.
237-246: LGTM!The
getBranchesprocedure is clean and delegates to the well-implementedlistBranchesutility which handles fetching and pruning.
276-314: LGTM!The type definitions correctly reflect the polymorphic workspace structure with optional
worktreeIdand requiredtypeandbranchfields. The switch togetWorkspacePath(workspace)properly abstracts the path resolution for both workspace types.
342-359: LGTM!The conditional worktree lookup correctly handles both workspace types. Branch workspaces will have
worktree: nullwhich is appropriate for the return type.
428-439: Verify: Branch workspace deletion doesn't warn about uncommitted changes in main repo.Branch workspaces return
hasChanges: falseandhasUnpushedCommits: falseunconditionally. While branch workspace deletion only removes the DB record (no worktree cleanup), users might still have uncommitted work on that branch in the main repo.Is this the intended UX? If the user deletes a branch workspace while the main repo is checked out to that branch with uncommitted changes, those changes persist but the workspace is removed.
524-570: LGTM!The delete procedure correctly differentiates between workspace types. Worktree cleanup (teardown scripts, git worktree removal) is appropriately skipped for branch workspaces, while DB cleanup applies to both.
689-698: LGTM!Branch workspaces return a static git status without rebase checks. This is reasonable since branch workspaces share the main repo and don't have isolated worktrees to rebase.
806-833: LGTM!The
getWorktreeInfoprocedure now correctly returns workspace-type-specific information with aworkspaceTypediscriminator. Branch workspaces appropriately returngitStatus: nullandgithubStatus: nullsince they don't have dedicated worktree tracking.
🚀 Preview Deployment🔗 Preview Links
Preview updates automatically with new commits |
Resolved conflict in WorkspaceGroupHeader.tsx - kept enhanced button styling (group flex, gap-1.5, rounded-md, active:brightness-95, scale-95 on drag)
- Auto-fixed formatting in db/index.ts, DeleteWorkspaceDialog, WorkspaceDropdown, WorkspaceItem - Replaced non-null assertion with proper null check in switchBranchWorkspace
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (5)
apps/desktop/src/lib/trpc/routers/workspaces/workspaces.ts (5)
236-241: Return value inconsistency: missinginitialCommandsfield.The
createprocedure (lines 150-155) returnsinitialCommands: setupConfig?.setup || null, butcreateBranchWorkspaceomits this field. If branch workspaces should also run setup commands, load and return the config:+ // Load setup configuration from the main repo + const setupConfig = loadSetupConfig(project.mainRepoPath); + return { workspace, + initialCommands: setupConfig?.setup || null, worktreePath: project.mainRepoPath, projectId: project.id, };If this is intentional (branch workspaces shouldn't run setup), consider adding a comment explaining why.
334-346: Consider extracting workspace shape to a shared type.The workspace shape with
worktreeId?,type, andbranchis used here and likely elsewhere. Extracting it to a shared type would improve maintainability and ensure consistency across the codebase.
486-497: Consider warning about uncommitted changes in branch workspaces.Returning
hasChanges: falseandhasUnpushedCommits: falsewithout checking the main repo could lead to data loss. When deleting a branch workspace, the user might have uncommitted changes inmainRepoPaththat aren't being surfaced.Consider checking the main repo status:
if (workspace.type === "branch") { + const project = db.data.projects.find( + (p) => p.id === workspace.projectId, + ); + let hasChanges = false; + let unpushedCommits = false; + if (project) { + [hasChanges, unpushedCommits] = await Promise.all([ + hasUncommittedChanges(project.mainRepoPath), + hasUnpushedCommits(project.mainRepoPath), + ]); + } return { canDelete: true, reason: null, workspace, warning: null, activeTerminalCount, - hasChanges: false, - hasUnpushedCommits: false, + hasChanges, + hasUnpushedCommits: unpushedCommits, }; }
767-804: Inconsistent error handling and persistence between branch and worktree paths.Two issues:
- Error handling inconsistency: For branch workspaces (line 770-774),
fetchDefaultBrancherrors are silently ignored. For worktree workspaces (line 803-804), errors propagate. Consider consistent handling:- // Fetch default branch to get latest - await fetchDefaultBranch(project.mainRepoPath, defaultBranch); + // Fetch default branch to get latest + try { + await fetchDefaultBranch(project.mainRepoPath, defaultBranch); + } catch { + // Ignore fetch errors (e.g., offline) + }
- Persistence inconsistency: Branch workspace
gitStatusis returned but not persisted anywhere, while worktreegitStatusis saved to the database (lines 818-824). If branch workspace status should be cached, consider where to store it.
847-857: Branch mismatch returns potentially incorrect PR status.When
currentBranch !== workspace.branch, the code logs a warning but still fetches and returns PR status for whatever branch HEAD is on. This could show PR information for the wrong branch, confusing users.Consider returning
nullor a specific error state when there's a mismatch:if (currentBranch !== workspace.branch) { console.warn( `[getGitHubStatus] Branch mismatch: workspace expects "${workspace.branch}" but HEAD is on "${currentBranch ?? "detached HEAD"}"`, ); - // Still fetch status but log warning - the PR status returned - // will be for whatever branch HEAD is actually on + // Don't return potentially misleading PR status + return null; } - - return fetchGitHubPRStatus(project.mainRepoPath);
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
apps/desktop/src/lib/trpc/routers/terminal/terminal.ts(5 hunks)apps/desktop/src/lib/trpc/routers/workspaces/utils/git.ts(1 hunks)apps/desktop/src/lib/trpc/routers/workspaces/workspaces.ts(13 hunks)apps/desktop/src/main/lib/db/index.ts(1 hunks)apps/desktop/src/main/lib/db/schemas.ts(1 hunks)apps/desktop/src/main/lib/terminal/manager.ts(1 hunks)apps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/DeleteWorkspaceDialog.tsx(5 hunks)apps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/WorkspaceDropdown.tsx(5 hunks)apps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/WorkspaceGroupHeader.tsx(4 hunks)apps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/WorkspaceItem.tsx(6 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/desktop/src/main/lib/db/schemas.ts
🧰 Additional context used
📓 Path-based instructions (6)
apps/desktop/**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (apps/desktop/AGENTS.md)
For Electron interprocess communication, ALWAYS use tRPC as defined in
src/lib/trpc
Files:
apps/desktop/src/main/lib/terminal/manager.tsapps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/DeleteWorkspaceDialog.tsxapps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/WorkspaceGroupHeader.tsxapps/desktop/src/lib/trpc/routers/workspaces/utils/git.tsapps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/WorkspaceItem.tsxapps/desktop/src/main/lib/db/index.tsapps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/WorkspaceDropdown.tsxapps/desktop/src/lib/trpc/routers/workspaces/workspaces.tsapps/desktop/src/lib/trpc/routers/terminal/terminal.ts
apps/desktop/**/*.{ts,tsx}
📄 CodeRabbit inference engine (apps/desktop/AGENTS.md)
apps/desktop/**/*.{ts,tsx}: Please use alias as defined intsconfig.jsonwhen possible
Prefer zustand for state management if it makes sense. Do not use effect unless absolutely necessary
Files:
apps/desktop/src/main/lib/terminal/manager.tsapps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/DeleteWorkspaceDialog.tsxapps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/WorkspaceGroupHeader.tsxapps/desktop/src/lib/trpc/routers/workspaces/utils/git.tsapps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/WorkspaceItem.tsxapps/desktop/src/main/lib/db/index.tsapps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/WorkspaceDropdown.tsxapps/desktop/src/lib/trpc/routers/workspaces/workspaces.tsapps/desktop/src/lib/trpc/routers/terminal/terminal.ts
**/*.{ts,tsx,js,jsx,json}
📄 CodeRabbit inference engine (AGENTS.md)
Use Biome for code formatting and linting, running at root level for speed
Files:
apps/desktop/src/main/lib/terminal/manager.tsapps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/DeleteWorkspaceDialog.tsxapps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/WorkspaceGroupHeader.tsxapps/desktop/src/lib/trpc/routers/workspaces/utils/git.tsapps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/WorkspaceItem.tsxapps/desktop/src/main/lib/db/index.tsapps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/WorkspaceDropdown.tsxapps/desktop/src/lib/trpc/routers/workspaces/workspaces.tsapps/desktop/src/lib/trpc/routers/terminal/terminal.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Avoid
anytype and prioritize type safety in TypeScript code
Files:
apps/desktop/src/main/lib/terminal/manager.tsapps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/DeleteWorkspaceDialog.tsxapps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/WorkspaceGroupHeader.tsxapps/desktop/src/lib/trpc/routers/workspaces/utils/git.tsapps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/WorkspaceItem.tsxapps/desktop/src/main/lib/db/index.tsapps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/WorkspaceDropdown.tsxapps/desktop/src/lib/trpc/routers/workspaces/workspaces.tsapps/desktop/src/lib/trpc/routers/terminal/terminal.ts
**/components/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/components/**/*.{ts,tsx}: Structure project folders as one folder per component with PascalCase naming (ComponentName/ComponentName.tsx + index.ts barrel export)
Co-locate component dependencies (utils, hooks, constants, config, tests, stories) next to the file using them
Use one component per file (no multi-component files)
Files:
apps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/DeleteWorkspaceDialog.tsxapps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/WorkspaceGroupHeader.tsxapps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/WorkspaceItem.tsxapps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/WorkspaceDropdown.tsx
apps/desktop/src/renderer/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Call IPC methods from renderer process using window.ipcRenderer.invoke with type-safe object parameters
Files:
apps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/DeleteWorkspaceDialog.tsxapps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/WorkspaceGroupHeader.tsxapps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/WorkspaceItem.tsxapps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/WorkspaceDropdown.tsx
🧠 Learnings (3)
📚 Learning: 2025-12-12T05:45:09.686Z
Learnt from: CR
Repo: superset-sh/superset PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-12T05:45:09.686Z
Learning: Applies to **/components/**/*.{ts,tsx} : Structure project folders as one folder per component with PascalCase naming (ComponentName/ComponentName.tsx + index.ts barrel export)
Applied to files:
apps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/WorkspaceGroupHeader.tsx
📚 Learning: 2025-12-12T05:45:09.686Z
Learnt from: CR
Repo: superset-sh/superset PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-12T05:45:09.686Z
Learning: Applies to packages/ui/**/*.{ts,tsx} : Use React + TailwindCSS v4 + shadcn/ui for UI components in `packages/ui`
Applied to files:
apps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/WorkspaceGroupHeader.tsx
📚 Learning: 2025-11-24T21:33:13.267Z
Learnt from: CR
Repo: superset-sh/superset PR: 0
File: apps/desktop/AGENTS.md:0-0
Timestamp: 2025-11-24T21:33:13.267Z
Learning: Applies to apps/desktop/**/*.{ts,tsx} : Please use alias as defined in `tsconfig.json` when possible
Applied to files:
apps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/WorkspaceItem.tsx
🧬 Code graph analysis (5)
apps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/WorkspaceGroupHeader.tsx (1)
apps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/WorkspaceGroupContextMenu.tsx (1)
WorkspaceGroupContextMenu(21-186)
apps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/WorkspaceItem.tsx (1)
packages/ui/src/components/ui/sonner.tsx (1)
toast(40-40)
apps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/WorkspaceDropdown.tsx (4)
apps/desktop/src/renderer/react-query/workspaces/index.ts (3)
useCreateWorkspace(2-2)useCreateBranchWorkspace(1-1)useSetActiveWorkspace(5-5)apps/desktop/src/renderer/react-query/projects/useOpenNew.ts (1)
useOpenNew(7-22)packages/ui/src/components/ui/sonner.tsx (1)
toast(40-40)packages/ui/src/components/ui/dropdown-menu.tsx (1)
DropdownMenuContent(252-252)
apps/desktop/src/lib/trpc/routers/workspaces/workspaces.ts (3)
apps/desktop/src/lib/trpc/routers/workspaces/utils/git.ts (7)
safeCheckoutBranch(600-620)listBranches(453-476)worktreeExists(264-281)removeWorktree(225-246)fetchDefaultBranch(339-347)getCurrentBranch(553-565)checkNeedsRebase(355-366)apps/desktop/src/lib/trpc/routers/workspaces/utils/worktree.ts (1)
getWorkspacePath(18-28)apps/desktop/src/lib/trpc/routers/workspaces/utils/github/github.ts (1)
fetchGitHubPRStatus(22-67)
apps/desktop/src/lib/trpc/routers/terminal/terminal.ts (2)
apps/desktop/src/lib/trpc/routers/workspaces/utils/worktree.ts (1)
getWorkspacePath(18-28)apps/desktop/src/lib/trpc/routers/terminal/utils/resolve-cwd.ts (1)
resolveCwd(14-54)
⏰ 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). (6)
- GitHub Check: Deploy Docs
- GitHub Check: Deploy Admin
- GitHub Check: Deploy Web
- GitHub Check: Deploy API
- GitHub Check: Deploy Marketing
- GitHub Check: Build
🔇 Additional comments (21)
apps/desktop/src/main/lib/terminal/manager.ts (1)
287-300: LGTM! Clean implementation for prompt refresh.The method properly filters alive sessions by workspace ID and safely writes a newline to refresh prompts. The return value is useful for telemetry and the implementation follows existing patterns in the class.
apps/desktop/src/main/lib/db/index.ts (2)
68-81: LGTM! Robust duplicate cleanup with proper fallback.The deduplication logic correctly keeps the most recently opened workspace and updates
lastActiveWorkspaceIdwhen it points to a removed workspace. The logging is helpful for debugging.
39-43: Ensure the branch field is never empty during migration or throw a descriptive error.The fallback at lines 39-43 sets
branchto an empty string if no worktree branch is found. However, the codebase explicitly validates that branch names cannot be empty:
- CLI rejects empty branch input with "Branch/ref cannot be empty" error
- Cloud workspace creation requires a non-empty branch
- Git checkout verification compares the expected branch against the current branch, which would fail with an empty string
Instead of silently setting an empty string, consider:
- Requiring that pre-migration workspaces always have a valid branch (if this shouldn't happen, remove the fallback)
- Throwing an error with clear context about which workspace is missing a branch
- Documenting why this edge case is safe to ignore if it truly cannot occur
apps/desktop/src/lib/trpc/routers/terminal/terminal.ts (2)
50-55: LGTM! Clean refactoring to workspace-centric path resolution.The change from
worktreePathtoworkspacePathwith the newgetWorkspacePathutility properly abstracts workspace path resolution to support both worktree and branch types.
129-136: LGTM! Simple and effective workspace terminal cleanup.The mutation properly delegates to the terminal manager and returns useful statistics about the cleanup operation.
apps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/WorkspaceGroupHeader.tsx (1)
17-28: LGTM! Proper luminance calculation for contrast.The relative luminance formula is correct and the threshold of 0.6 provides good text contrast. This is a standard approach for determining whether to use light or dark text on colored backgrounds.
apps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/WorkspaceItem.tsx (1)
67-93: LGTM! Appropriate handling for branch workspace deletion.The logic correctly distinguishes branch workspaces (which only close without destructive deletion) from worktree workspaces. Skipping confirmation when there are no active terminals is a good UX decision for branch workspaces.
apps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/DeleteWorkspaceDialog.tsx (1)
65-92: LGTM! Clear distinction between close and delete operations.The messaging appropriately reflects the different implications of closing a branch workspace (non-destructive) versus deleting a worktree workspace (destructive).
apps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/WorkspaceDropdown.tsx (2)
156-212: Complex branch switching logic - verify state management.The handler manages three scenarios well (worktree exists, switch existing branch workspace, create new), but the explicit
setActiveWorkspacecall at line 185 within the toast.promise success handler could create a race condition if the mutation hasn't fully invalidated queries yet.Consider moving the workspace activation to the
onSuccesscallback of the mutation itself, or ensure proper query invalidation ordering:switchBranchWorkspace.mutateAsync({ projectId: currentProjectId, branch, }).then(() => { // Wait for invalidation, then activate return utils.workspaces.invalidate(); }).then(() => { setActiveWorkspace.mutate({ id: existingBranchWorkspace.id }); closeDropdown(); });Alternatively, verify that
switchBranchWorkspacealready includes the workspace activation logic server-side.
112-122: LGTM! Proper branch deduplication and sorting.The logic correctly combines local and remote branches, removes duplicates, and prioritizes main/master branches. This provides a good UX for branch selection.
apps/desktop/src/lib/trpc/routers/workspaces/utils/git.ts (3)
502-546: Verify untracked files don't block checkout when they should.The safety check detects untracked files (line 516) but doesn't block the checkout. Git will fail if untracked files would be overwritten by checkout, but this might provide a confusing error message to users.
Consider making untracked files a blocking condition if they would conflict with the target branch:
// After detecting untracked files, check if they would conflict if (hasUntrackedFiles) { // Could use: git checkout --dry-run <branch> // Or: git diff --name-only HEAD <branch> // to check if untracked files would be overwritten }Alternatively, document that git's native error handling for untracked file conflicts is intentional and sufficient.
567-591: LGTM! Robust branch checkout with proper remote tracking.The function correctly handles three scenarios: local branch exists, remote branch needs tracking, and branch doesn't exist. The fallback to git's native error for non-existent branches provides clear feedback.
600-620: Excellent post-checkout verification.The verification that HEAD matches the target branch (lines 614-619) is a critical safety check that catches silent failures. This is a best practice for git operations.
apps/desktop/src/lib/trpc/routers/workspaces/workspaces.ts (8)
14-27: LGTM!The new imports for branch operations (
getCurrentBranch,listBranches,safeCheckoutBranch) andgetWorkspacePathalign well with the new branch workspace functionality.
111-122: LGTM!The addition of
type: "worktree" as constand thebranchfield to the workspace object correctly implements the workspace type distinction.
243-252: LGTM!Clean implementation that properly validates the project exists before delegating to
listBranches.
254-304: LGTM!Well-structured implementation with proper safety checks via
safeCheckoutBranch. Good UX decision to preserve the user's custom alias (name) when switching branches.
400-406: LGTM!Correct conditional handling for worktree lookup based on
workspace.worktreeIdexistence, and proper use ofgetWorkspacePathfor unified path resolution.
583-627: LGTM!Clean conditional handling that correctly skips worktree cleanup for branch-type workspaces while maintaining the same DB cleanup logic for both types.
668-698: LGTM!Good implementation that performs the checkout with safety checks before updating the database. The terminal prompt refresh ensures the UI reflects the current branch.
892-920: LGTM!The
workspaceTypefield addition enables proper client-side differentiation. Thenullvalues forgitStatusandgithubStatusare reasonable since branch workspace status is fetched through different endpoints.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
apps/desktop/src/lib/trpc/routers/workspaces/workspaces.ts (1)
175-188: Race condition between duplicate check and workspace creation (previously flagged).The existingBranchWorkspace check (lines 175-188) occurs outside the
db.updatetransaction (lines 214-234). Concurrent requests for the same branch could both pass the check before either completes creation, resulting in duplicate branch workspaces despite the constraint.The past review suggested moving the check inside
db.updateto achieve atomicity, but the current code retains the non-atomic pattern.Also applies to: 214-234
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
apps/desktop/src/lib/trpc/routers/workspaces/utils/git.ts(1 hunks)apps/desktop/src/lib/trpc/routers/workspaces/workspaces.ts(13 hunks)apps/desktop/src/shared/ipc-channels/tab.ts(5 hunks)apps/desktop/src/shared/types.ts(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/desktop/src/lib/trpc/routers/workspaces/utils/git.ts
🧰 Additional context used
📓 Path-based instructions (4)
apps/desktop/**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (apps/desktop/AGENTS.md)
For Electron interprocess communication, ALWAYS use tRPC as defined in
src/lib/trpc
Files:
apps/desktop/src/shared/types.tsapps/desktop/src/shared/ipc-channels/tab.tsapps/desktop/src/lib/trpc/routers/workspaces/workspaces.ts
apps/desktop/**/*.{ts,tsx}
📄 CodeRabbit inference engine (apps/desktop/AGENTS.md)
apps/desktop/**/*.{ts,tsx}: Please use alias as defined intsconfig.jsonwhen possible
Prefer zustand for state management if it makes sense. Do not use effect unless absolutely necessary
Files:
apps/desktop/src/shared/types.tsapps/desktop/src/shared/ipc-channels/tab.tsapps/desktop/src/lib/trpc/routers/workspaces/workspaces.ts
**/*.{ts,tsx,js,jsx,json}
📄 CodeRabbit inference engine (AGENTS.md)
Use Biome for code formatting and linting, running at root level for speed
Files:
apps/desktop/src/shared/types.tsapps/desktop/src/shared/ipc-channels/tab.tsapps/desktop/src/lib/trpc/routers/workspaces/workspaces.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Avoid
anytype and prioritize type safety in TypeScript code
Files:
apps/desktop/src/shared/types.tsapps/desktop/src/shared/ipc-channels/tab.tsapps/desktop/src/lib/trpc/routers/workspaces/workspaces.ts
🧠 Learnings (3)
📚 Learning: 2025-12-12T05:45:09.686Z
Learnt from: CR
Repo: superset-sh/superset PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-12T05:45:09.686Z
Learning: Applies to apps/desktop/src/main/lib/*.ts : Implement IPC handlers accepting object parameters (not positional parameters) in apps/desktop/src/main/lib/*.ts files
Applied to files:
apps/desktop/src/shared/types.tsapps/desktop/src/shared/ipc-channels/tab.ts
📚 Learning: 2025-11-24T21:33:13.267Z
Learnt from: CR
Repo: superset-sh/superset PR: 0
File: apps/desktop/AGENTS.md:0-0
Timestamp: 2025-11-24T21:33:13.267Z
Learning: Applies to apps/desktop/**/*.{ts,tsx} : Please use alias as defined in `tsconfig.json` when possible
Applied to files:
apps/desktop/src/shared/types.ts
📚 Learning: 2025-12-12T05:45:09.686Z
Learnt from: CR
Repo: superset-sh/superset PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-12T05:45:09.686Z
Learning: Applies to apps/desktop/src/shared/ipc-channels.ts : Define IPC channel types in apps/desktop/src/shared/ipc-channels.ts before implementing handlers
Applied to files:
apps/desktop/src/shared/ipc-channels/tab.ts
🧬 Code graph analysis (1)
apps/desktop/src/lib/trpc/routers/workspaces/workspaces.ts (6)
apps/desktop/src/main/lib/db/index.ts (1)
db(144-151)apps/desktop/src/lib/trpc/routers/workspaces/utils/git.ts (6)
safeCheckoutBranch(604-630)listBranches(454-480)removeWorktree(225-246)fetchDefaultBranch(339-347)getCurrentBranch(557-569)checkNeedsRebase(355-366)apps/desktop/src/main/lib/terminal/manager.ts (1)
terminalManager(424-424)apps/desktop/src/lib/trpc/routers/workspaces/utils/worktree.ts (1)
getWorkspacePath(18-28)apps/desktop/src/lib/trpc/routers/workspaces/utils/teardown.ts (1)
runTeardown(37-75)apps/desktop/src/lib/trpc/routers/workspaces/utils/github/github.ts (1)
fetchGitHubPRStatus(22-67)
🔇 Additional comments (11)
apps/desktop/src/shared/types.ts (1)
107-107: LGTM! Type changes support branch workspaces correctly.The optional
worktreeIdfield correctly reflects that branch workspaces operate on the main repository path instead of a dedicated worktree. The comments provide helpful context.Also applies to: 119-119
apps/desktop/src/shared/ipc-channels/tab.ts (1)
22-22: LGTM! IPC channel types consistently updated for optional worktreeId.All tab operation channels now support optional
worktreeId, enabling branch workspaces to perform tab operations without requiring a worktree identifier.Also applies to: 36-36, 46-46, 56-56, 68-68
apps/desktop/src/lib/trpc/routers/workspaces/workspaces.ts (9)
243-257: LGTM! Clean branch listing implementation.The procedure correctly delegates to
listBrancheswith proper input validation and optional fetch parameter for remote refresh.
339-351: LGTM! Workspace grouping correctly extended for branch workspaces.The type additions (
worktreeId?,type,branch) and use ofgetWorkspacePathproperly support both worktree and branch workspace types with appropriate fallback handling.Also applies to: 374-377
772-806: LGTM! Git status refresh correctly handles both workspace types.The procedure appropriately branches on workspace type, using
mainRepoPathfor branch workspaces with proper null handling forgetCurrentBranch(line 792), and only looking up worktree data when needed.
844-866: LGTM! GitHub status fetching handles branch mismatches gracefully.The branch mismatch warning (lines 853-859) is a good defensive measure that logs the inconsistency while still fetching the actual status. This handles cases where branches are manually switched outside the app.
491-506: LGTM! Deletion logic correctly distinguishes workspace types.Branch workspaces can be deleted without worktree cleanup checks (lines 492-502), which is correct since they operate directly on the main repo. The conditional worktree lookup (lines 504-506) prevents unnecessary queries.
897-910: LGTM! Worktree info correctly returns branch metadata.The procedure properly distinguishes between branch and worktree workspace types, returning appropriate metadata for each with the
workspaceTypediscriminator field.
604-611: TherunTeardownfunction is synchronous and does not requireawait.The
runTeardownfunction defined inapps/desktop/src/lib/trpc/routers/workspaces/utils/teardown.tsreturns aTeardownResultobject directly, not a Promise. It usesexecSyncfor shell command execution, making it a blocking synchronous operation. Calling it withoutawaitat line 604 is correct.Likely an incorrect or invalid review comment.
682-691: Branch checkout properly handles uncommitted changes with clear error messaging.The
safeCheckoutBranchfunction performs comprehensive safety checks before switching branches viacheckBranchCheckoutSafety, which detects uncommitted changes (staged, modified, deleted files) and throws a clear error message: "Cannot switch branches: you have uncommitted changes. Please commit or stash your changes first." The function also verifies the checkout succeeded by confirming the current branch after the operation. If any safety check fails, the error propagates through tRPC, preventing the workspace from being marked active and leaving the repository in a safe, consistent state.
190-191: Branch existence is already validated during checkout.The
checkoutBranch()function comprehensively validates branch existence before attempting checkout: it checks if the branch exists locally viagit.branchLocal(), checks if it exists on remote viagit.branch(["-r"]), and only fails if the branch doesn't exist anywhere. ThesafeCheckoutBranch()wrapper further verifies the checkout succeeded. No additional validation is needed.Likely an incorrect or invalid review comment.
| // Checkout the new branch with safety checks (terminals continue running on the new branch) | ||
| await safeCheckoutBranch(project.mainRepoPath, input.branch); |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Verify branch existence before checkout attempt.
Similar to createBranchWorkspace, this procedure should verify that input.branch exists before attempting checkout to provide clearer error messages.
Apply the same validation pattern:
+ // Verify branch exists
+ const branches = await listBranches(project.mainRepoPath);
+ const branchExists = branches.local.includes(input.branch) || branches.remote.includes(input.branch);
+ if (!branchExists) {
+ throw new Error(`Branch "${input.branch}" not found in repository`);
+ }
+
// Checkout the new branch with safety checks (terminals continue running on the new branch)
await safeCheckoutBranch(project.mainRepoPath, input.branch);🤖 Prompt for AI Agents
In apps/desktop/src/lib/trpc/routers/workspaces/workspaces.ts around lines
280-281, before calling safeCheckoutBranch(project.mainRepoPath, input.branch)
add the same branch-existence validation used in createBranchWorkspace: check
whether input.branch exists in the repository (e.g., via the repo helper or git
list/hasBranch function), and if it does not exist throw a clear TRPC/error with
a descriptive message and status (matching createBranchWorkspace). Only proceed
to call safeCheckoutBranch when the branch-existence check passes so the error
message is explicit rather than failing during checkout.
1 participant
Description
Related Issues
Type of Change
Testing
Screenshots (if applicable)
Additional Notes
Summary by CodeRabbit
New Features
Bug Fixes / UX
✏️ Tip: You can customize this high-level summary in your review settings.