feat(desktop): main terminal vs worktrees terminal#437
Conversation
WalkthroughThis PR introduces a branch workspace feature alongside existing worktree workspaces. Changes include database schema updates with a new Changes
Sequence DiagramssequenceDiagram
actor User
participant UI as UI Component
participant TRPC as TRPC Router
participant Git as Git Utils
participant DB as Database
participant Terminal as Terminal Manager
User->>UI: Create Branch Workspace
UI->>TRPC: createBranchWorkspace({ projectId })
TRPC->>Git: getCurrentBranch(repoPath)
Git-->>TRPC: current branch
Note over TRPC: Resolve target branch<br/>(from input or current)
TRPC->>Git: checkBranchCheckoutSafety(repoPath)
Git-->>TRPC: CheckoutSafetyResult
alt Unsafe
TRPC-->>UI: Error
UI-->>User: Toast: Safety check failed
else Safe
TRPC->>Git: safeCheckoutBranch(repoPath, branch)
Git->>Git: Fetch, checkout, verify
Git-->>TRPC: Success
TRPC->>DB: Create/update workspace<br/>(type: "branch", branch)
DB-->>TRPC: Workspace
TRPC->>Terminal: refreshPromptsForWorkspace(id)
Terminal-->>TRPC: Ack
TRPC-->>UI: Workspace + wasExisting
UI->>UI: Invalidate queries
alt New workspace
UI->>UI: Add tab
end
UI-->>User: Toast: Success
end
sequenceDiagram
actor User
participant UI as BranchSwitcher UI
participant TRPC as TRPC Router
participant Git as Git Utils
participant DB as Database
User->>UI: Open branch dropdown
UI->>TRPC: getBranches({ projectId })
TRPC->>Git: listBranches(repoPath)
Git-->>TRPC: { local, remote }
TRPC->>DB: Query workspace branches
DB-->>TRPC: Branches in use
TRPC-->>UI: Branches + inUse mappings
UI->>UI: Filter, sort, render
User->>UI: Select branch
UI->>TRPC: switchBranchWorkspace(workspaceId, branch)
TRPC->>Git: checkBranchCheckoutSafety(repoPath)
Git-->>TRPC: Result
alt Unsafe
TRPC-->>UI: Error
UI-->>User: Toast: Cannot switch
else Safe
TRPC->>Git: safeCheckoutBranch(repoPath, branch)
Git-->>TRPC: Success
TRPC->>DB: Update workspace<br/>(branch, name, timestamps)
DB-->>TRPC: Workspace
TRPC->>TRPC: Invalidate queries
TRPC-->>UI: Success
UI->>UI: Close dropdown
UI-->>User: Toast: Switched
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~65 minutes
Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (3 warnings)
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: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
apps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/WorkspaceDropdown.tsx (1)
34-49: Inconsistent workspace creation path for existing projects.The primary action for existing projects still uses
createWorkspace(line 38), while the new project flow usescreateBranchWorkspace(line 72). This creates an inconsistency where:
- Quick action on existing project → creates worktree workspace
- Opening new project → creates branch workspace
Based on the PR's branch workspace architecture, consider whether the primary action should also use
createBranchWorkspacefor consistency, or if this distinction is intentional.apps/desktop/src/lib/trpc/routers/terminal/terminal.ts (1)
166-178: Inconsistent path resolution ingetWorkspaceCwd.While the
createOrAttachprocedure was updated to usegetWorkspacePath(line 52-54), thegetWorkspaceCwdprocedure (lines 166-178) still uses worktree-specific logic and only returnsworktree?.path. This means it won't work correctly for branch workspaces.Consider updating to use
getWorkspacePathfor consistency:🔎 Proposed fix
getWorkspaceCwd: publicProcedure .input(z.string()) .query(async ({ input: workspaceId }) => { const workspace = db.data.workspaces.find((w) => w.id === workspaceId); if (!workspace) { return undefined; } - const worktree = db.data.worktrees.find( - (wt) => wt.id === workspace.worktreeId, - ); - return worktree?.path; + return getWorkspacePath(workspace) ?? undefined; }),apps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/WorkspaceItem.tsx (1)
230-289: MissingneedsAttentionindicator for branch workspaces.The
needsAttentionbadge (lines 282-287) is only rendered for worktree workspaces. Branch workspaces can also have terminal panes that need attention, but this indicator won't appear for them.🔎 Proposed fix to include needsAttention for branch workspaces
) : isBranchWorkspace ? ( <div className="flex items-center gap-2 flex-1 min-w-0"> <Tooltip delayDuration={600}> <TooltipTrigger asChild> <div className="flex items-center gap-2 flex-1 min-w-0"> <div className="flex items-center justify-center size-5 rounded bg-primary/10 shrink-0"> <LuGitBranch className="size-3 text-primary" /> </div> <span className="text-sm font-medium whitespace-nowrap overflow-hidden flex-1 text-left" style={{ maskImage: "linear-gradient(to right, black calc(100% - 16px), transparent 100%)", WebkitMaskImage: "linear-gradient(to right, black calc(100% - 16px), transparent 100%)", }} > {title} </span> + {needsAttention && ( + <span className="relative flex size-2 shrink-0"> + <span className="absolute inline-flex h-full w-full animate-ping rounded-full bg-red-400 opacity-75" /> + <span className="relative inline-flex size-2 rounded-full bg-red-500" /> + </span> + )} </div> </TooltipTrigger>
🧹 Nitpick comments (3)
apps/desktop/src/main/lib/db/index.ts (1)
41-41: Consider using structured logging.Using
console.logdirectly is acceptable for migration messages, but consider using a structured logger if one is available in the project for consistency with other logging patterns.apps/desktop/src/renderer/components/NewWorkspaceModal/NewWorkspaceModal.tsx (1)
141-142: Inconsistent user feedback for branch workspace creation.This creates a branch workspace without toast feedback, while
WorkspaceDropdown.tsx(lines 71-79) wraps the same operation intoast.promise()with loading/success/error states. This leads to inconsistent UX where the same operation provides feedback in one place but not another.Consider adding toast feedback here for consistency:
🔎 Proposed fix for consistent feedback
- // Create a main workspace on the current branch for the new project - await createBranchWorkspace.mutateAsync({ projectId: result.project.id }); + // Create a main workspace on the current branch for the new project + await toast.promise( + createBranchWorkspace.mutateAsync({ projectId: result.project.id }), + { + loading: "Opening project...", + success: "Project opened", + error: (err) => + err instanceof Error ? err.message : "Failed to open project", + }, + );apps/desktop/src/lib/trpc/routers/workspaces/utils/git.ts (1)
470-479: Consider relaxing the untracked files check.Git's default checkout only blocks if untracked files would be overwritten by files in the target branch. This check blocks on any untracked files, which could prevent legitimate branch switches when users have build artifacts, IDE files, or other harmless untracked content.
Consider using
git checkout --dry-runor checking if the untracked files conflict with the target branch rather than blocking unconditionally.🔎 Alternative approach using dry-run
- // Block on untracked files as they could be overwritten - if (hasUntrackedFiles) { - return { - safe: false, - error: - "Cannot switch branches: you have untracked files that may be overwritten. Please commit, stash, or remove them first.", - hasUncommittedChanges: false, - hasUntrackedFiles: true, - }; - } + // Note: We only check uncommitted changes here. + // Git checkout will fail naturally if untracked files conflict with the target branch.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (21)
apps/desktop/docs/BRANCH_WORKSPACE_UX.md(1 hunks)apps/desktop/src/lib/trpc/routers/terminal/terminal.ts(3 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(10 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/components/NewWorkspaceModal/NewWorkspaceModal.tsx(3 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/StartView/index.tsx(5 hunks)apps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/BranchSwitcher/BranchSwitcher.tsx(1 hunks)apps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/BranchSwitcher/index.ts(1 hunks)apps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/DeleteWorkspaceDialog.tsx(2 hunks)apps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/WorkspaceDropdown.tsx(4 hunks)apps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/WorkspaceGroup.tsx(2 hunks)apps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/WorkspaceItem.tsx(10 hunks)apps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/WorkspaceItemContextMenu.tsx(3 hunks)apps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/index.tsx(3 hunks)apps/desktop/src/renderer/stores/tabs/store.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (10)
apps/desktop/**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (apps/desktop/AGENTS.md)
apps/desktop/**/*.{ts,tsx,js,jsx}: For Electron interprocess communication, ALWAYS use trpc as defined insrc/lib/trpc
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/renderer/react-query/workspaces/index.tsapps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/DeleteWorkspaceDialog.tsxapps/desktop/src/main/lib/db/index.tsapps/desktop/src/renderer/components/NewWorkspaceModal/NewWorkspaceModal.tsxapps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/WorkspaceGroup.tsxapps/desktop/src/renderer/stores/tabs/store.tsapps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/WorkspaceItemContextMenu.tsxapps/desktop/src/lib/trpc/routers/workspaces/utils/worktree.tsapps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/BranchSwitcher/BranchSwitcher.tsxapps/desktop/src/lib/trpc/routers/terminal/terminal.tsapps/desktop/src/renderer/screens/main/components/StartView/index.tsxapps/desktop/src/lib/trpc/routers/workspaces/utils/git.tsapps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/BranchSwitcher/index.tsapps/desktop/src/renderer/react-query/workspaces/useCreateBranchWorkspace.tsapps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/WorkspaceDropdown.tsxapps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/index.tsxapps/desktop/src/main/lib/terminal/manager.tsapps/desktop/src/lib/trpc/routers/workspaces/workspaces.tsapps/desktop/src/main/lib/db/schemas.tsapps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/WorkspaceItem.tsx
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Avoid using any type in TypeScript - maintain type safety unless absolutely necessary
Files:
apps/desktop/src/renderer/react-query/workspaces/index.tsapps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/DeleteWorkspaceDialog.tsxapps/desktop/src/main/lib/db/index.tsapps/desktop/src/renderer/components/NewWorkspaceModal/NewWorkspaceModal.tsxapps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/WorkspaceGroup.tsxapps/desktop/src/renderer/stores/tabs/store.tsapps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/WorkspaceItemContextMenu.tsxapps/desktop/src/lib/trpc/routers/workspaces/utils/worktree.tsapps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/BranchSwitcher/BranchSwitcher.tsxapps/desktop/src/lib/trpc/routers/terminal/terminal.tsapps/desktop/src/renderer/screens/main/components/StartView/index.tsxapps/desktop/src/lib/trpc/routers/workspaces/utils/git.tsapps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/BranchSwitcher/index.tsapps/desktop/src/renderer/react-query/workspaces/useCreateBranchWorkspace.tsapps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/WorkspaceDropdown.tsxapps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/index.tsxapps/desktop/src/main/lib/terminal/manager.tsapps/desktop/src/lib/trpc/routers/workspaces/workspaces.tsapps/desktop/src/main/lib/db/schemas.tsapps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/WorkspaceItem.tsx
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
Run Biome for formatting, linting, import organization, and safe fixes at the root level using bun run lint:fix
Files:
apps/desktop/src/renderer/react-query/workspaces/index.tsapps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/DeleteWorkspaceDialog.tsxapps/desktop/src/main/lib/db/index.tsapps/desktop/src/renderer/components/NewWorkspaceModal/NewWorkspaceModal.tsxapps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/WorkspaceGroup.tsxapps/desktop/src/renderer/stores/tabs/store.tsapps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/WorkspaceItemContextMenu.tsxapps/desktop/src/lib/trpc/routers/workspaces/utils/worktree.tsapps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/BranchSwitcher/BranchSwitcher.tsxapps/desktop/src/lib/trpc/routers/terminal/terminal.tsapps/desktop/src/renderer/screens/main/components/StartView/index.tsxapps/desktop/src/lib/trpc/routers/workspaces/utils/git.tsapps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/BranchSwitcher/index.tsapps/desktop/src/renderer/react-query/workspaces/useCreateBranchWorkspace.tsapps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/WorkspaceDropdown.tsxapps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/index.tsxapps/desktop/src/main/lib/terminal/manager.tsapps/desktop/src/lib/trpc/routers/workspaces/workspaces.tsapps/desktop/src/main/lib/db/schemas.tsapps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/WorkspaceItem.tsx
apps/desktop/src/renderer/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Never import Node.js modules in renderer process or shared code - use only in main process (src/main/)
Files:
apps/desktop/src/renderer/react-query/workspaces/index.tsapps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/DeleteWorkspaceDialog.tsxapps/desktop/src/renderer/components/NewWorkspaceModal/NewWorkspaceModal.tsxapps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/WorkspaceGroup.tsxapps/desktop/src/renderer/stores/tabs/store.tsapps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/WorkspaceItemContextMenu.tsxapps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/BranchSwitcher/BranchSwitcher.tsxapps/desktop/src/renderer/screens/main/components/StartView/index.tsxapps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/BranchSwitcher/index.tsapps/desktop/src/renderer/react-query/workspaces/useCreateBranchWorkspace.tsapps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/WorkspaceDropdown.tsxapps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/index.tsxapps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/WorkspaceItem.tsx
apps/desktop/src/{main,renderer,preload}/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use type-safe IPC communication - define channel types in apps/desktop/src/shared/ipc-channels.ts before implementing handlers
Files:
apps/desktop/src/renderer/react-query/workspaces/index.tsapps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/DeleteWorkspaceDialog.tsxapps/desktop/src/main/lib/db/index.tsapps/desktop/src/renderer/components/NewWorkspaceModal/NewWorkspaceModal.tsxapps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/WorkspaceGroup.tsxapps/desktop/src/renderer/stores/tabs/store.tsapps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/WorkspaceItemContextMenu.tsxapps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/BranchSwitcher/BranchSwitcher.tsxapps/desktop/src/renderer/screens/main/components/StartView/index.tsxapps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/BranchSwitcher/index.tsapps/desktop/src/renderer/react-query/workspaces/useCreateBranchWorkspace.tsapps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/WorkspaceDropdown.tsxapps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/index.tsxapps/desktop/src/main/lib/terminal/manager.tsapps/desktop/src/main/lib/db/schemas.tsapps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/WorkspaceItem.tsx
**/{components,features}/**/[!.]*.tsx
📄 CodeRabbit inference engine (AGENTS.md)
Organize project structure with one folder per component: ComponentName/ComponentName.tsx with index.ts barrel export
Files:
apps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/DeleteWorkspaceDialog.tsxapps/desktop/src/renderer/components/NewWorkspaceModal/NewWorkspaceModal.tsxapps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/WorkspaceGroup.tsxapps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/WorkspaceItemContextMenu.tsxapps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/BranchSwitcher/BranchSwitcher.tsxapps/desktop/src/renderer/screens/main/components/StartView/index.tsxapps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/WorkspaceDropdown.tsxapps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/index.tsxapps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/WorkspaceItem.tsx
**/{components,features}/**/*.{ts,tsx,test.ts,test.tsx,stories.tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Co-locate component dependencies (utils, hooks, constants, config, tests, stories) next to the file using them
Files:
apps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/DeleteWorkspaceDialog.tsxapps/desktop/src/renderer/components/NewWorkspaceModal/NewWorkspaceModal.tsxapps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/WorkspaceGroup.tsxapps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/WorkspaceItemContextMenu.tsxapps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/BranchSwitcher/BranchSwitcher.tsxapps/desktop/src/renderer/screens/main/components/StartView/index.tsxapps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/BranchSwitcher/index.tsapps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/WorkspaceDropdown.tsxapps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/index.tsxapps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/WorkspaceItem.tsx
**/*.{tsx,css}
📄 CodeRabbit inference engine (AGENTS.md)
Use React + TailwindCSS v4 + shadcn/ui for UI development
Files:
apps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/DeleteWorkspaceDialog.tsxapps/desktop/src/renderer/components/NewWorkspaceModal/NewWorkspaceModal.tsxapps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/WorkspaceGroup.tsxapps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/WorkspaceItemContextMenu.tsxapps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/BranchSwitcher/BranchSwitcher.tsxapps/desktop/src/renderer/screens/main/components/StartView/index.tsxapps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/WorkspaceDropdown.tsxapps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/index.tsxapps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/WorkspaceItem.tsx
**/{components,features}/**/*.tsx
📄 CodeRabbit inference engine (AGENTS.md)
**/{components,features}/**/*.tsx: Nest components in parent's components/ folder if used only once, promote to highest shared parent's components/ if used 2+ times
Use one component per file - do not combine multiple components in a single file
Files:
apps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/DeleteWorkspaceDialog.tsxapps/desktop/src/renderer/components/NewWorkspaceModal/NewWorkspaceModal.tsxapps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/WorkspaceGroup.tsxapps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/WorkspaceItemContextMenu.tsxapps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/BranchSwitcher/BranchSwitcher.tsxapps/desktop/src/renderer/screens/main/components/StartView/index.tsxapps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/WorkspaceDropdown.tsxapps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/index.tsxapps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/WorkspaceItem.tsx
apps/desktop/src/main/**/*.ts
📄 CodeRabbit inference engine (AGENTS.md)
Accept object parameters in IPC handlers - do not use positional parameters in ipcMain.handle()
Files:
apps/desktop/src/main/lib/db/index.tsapps/desktop/src/main/lib/terminal/manager.tsapps/desktop/src/main/lib/db/schemas.ts
🧠 Learnings (6)
📚 Learning: 2025-12-18T23:19:10.405Z
Learnt from: CR
Repo: superset-sh/superset PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-18T23:19:10.405Z
Learning: Applies to packages/db/src/**/*.{ts,tsx} : Store database schema in packages/db/src/ and use Drizzle ORM for schema management
Applied to files:
apps/desktop/src/main/lib/db/index.ts
📚 Learning: 2025-12-18T23:19:10.405Z
Learnt from: CR
Repo: superset-sh/superset PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-18T23:19:10.405Z
Learning: Applies to **/*.{tsx,css} : Use React + TailwindCSS v4 + shadcn/ui for UI development
Applied to files:
apps/desktop/src/renderer/components/NewWorkspaceModal/NewWorkspaceModal.tsxapps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/BranchSwitcher/BranchSwitcher.tsxapps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/WorkspaceItem.tsx
📚 Learning: 2025-12-18T23:19:10.405Z
Learnt from: CR
Repo: superset-sh/superset PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-18T23:19:10.405Z
Learning: Applies to apps/desktop/src/{main,renderer,preload}/**/*.{ts,tsx} : Use type-safe IPC communication - define channel types in apps/desktop/src/shared/ipc-channels.ts before implementing handlers
Applied to files:
apps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/WorkspaceGroup.tsxapps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/BranchSwitcher/BranchSwitcher.tsxapps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/WorkspaceItem.tsx
📚 Learning: 2025-12-18T23:19:10.405Z
Learnt from: CR
Repo: superset-sh/superset PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-18T23:19:10.405Z
Learning: Applies to **/{components,features}/**/[!.]*.tsx : Organize project structure with one folder per component: ComponentName/ComponentName.tsx with index.ts barrel export
Applied to files:
apps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/BranchSwitcher/BranchSwitcher.tsxapps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/BranchSwitcher/index.ts
📚 Learning: 2025-12-18T17:26:38.664Z
Learnt from: CR
Repo: superset-sh/superset PR: 0
File: apps/desktop/AGENTS.md:0-0
Timestamp: 2025-12-18T17:26:38.664Z
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.ts
📚 Learning: 2025-12-18T23:19:10.405Z
Learnt from: CR
Repo: superset-sh/superset PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-18T23:19:10.405Z
Learning: Applies to apps/desktop/src/renderer/**/*.{ts,tsx} : Never import Node.js modules in renderer process or shared code - use only in main process (src/main/)
Applied to files:
apps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/BranchSwitcher/index.ts
🧬 Code graph analysis (9)
apps/desktop/src/main/lib/db/index.ts (2)
apps/desktop/src/main/lib/db/schemas.ts (3)
Database(119-124)Workspace(55-66)defaultDatabase(126-131)apps/desktop/src/main/lib/app-environment.ts (1)
DB_PATH(8-8)
apps/desktop/src/renderer/components/NewWorkspaceModal/NewWorkspaceModal.tsx (1)
apps/desktop/src/renderer/react-query/workspaces/useCreateBranchWorkspace.ts (1)
useCreateBranchWorkspace(9-32)
apps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/WorkspaceItemContextMenu.tsx (2)
packages/ui/src/components/ui/context-menu.tsx (5)
ContextMenu(237-237)ContextMenuTrigger(238-238)ContextMenuContent(239-239)ContextMenuItem(240-240)ContextMenuSeparator(244-244)packages/ui/src/components/ui/hover-card.tsx (2)
HoverCard(44-44)HoverCardTrigger(44-44)
apps/desktop/src/lib/trpc/routers/terminal/terminal.ts (2)
apps/desktop/src/lib/trpc/routers/workspaces/utils/worktree.ts (1)
getWorkspacePath(17-32)apps/desktop/src/lib/trpc/routers/terminal/utils/resolve-cwd.ts (1)
resolveCwd(14-54)
apps/desktop/src/renderer/screens/main/components/StartView/index.tsx (2)
apps/desktop/src/renderer/react-query/workspaces/useCreateBranchWorkspace.ts (1)
useCreateBranchWorkspace(9-32)apps/desktop/src/renderer/react-query/workspaces/index.ts (1)
useCreateBranchWorkspace(2-2)
apps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/WorkspaceDropdown.tsx (2)
apps/desktop/src/renderer/react-query/workspaces/useCreateBranchWorkspace.ts (1)
useCreateBranchWorkspace(9-32)apps/desktop/src/renderer/react-query/workspaces/index.ts (1)
useCreateBranchWorkspace(2-2)
apps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/index.tsx (2)
apps/desktop/src/renderer/react-query/workspaces/useCreateBranchWorkspace.ts (1)
useCreateBranchWorkspace(9-32)apps/desktop/src/renderer/react-query/workspaces/index.ts (1)
useCreateBranchWorkspace(2-2)
apps/desktop/src/lib/trpc/routers/workspaces/workspaces.ts (3)
apps/desktop/src/lib/trpc/routers/workspaces/utils/git.ts (5)
getCurrentBranch(401-413)safeCheckoutBranch(540-566)listBranches(368-394)worktreeExists(221-236)removeWorktree(190-209)apps/desktop/src/lib/trpc/routers/workspaces/utils/worktree.ts (1)
getWorkspacePath(17-32)apps/desktop/src/lib/trpc/routers/workspaces/utils/teardown.ts (1)
runTeardown(37-75)
apps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/WorkspaceItem.tsx (2)
packages/ui/src/lib/utils.ts (1)
cn(4-6)apps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/BranchSwitcher/BranchSwitcher.tsx (1)
BranchSwitcher(22-190)
🔇 Additional comments (39)
apps/desktop/src/main/lib/terminal/manager.ts (2)
363-381: Method is properly integrated with TRPC and behavior is intentional.The
refreshPromptsForWorkspacemethod is correctly called within thecheckoutBranchmutation in the workspaces TRPC router, complying with the coding guideline to use TRPC for Electron interprocess communication. The newline write is intentional—it refreshes terminal prompts after a branch checkout to display the updated branch name. With proper error handling via try/catch, this implementation is sound.
363-381: This method is used internally within the main process and is not called from the renderer, so TRPC exposure is not required. The implementation is correct—it's properly called from a TRPC procedure (workspace branch checkout) as an internal utility to refresh terminal prompts after branch changes.apps/desktop/src/renderer/react-query/workspaces/index.ts (1)
2-2: LGTM!The barrel export follows the established pattern and properly exposes the new hook.
apps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/BranchSwitcher/index.ts (1)
1-1: LGTM!The barrel export follows the coding guidelines for component structure (ComponentName/ComponentName.tsx with index.ts barrel export).
apps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/WorkspaceGroup.tsx (2)
10-11: LGTM!The Workspace interface correctly extends to include the new type and branch fields, aligning with the schema updates for branch workspace support.
78-79: LGTM!The props are correctly passed to WorkspaceItem, enabling branch-aware rendering in the UI.
apps/desktop/src/renderer/react-query/workspaces/useCreateBranchWorkspace.ts (1)
1-32: LGTM!The hook implementation is well-structured:
- Properly invalidates workspace queries on success
- Conditionally creates tabs only for newly created workspaces (not existing ones)
- Correctly chains user-provided onSuccess callbacks
- Uses async/await for proper error handling
The logic integrates well with the idempotent
addTabbehavior in the tabs store.apps/desktop/src/lib/trpc/routers/workspaces/utils/worktree.ts (1)
12-32: LGTM!The
getWorkspacePathfunction correctly handles both workspace types:
- Branch workspaces resolve to the main repository path
- Worktree workspaces resolve to the worktree path
- Proper null handling for missing projects or worktrees
This provides a unified interface for path resolution across workspace types.
apps/desktop/src/main/lib/db/schemas.ts (1)
53-66: LGTM!The schema updates properly model the dual nature of workspaces (worktree vs. branch):
WorkspaceTypeclearly defines the two variantsbranchfield is present for both typesworktreeIdis appropriately optional (only needed for worktree type)- Clear comment documents when
worktreeIdis setThe breaking change (optional
worktreeId) is handled by the migration logic inapps/desktop/src/main/lib/db/index.ts.apps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/WorkspaceDropdown.tsx (3)
9-12: LGTM! Clean import and hook initialization.The import of
useCreateBranchWorkspaceand hook initialization follows React best practices and aligns with the branch workspace architecture introduced in this PR.Also applies to: 26-26
70-79: LGTM! Branch workspace creation for new projects.The new project flow correctly uses
createBranchWorkspaceto create a main workspace on the current branch, with appropriate loading states and error handling.
106-110: LGTM! Proper disabled state handling.The disabled state correctly includes
createBranchWorkspace.isPendingalongside the other pending states, preventing race conditions during workspace creation.apps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/DeleteWorkspaceDialog.tsx (2)
18-33: LGTM! Clean workspace type discrimination.The addition of
workspaceTypeprop with a sensible default and theisBranchflag provides clear type discrimination for different workspace behaviors.
117-157: LGTM! Branch workspace close dialog follows UX design.The simplified dialog for branch workspaces correctly implements the "Close" (non-destructive) pattern as documented in
BRANCH_WORKSPACE_UX.md. The messaging clearly communicates that the branch and commits remain in the repository.apps/desktop/src/renderer/components/NewWorkspaceModal/NewWorkspaceModal.tsx (1)
22-25: LGTM! Proper hook integration.The import and initialization of
useCreateBranchWorkspacefollows established patterns in the codebase.Also applies to: 61-61
apps/desktop/docs/BRANCH_WORKSPACE_UX.md (1)
1-174: LGTM! Comprehensive design documentation.This document provides excellent coverage of the branch workspace architecture, including:
- Clear rationale for the one-branch-workspace-per-project rule
- Terminology guidance (Close vs Delete)
- Schema and API changes
- Migration considerations
The documentation will be valuable for future developers working with this feature.
apps/desktop/src/lib/trpc/routers/terminal/terminal.ts (1)
8-8: LGTM! Proper workspace path abstraction.The migration from
getWorktreePathtogetWorkspacePathcorrectly abstracts away the workspace type, allowing terminals to work with both branch and worktree workspaces through a unified interface.Also applies to: 50-55
apps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/WorkspaceItemContextMenu.tsx (2)
23-24: LGTM! Flexible context menu for different workspace types.The addition of
canRenameandshowHoverCardprops with sensible defaults provides appropriate flexibility for branch vs worktree workspaces, while maintaining backward compatibility.Also applies to: 33-34, 44-62
71-76: LGTM! Consistent rename gating.The conditional rendering of the Rename menu item based on
canRenameis consistent across both rendering paths.apps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/index.tsx (1)
4-7: LGTM! Proper hook integration.The imports and hook initialization follow established patterns in the codebase.
Also applies to: 26-26
apps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/BranchSwitcher/BranchSwitcher.tsx (5)
16-26: LGTM! Clean component interface.The props interface is well-defined and the component follows React best practices.
30-43: LGTM! Efficient data fetching pattern.The query is correctly gated by
enabled: isOpento avoid unnecessary fetches, and workspace invalidation on success ensures UI consistency.
45-74: LGTM! Well-structured branch prioritization.The sorting logic correctly implements the priority scheme from the UX documentation:
- Default branches (main/master/develop)
- Branches in use by worktrees
- Alphabetical ordering
Proper use of memoization prevents unnecessary re-computation.
76-96: LGTM! Clean search and switch logic.The search filtering is case-insensitive and properly memoized. The switch handler includes a short-circuit for the current branch and provides clear user feedback via toast notifications.
98-189: LGTM! Comprehensive branch switcher UI.The rendering logic provides:
- Proper loading and empty states
- Clear visual differentiation (in-use vs regular branches)
- Contextual labels ("worktree", "default")
- Current branch indication with checkmark
- Pagination with 50-item limit and overflow message
- Proper event handling with
stopPropagationThe implementation aligns well with the UX documentation.
apps/desktop/src/renderer/screens/main/components/StartView/index.tsx (1)
8-8: LGTM! Clean migration to branch workspace paradigm.The refactor from
useCreateWorkspacetouseCreateBranchWorkspaceis consistent throughout the component. The updated error message "Failed to open workspace" is more accurate sincecreateBranchWorkspacemay activate an existing workspace rather than always creating a new one.Also applies to: 18-18, 50-51, 61-66, 77-77
apps/desktop/src/lib/trpc/routers/workspaces/utils/git.ts (4)
361-394: LGTM! Well-structured branch listing utility.Clean implementation with proper error handling for offline scenarios. The remote branch normalization (stripping
origin/and filtering out symbolic refs likeorigin/HEAD -> origin/main) is correct.
396-413: LGTM! Correctly handles detached HEAD state.Returning
nullfor detached HEAD ("HEAD"literal) is the right behavior for callers that need to know if they're on a named branch.
501-531: LGTM! Good handling of remote-only branches.The function correctly creates a local tracking branch when checking out a branch that only exists on remote. The fallthrough to let git provide its natural error message for non-existent branches is a clean approach.
533-566: LGTM! Solid implementation with post-checkout verification.The early return optimization when already on the target branch, combined with the post-checkout verification, makes this a robust utility. The verification step is good defensive programming to catch edge cases where checkout might silently fail or land on a different branch.
apps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/WorkspaceItem.tsx (2)
76-96: LGTM! Appropriate non-destructive delete flow for branch workspaces.The logic correctly differentiates branch workspaces from worktrees: branch workspaces only show a confirmation dialog when there are active terminals (since closing is non-destructive), while worktree deletion continues to check for uncommitted changes, unpushed commits, etc.
306-319: LGTM! Appropriate UX copy for close vs delete semantics.The aria-labels and tooltip text correctly distinguish "Close workspace" (non-destructive for branches) from "Delete workspace" (destructive for worktrees).
apps/desktop/src/lib/trpc/routers/workspaces/workspaces.ts (7)
116-117: LGTM! Existing create mutation extended with type and branch fields.The addition of
type: "worktree"andbranchfields ensures backward compatibility while supporting the new workspace type system.
265-296: LGTM! Clean branch listing query with worktree usage tracking.The
inUsearray correctly identifies branches that are currently checked out in worktrees, which is valuable for the UI to warn users or disable selection.
298-348: LGTM! Well-implemented branch switching with terminal refresh.The flow correctly:
- Uses
safeCheckoutBranchfor safety checks before switching- Refreshes terminal prompts so they display the new branch
- Updates workspace metadata atomically
378-416: LGTM! Unified path resolution viagetWorkspacePath.The switch to
getWorkspacePath(workspace)correctly abstracts the path resolution for both workspace types, returning the main repo path for branch workspaces and the worktree path for worktree workspaces.
516-527: LGTM! Appropriate short-circuit for branch workspaces.Branch workspaces don't require git status checks since closing them is non-destructive (no worktree removal, no branch deletion). The early return with
canDelete: trueis correct.
629-671: LGTM! Correct conditional worktree removal.The guard
workspace.type === "worktree" && workspace.worktreeIdcorrectly skips worktree teardown/removal for branch workspaces while still performing terminal cleanup and database removal for both workspace types.
957-968: LGTM! Consistent workspace creation in openWorktree.Adding
type: "worktree"andbranchfields ensures consistency with the updated workspace schema across all creation paths.
… for useEffect and panelID ish
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
apps/desktop/src/lib/trpc/routers/workspaces/workspaces.ts (1)
449-449: Same empty string fallback concern as line 414.Similar to line 414, this empty string fallback could hide path resolution failures. Consider handling
nullexplicitly or filtering out cases where the path cannot be determined.
🧹 Nitpick comments (1)
apps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/index.tsx (1)
46-77: Consider refactoring to zustand to align with project guidelines.The effect logic is correct and the dependency array properly uses
createBranchWorkspace.mutate(addressing the past review concern). However, per the coding guidelines and learnings, effects should be avoided unless absolutely necessary, with zustand preferred for state management.This background auto-creation logic could be refactored into a zustand store action that:
- Tracks attempted projects in the store
- Executes after groups data loads
- Maintains the serialization logic
This would eliminate the effect and centralize the state management.
Based on learnings: The project prefers zustand for state management and avoiding effects when possible.
The current implementation works correctly, but if you'd like to align more closely with the project's architecture preferences, I can help design a zustand-based approach. Would you like me to propose a refactor?
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
apps/desktop/src/lib/trpc/routers/workspaces/workspaces.ts(10 hunks)apps/desktop/src/main/lib/db/index.ts(1 hunks)apps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/index.tsx(3 hunks)apps/desktop/src/renderer/stores/tabs/store.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (10)
apps/desktop/**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (apps/desktop/AGENTS.md)
apps/desktop/**/*.{ts,tsx,js,jsx}: For Electron interprocess communication, ALWAYS use trpc as defined insrc/lib/trpc
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/renderer/stores/tabs/store.tsapps/desktop/src/lib/trpc/routers/workspaces/workspaces.tsapps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/index.tsxapps/desktop/src/main/lib/db/index.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Avoid using any type in TypeScript - maintain type safety unless absolutely necessary
Files:
apps/desktop/src/renderer/stores/tabs/store.tsapps/desktop/src/lib/trpc/routers/workspaces/workspaces.tsapps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/index.tsxapps/desktop/src/main/lib/db/index.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
Run Biome for formatting, linting, import organization, and safe fixes at the root level using bun run lint:fix
Files:
apps/desktop/src/renderer/stores/tabs/store.tsapps/desktop/src/lib/trpc/routers/workspaces/workspaces.tsapps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/index.tsxapps/desktop/src/main/lib/db/index.ts
apps/desktop/src/renderer/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Never import Node.js modules in renderer process or shared code - use only in main process (src/main/)
Files:
apps/desktop/src/renderer/stores/tabs/store.tsapps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/index.tsx
apps/desktop/src/{main,renderer,preload}/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use type-safe IPC communication - define channel types in apps/desktop/src/shared/ipc-channels.ts before implementing handlers
Files:
apps/desktop/src/renderer/stores/tabs/store.tsapps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/index.tsxapps/desktop/src/main/lib/db/index.ts
**/{components,features}/**/[!.]*.tsx
📄 CodeRabbit inference engine (AGENTS.md)
Organize project structure with one folder per component: ComponentName/ComponentName.tsx with index.ts barrel export
Files:
apps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/index.tsx
**/{components,features}/**/*.{ts,tsx,test.ts,test.tsx,stories.tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Co-locate component dependencies (utils, hooks, constants, config, tests, stories) next to the file using them
Files:
apps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/index.tsx
**/*.{tsx,css}
📄 CodeRabbit inference engine (AGENTS.md)
Use React + TailwindCSS v4 + shadcn/ui for UI development
Files:
apps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/index.tsx
**/{components,features}/**/*.tsx
📄 CodeRabbit inference engine (AGENTS.md)
**/{components,features}/**/*.tsx: Nest components in parent's components/ folder if used only once, promote to highest shared parent's components/ if used 2+ times
Use one component per file - do not combine multiple components in a single file
Files:
apps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/index.tsx
apps/desktop/src/main/**/*.ts
📄 CodeRabbit inference engine (AGENTS.md)
Accept object parameters in IPC handlers - do not use positional parameters in ipcMain.handle()
Files:
apps/desktop/src/main/lib/db/index.ts
🧠 Learnings (2)
📚 Learning: 2025-12-18T17:26:38.664Z
Learnt from: CR
Repo: superset-sh/superset PR: 0
File: apps/desktop/AGENTS.md:0-0
Timestamp: 2025-12-18T17:26:38.664Z
Learning: Applies to apps/desktop/**/*.{ts,tsx,js,jsx} : Prefer zustand for state management if it makes sense. Do not use effect unless absolutely necessary.
Applied to files:
apps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/index.tsx
📚 Learning: 2025-12-18T23:19:10.405Z
Learnt from: CR
Repo: superset-sh/superset PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-18T23:19:10.405Z
Learning: Applies to packages/db/src/**/*.{ts,tsx} : Store database schema in packages/db/src/ and use Drizzle ORM for schema management
Applied to files:
apps/desktop/src/main/lib/db/index.ts
🧬 Code graph analysis (3)
apps/desktop/src/renderer/stores/tabs/store.ts (2)
apps/desktop/src/renderer/stores/tabs/utils.ts (1)
getFirstPaneId(204-209)apps/cli/src/lib/storage/lowdb-adapter.ts (1)
set(162-172)
apps/desktop/src/lib/trpc/routers/workspaces/workspaces.ts (3)
apps/desktop/src/lib/trpc/routers/workspaces/utils/git.ts (5)
getCurrentBranch(401-413)safeCheckoutBranch(540-566)listBranches(368-394)worktreeExists(221-236)removeWorktree(190-209)apps/desktop/src/lib/trpc/routers/workspaces/utils/worktree.ts (1)
getWorkspacePath(17-32)apps/desktop/src/lib/trpc/routers/workspaces/utils/teardown.ts (1)
runTeardown(37-75)
apps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/index.tsx (2)
apps/desktop/src/renderer/react-query/workspaces/useCreateBranchWorkspace.ts (1)
useCreateBranchWorkspace(9-32)apps/desktop/src/renderer/react-query/workspaces/index.ts (1)
useCreateBranchWorkspace(2-2)
⏰ 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). (1)
- GitHub Check: Build
🔇 Additional comments (11)
apps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/index.tsx (3)
4-7: LGTM! Clean import structure.The import addition follows the project's alias patterns and maintains type safety.
26-26: LGTM! Proper hook initialization.The mutation hook is correctly initialized and follows the established patterns in the codebase.
39-42: LGTM! Appropriate use of ref and state.Using a ref for
attemptedProjectsRefavoids unnecessary re-renders whileisCreatingstate correctly triggers effect re-evaluation for serialization.apps/desktop/src/renderer/stores/tabs/store.ts (1)
85-107: Previous concern addressed - idempotency logic now ensures valid paneId.The addition of the idempotency check correctly handles the case where a tab already exists for the workspace. The use of
getFirstPaneId(existingTab.layout)as a fallback (lines 91-93) ensures that a valid paneId is always returned, resolving the concern raised in the previous review about potentially undefined paneId values. Additionally, the code now explicitly setsfocusedPaneIdsfor the existing tab, ensuring consistent state.apps/desktop/src/main/lib/db/index.ts (2)
10-56: Previous concern addressed - migration now handles all edge cases.The migration logic correctly addresses the previous review concern by ensuring that
ws.branchis always set to a valid string value. When the associated worktree is not found (lines 35-40) or when there's noworktreeId(lines 43-46), the code now falls back to"unknown"and logs appropriate warnings. This ensures the schema requirement thatbranchis a required string field is always satisfied.
65-66: LGTM - migration properly integrated.The migration is correctly hooked into the initialization flow, ensuring existing data is migrated when the database is first loaded.
apps/desktop/src/lib/trpc/routers/workspaces/workspaces.ts (5)
159-262: Previous race condition concern addressed with atomic check.The existence check for branch workspaces is now performed atomically inside the
db.updatecallback (lines 215-227), which addresses the race condition concern from the previous review. If an existing branch workspace is found, it's activated without creating a duplicate; otherwise, a new one is created. ThewasExistingflag in the response (line 260) provides useful information to callers about whether this was a new creation or an activation.
264-295: LGTM - getBranches provides useful branch metadata.The
getBranchesprocedure correctly lists local and remote branches with aninUseset indicating which branches are currently associated with worktrees. The optionalfetchparameter allows callers to control whether to refresh remote refs, avoiding UI stalls during normal operation.
516-526: LGTM - correct handling for non-destructive branch workspaces.Branch workspaces are correctly identified as non-destructive and bypass the expensive git checks. Since branch workspaces operate on the main repository path and don't involve worktree removal, the early return with
canDelete: trueis appropriate.
628-670: LGTM - branch workspaces correctly skip worktree operations.The deletion logic properly distinguishes between branch and worktree workspace types. Branch workspaces skip the worktree-specific teardown and removal operations (lines 629-670), which is correct since they don't have associated worktrees to clean up. The guard on line 629 ensures that only worktree-type workspaces with a
worktreeIdproceed with worktree removal.
298-347: No action needed. TheterminalManager.refreshPromptsForWorkspacemethod exists inapps/desktop/src/main/lib/terminal/manager.ts(line 367) and is properly imported in the workspaces file. The implementation correctly iterates through active terminal sessions for the given workspace and sends a newline character to refresh their prompts. The method call on line 322 is correct.
| groupsMap.get(workspace.projectId)?.workspaces.push({ | ||
| ...workspace, | ||
| worktreePath: getWorktreePath(workspace.worktreeId) ?? "", | ||
| worktreePath: getWorkspacePath(workspace) ?? "", |
There was a problem hiding this comment.
Empty string fallback could hide path resolution failures.
When getWorkspacePath(workspace) returns null, this code defaults to an empty string. According to the relevant code snippets, getWorkspacePath returns null when the path "is not determinable." Using an empty string as a fallback could hide these failure cases and cause issues for downstream consumers expecting a valid filesystem path.
Consider one of the following approaches:
Option 1: Filter out workspaces with undeterminable paths
for (const workspace of workspaces) {
if (groupsMap.has(workspace.projectId)) {
+ const worktreePath = getWorkspacePath(workspace);
+ if (!worktreePath) {
+ console.warn(`Could not determine path for workspace ${workspace.id}`);
+ continue;
+ }
groupsMap.get(workspace.projectId)?.workspaces.push({
...workspace,
- worktreePath: getWorkspacePath(workspace) ?? "",
+ worktreePath,
});
}
}Option 2: Make worktreePath optional in the response type
worktreePath: string;
+worktreePath: string | null;
// ... in response type
// Then in the mapping:
-worktreePath: getWorkspacePath(workspace) ?? "",
+worktreePath: getWorkspacePath(workspace),Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In apps/desktop/src/lib/trpc/routers/workspaces/workspaces.ts around line 414,
the code uses getWorkspacePath(workspace) ?? "" which masks path resolution
failures by supplying an empty string; either (A) filter out workspaces where
getWorkspacePath(workspace) returns null before mapping so only entries with
determinable paths are returned, or (B) make worktreePath optional in the
response DTO/Type and pass undefined/null when getWorkspacePath returns null;
implement one approach and update the response type and all callers/tests
accordingly to handle the new behavior (remove the empty-string fallback and
ensure downstream consumers correctly treat missing paths).
🧹 Preview Cleanup CompleteThe following preview resources have been cleaned up:
Thank you for your contribution! 🎉 Preview resources have been processed for cleanup |
1 participant
Description
Related Issues
Type of Change
Testing
Screenshots (if applicable)
Additional Notes
Summary by CodeRabbit
Release Notes
New Features
Documentation
✏️ Tip: You can customize this high-level summary in your review settings.