Skip to content

feature(desktop): manage worktrees better#407

Merged
AviPeltz merged 5 commits intomainfrom
key-muskox-01b285
Dec 18, 2025
Merged

feature(desktop): manage worktrees better#407
AviPeltz merged 5 commits intomainfrom
key-muskox-01b285

Conversation

@AviPeltz
Copy link
Copy Markdown
Collaborator

@AviPeltz AviPeltz commented Dec 18, 2025

Description

Related Issues

Type of Change

  • Bug fix
  • [ x] New feature
  • Documentation
  • Refactor
  • Other (please describe):

Testing

Screenshots (if applicable)

Additional Notes

Summary by CodeRabbit

  • New Features

    • Open existing worktrees as separate workspaces; open-worktree flow provisions terminal tabs and optional setup commands.
    • Close workspace action that terminates sessions but preserves worktree data.
    • New UI list to browse and open a project's existing worktrees.
  • Improvements

    • Redesigned "Open Workspace" modal with compact project selector and Existing vs New mode.
    • Clearer confirm dialog for closing/deleting workspaces with tooltips and concise messaging.

✏️ Tip: You can customize this high-level summary in your review settings.

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Dec 18, 2025

Walkthrough

Adds three TRPC procedures to workspaces: getWorktreesByProject, openWorktree, and close. Frontend: replaces NewWorkspaceModal project list with a Select + mode toggle, adds ExistingWorktreesList, introduces useOpenWorktree and useCloseWorkspace hooks, and refactors DeleteWorkspaceDialog to support closing workspaces.

Changes

Cohort / File(s) Summary
Backend TRPC Procedures
apps/desktop/src/lib/trpc/routers/workspaces/workspaces.ts
Added getWorktreesByProject(projectId), openWorktree(worktreeId, name?) (validates, ensures no active workspace, computes tab order, persists workspace, loads setup config, returns workspace + initialCommands + worktreePath + projectId), and close(id) (terminates terminals, removes workspace record, updates project visibility/lastActiveWorkspace, returns success and optional terminalWarning).
New Workspace UI & Modal
apps/desktop/src/renderer/components/NewWorkspaceModal/NewWorkspaceModal.tsx
Replaced manual project list with a Select chooser; introduced mode state ("existing" | "new"); conditionally renders ExistingWorktreesList for existing worktrees or new-workspace inputs; adjusted header, button/footer behavior, and form reset logic.
Existing Worktrees Component
apps/desktop/src/renderer/components/NewWorkspaceModal/components/ExistingWorktreesList/*, .../index.ts
Added ExistingWorktreesList component and re-export: fetches worktrees via TRPC, groups open vs closed, renders closed worktrees as openable buttons, handles open mutation with toasts and callbacks, shows loading/empty states.
React Query Hooks (Workspaces)
apps/desktop/src/renderer/react-query/workspaces/useOpenWorktree.ts, .../useCloseWorkspace.ts, .../index.ts
Added useOpenWorktree (TRPC mutation wrapper that invalidates caches, creates/attaches terminal tab(s), handles initialCommands and config toast, and forwards user onSuccess) and useCloseWorkspace (mutation with cache invalidation). Re-exported both in index.
Workspace Deletion Dialog
apps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/DeleteWorkspaceDialog.tsx
Refactored dialog to add explicit Close and Delete flows with tooltips, new footer layout, toast-based close handling with terminal warnings, compact status messaging, and a warning banner for unpushed changes.
Exports / Minor
apps/desktop/src/renderer/react-query/workspaces/index.ts, apps/desktop/src/renderer/components/NewWorkspaceModal/components/ExistingWorktreesList/index.ts
Added re-exports for new hooks and the ExistingWorktreesList component.

Sequence Diagram(s)

sequenceDiagram
    actor User
    participant Modal as NewWorkspaceModal / ExistingWorktreesList
    participant Hook as useOpenWorktree
    participant TRPC as TRPC Server (workspaces)
    participant DB as Database
    participant FS as File System
    participant Term as Terminal Manager

    User->>Modal: Click "Open" on closed worktree
    Modal->>Hook: call mutation (worktreeId, name?)
    Hook->>TRPC: workspaces.openWorktree mutation request
    TRPC->>TRPC: validate worktree exists & no active workspace
    TRPC->>FS: verify worktree path exists
    TRPC->>DB: compute tab order & persist workspace
    TRPC->>FS: load setup config -> initialCommands
    TRPC-->>Hook: return workspace, initialCommands, worktreePath, projectId

    Hook->>Hook: invalidate workspace/project caches
    Hook->>Term: createOrAttach terminal tab (workspaceId, initialCommands)
    alt initialCommands present
        Hook->>Term: set tab title "Workspace Setup"
    else
        Hook->>Modal: show config toast (open config modal)
    end
    Hook-->>Modal: trigger success callback / close modal
    Term-->>User: terminal tab ready
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

  • Pay attention to: validation and race conditions in openWorktree (existence checks, no-active-workspace), DB writes and tab-order computation, terminal termination counting in close.
  • Review useOpenWorktree for correct cache invalidation ordering, correct parameters to createOrAttach, and config-toast lifecycle.
  • Check modal mode transitions and integration with ExistingWorktreesList.

Possibly related PRs

Poem

🐰
I hop where worktrees quietly lie,
Open a workspace, let terminals fly.
Select a project, existing or new,
Close with care — leave worktrees true.
Tiny paws, big tasks — cheers to you! 🥕✨

Pre-merge checks and finishing touches

❌ Failed checks (2 warnings, 1 inconclusive)
Check name Status Explanation Resolution
Description check ⚠️ Warning The PR description follows the template structure but lacks substantive content; only 'New feature' is checked while all other sections remain empty or contain only template placeholders. Complete the description with details of the changes: explain what worktree management features were added, link related issues if any, describe testing performed, and provide context for reviewers.
Docstring Coverage ⚠️ Warning Docstring coverage is 33.33% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
Title check ❓ Inconclusive The title 'feature(desktop): manage worktrees better' is related to the PR's intent but lacks specificity about the actual changes implemented. Clarify the title to reflect specific changes, such as 'feature(desktop): add workspace opening and closing operations for worktrees' or similar more descriptive phrasing.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch key-muskox-01b285

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

# Conflicts:
#	apps/desktop/src/renderer/components/NewWorkspaceModal/NewWorkspaceModal.tsx
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Dec 18, 2025

🚀 Preview Deployment

🔗 Preview Links

Service Status Link
Neon Database (Neon) View Branch
Vercel API (Vercel) Open Preview
Vercel Web (Vercel) Open Preview
Vercel Marketing (Vercel) Open Preview
Vercel Admin (Vercel) Open Preview
Vercel Docs (Vercel) Open Preview

Preview updates automatically with new commits

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
apps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/DeleteWorkspaceDialog.tsx (1)

25-30: Rename component and update dialog title for clarity.

The component is named DeleteWorkspaceDialog but now handles both closing and deleting workspaces. Additionally, the dialog title "Close workspace?" is confusing when three distinct actions are presented (Cancel, Close, Delete).

Recommendations:

  1. Rename the component to WorkspaceActionsDialog or ManageWorkspaceDialog
  2. Update the title to something more neutral like "Manage Workspace" or "Workspace Actions"
  3. Update the file name accordingly

This will improve code maintainability and reduce user confusion about the dialog's purpose.

🔎 Suggested changes

Component and file renaming:

-export function DeleteWorkspaceDialog({
+export function WorkspaceActionsDialog({

Dialog title update:

-					<AlertDialogTitle className="text-sm font-medium">
-						Close workspace?
-					</AlertDialogTitle>
+					<AlertDialogTitle className="text-sm font-medium">
+						Manage Workspace
+					</AlertDialogTitle>

Then rename the file from DeleteWorkspaceDialog.tsx to WorkspaceActionsDialog.tsx and update all imports across the codebase.

Also applies to: 119-120

🧹 Nitpick comments (3)
apps/desktop/src/renderer/react-query/workspaces/useOpenWorktree.ts (1)

42-47: Consider error handling for terminal creation.

The createOrAttach.mutate call is fire-and-forget. If terminal session creation fails, the user won't see any error feedback, potentially leaving them with an empty terminal tab.

Consider using mutateAsync with error handling or at least logging failures:

🔎 Suggested approach
-			createOrAttach.mutate({
-				paneId,
-				tabId,
-				workspaceId: data.workspace.id,
-				initialCommands,
-			});
+			createOrAttach.mutateAsync({
+				paneId,
+				tabId,
+				workspaceId: data.workspace.id,
+				initialCommands,
+			}).catch((err) => {
+				console.error("Failed to create terminal session:", err);
+				toast.error("Failed to initialize terminal");
+			});
apps/desktop/src/renderer/components/NewWorkspaceModal/NewWorkspaceModal.tsx (1)

186-211: Consider extracting the mode switcher to a reusable component.

The toggle pattern here could be encapsulated into a reusable SegmentedControl or TabToggle component for consistency across the app. This is a minor improvement for maintainability.

apps/desktop/src/lib/trpc/routers/workspaces/workspaces.ts (1)

694-783: LGTM with a note on code duplication.

The procedure correctly validates the worktree state before creating a workspace. The validation chain (worktree existence → no active workspace → project lookup → disk verification) is thorough.

The tab order calculation and project activation logic (lines 733-771) duplicates code from the create procedure (lines 101-141). Consider extracting this into a shared helper function for maintainability if more procedures need this pattern.

📜 Review details

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between dd8a30d and b5c0655.

📒 Files selected for processing (8)
  • apps/desktop/src/lib/trpc/routers/workspaces/workspaces.ts (1 hunks)
  • apps/desktop/src/renderer/components/NewWorkspaceModal/NewWorkspaceModal.tsx (4 hunks)
  • apps/desktop/src/renderer/components/NewWorkspaceModal/components/ExistingWorktreesList/ExistingWorktreesList.tsx (1 hunks)
  • apps/desktop/src/renderer/components/NewWorkspaceModal/components/ExistingWorktreesList/index.ts (1 hunks)
  • apps/desktop/src/renderer/react-query/workspaces/index.ts (1 hunks)
  • apps/desktop/src/renderer/react-query/workspaces/useCloseWorkspace.ts (1 hunks)
  • apps/desktop/src/renderer/react-query/workspaces/useOpenWorktree.ts (1 hunks)
  • apps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/DeleteWorkspaceDialog.tsx (3 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/renderer/react-query/workspaces/index.ts
  • apps/desktop/src/renderer/react-query/workspaces/useOpenWorktree.ts
  • apps/desktop/src/renderer/react-query/workspaces/useCloseWorkspace.ts
  • apps/desktop/src/renderer/components/NewWorkspaceModal/components/ExistingWorktreesList/ExistingWorktreesList.tsx
  • apps/desktop/src/lib/trpc/routers/workspaces/workspaces.ts
  • apps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/DeleteWorkspaceDialog.tsx
  • apps/desktop/src/renderer/components/NewWorkspaceModal/NewWorkspaceModal.tsx
  • apps/desktop/src/renderer/components/NewWorkspaceModal/components/ExistingWorktreesList/index.ts
apps/desktop/**/*.{ts,tsx}

📄 CodeRabbit inference engine (apps/desktop/AGENTS.md)

apps/desktop/**/*.{ts,tsx}: Please use alias as defined in tsconfig.json when 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.ts
  • apps/desktop/src/renderer/react-query/workspaces/useOpenWorktree.ts
  • apps/desktop/src/renderer/react-query/workspaces/useCloseWorkspace.ts
  • apps/desktop/src/renderer/components/NewWorkspaceModal/components/ExistingWorktreesList/ExistingWorktreesList.tsx
  • apps/desktop/src/lib/trpc/routers/workspaces/workspaces.ts
  • apps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/DeleteWorkspaceDialog.tsx
  • apps/desktop/src/renderer/components/NewWorkspaceModal/NewWorkspaceModal.tsx
  • apps/desktop/src/renderer/components/NewWorkspaceModal/components/ExistingWorktreesList/index.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/renderer/react-query/workspaces/index.ts
  • apps/desktop/src/renderer/react-query/workspaces/useOpenWorktree.ts
  • apps/desktop/src/renderer/react-query/workspaces/useCloseWorkspace.ts
  • apps/desktop/src/renderer/components/NewWorkspaceModal/components/ExistingWorktreesList/ExistingWorktreesList.tsx
  • apps/desktop/src/lib/trpc/routers/workspaces/workspaces.ts
  • apps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/DeleteWorkspaceDialog.tsx
  • apps/desktop/src/renderer/components/NewWorkspaceModal/NewWorkspaceModal.tsx
  • apps/desktop/src/renderer/components/NewWorkspaceModal/components/ExistingWorktreesList/index.ts
**/*.{ts,tsx}

📄 CodeRabbit inference engine (AGENTS.md)

Avoid any type and prioritize type safety in TypeScript code

Files:

  • apps/desktop/src/renderer/react-query/workspaces/index.ts
  • apps/desktop/src/renderer/react-query/workspaces/useOpenWorktree.ts
  • apps/desktop/src/renderer/react-query/workspaces/useCloseWorkspace.ts
  • apps/desktop/src/renderer/components/NewWorkspaceModal/components/ExistingWorktreesList/ExistingWorktreesList.tsx
  • apps/desktop/src/lib/trpc/routers/workspaces/workspaces.ts
  • apps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/DeleteWorkspaceDialog.tsx
  • apps/desktop/src/renderer/components/NewWorkspaceModal/NewWorkspaceModal.tsx
  • apps/desktop/src/renderer/components/NewWorkspaceModal/components/ExistingWorktreesList/index.ts
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/react-query/workspaces/index.ts
  • apps/desktop/src/renderer/react-query/workspaces/useOpenWorktree.ts
  • apps/desktop/src/renderer/react-query/workspaces/useCloseWorkspace.ts
  • apps/desktop/src/renderer/components/NewWorkspaceModal/components/ExistingWorktreesList/ExistingWorktreesList.tsx
  • apps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/DeleteWorkspaceDialog.tsx
  • apps/desktop/src/renderer/components/NewWorkspaceModal/NewWorkspaceModal.tsx
  • apps/desktop/src/renderer/components/NewWorkspaceModal/components/ExistingWorktreesList/index.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/components/NewWorkspaceModal/components/ExistingWorktreesList/ExistingWorktreesList.tsx
  • apps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/DeleteWorkspaceDialog.tsx
  • apps/desktop/src/renderer/components/NewWorkspaceModal/NewWorkspaceModal.tsx
  • apps/desktop/src/renderer/components/NewWorkspaceModal/components/ExistingWorktreesList/index.ts
🧠 Learnings (1)
📚 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/react-query/workspaces/index.ts
  • apps/desktop/src/renderer/components/NewWorkspaceModal/components/ExistingWorktreesList/index.ts
🧬 Code graph analysis (4)
apps/desktop/src/renderer/react-query/workspaces/useOpenWorktree.ts (3)
apps/desktop/src/renderer/stores/tabs/store.ts (1)
  • useTabsStore (72-626)
apps/desktop/src/renderer/stores/config-modal.ts (1)
  • useOpenConfigModal (34-35)
packages/ui/src/components/ui/sonner.tsx (1)
  • toast (40-40)
apps/desktop/src/renderer/react-query/workspaces/useCloseWorkspace.ts (1)
apps/desktop/src/renderer/react-query/workspaces/index.ts (1)
  • useCloseWorkspace (1-1)
apps/desktop/src/lib/trpc/routers/workspaces/workspaces.ts (3)
apps/desktop/src/lib/trpc/routers/workspaces/utils/git.ts (1)
  • worktreeExists (264-281)
apps/desktop/src/lib/trpc/routers/workspaces/utils/setup.ts (1)
  • loadSetupConfig (5-27)
apps/desktop/src/main/lib/terminal/manager.ts (1)
  • terminalManager (409-409)
apps/desktop/src/renderer/components/NewWorkspaceModal/NewWorkspaceModal.tsx (1)
apps/desktop/src/renderer/components/NewWorkspaceModal/components/ExistingWorktreesList/ExistingWorktreesList.tsx (1)
  • ExistingWorktreesList (12-104)
⏰ 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 Web
  • GitHub Check: Deploy API
  • GitHub Check: Deploy Marketing
  • GitHub Check: Deploy Docs
  • GitHub Check: Deploy Admin
  • GitHub Check: Build
🔇 Additional comments (21)
apps/desktop/src/renderer/components/NewWorkspaceModal/components/ExistingWorktreesList/index.ts (1)

1-1: LGTM!

The barrel export follows the project's component folder structure guidelines correctly.

apps/desktop/src/renderer/react-query/workspaces/index.ts (1)

1-7: LGTM!

The new hook exports are correctly added and maintain alphabetical ordering consistent with the existing exports.

apps/desktop/src/renderer/react-query/workspaces/useCloseWorkspace.ts (1)

1-24: LGTM!

The hook follows proper patterns: uses tRPC per coding guidelines, correctly chains cache invalidations before the user callback, and has clean type inference for the options parameter.

apps/desktop/src/renderer/components/NewWorkspaceModal/components/ExistingWorktreesList/ExistingWorktreesList.tsx (1)

1-104: LGTM!

Well-structured component with proper use of tRPC for data fetching, clean state management for worktree categorization, and good UX with loading states and toast feedback. The alias imports follow coding guidelines.

apps/desktop/src/renderer/react-query/workspaces/useOpenWorktree.ts (1)

49-61: Verify if config toast should respect prior dismissal.

The toast shows every time a worktree is opened without setup commands. The dismissConfigToast mutation is called on dismiss, but there's no check here to see if the user previously dismissed it.

If this is intentional UX (always remind users), this is fine. Otherwise, you may want to check a dismissed flag from data or query the dismissal state before showing.

apps/desktop/src/renderer/components/NewWorkspaceModal/NewWorkspaceModal.tsx (6)

10-27: LGTM!

Clean imports and type definition. The Mode type effectively captures the two UI states.

Also applies to: 42-42


53-53: LGTM!

Appropriate default state for the mode selector, prioritizing the "existing" worktrees view.


76-82: LGTM!

The form reset correctly includes the new mode state.


153-181: LGTM!

The project selector implementation is clean and follows the @superset/ui/select patterns correctly. The null-to-empty-string coercion for the Select value is appropriate.


213-264: LGTM!

Good conditional rendering structure. The ExistingWorktreesList integration is clean, and the form inputs have proper label associations for accessibility.


266-284: LGTM!

The conditional footer rendering correctly shows the "Create Workspace" action only when applicable. Users can still dismiss the dialog via standard dialog close mechanisms.

apps/desktop/src/lib/trpc/routers/workspaces/workspaces.ts (2)

675-692: LGTM!

Clean query implementation. The enrichment with hasActiveWorkspace and workspace fields provides useful data for the frontend without extra round trips.


799-829: LGTM!

The close logic correctly preserves the worktree while cleaning up the workspace record. The project visibility and active workspace fallback logic properly mirrors the delete procedure's behavior.

apps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/DeleteWorkspaceDialog.tsx (8)

13-16: LGTM: Hook imports are appropriate.

The imports for useCloseWorkspace and useDeleteWorkspace follow the project's React Query patterns and provide type-safe workspace operations.


61-79: LGTM: Close handler follows consistent async operation pattern.

The implementation correctly:

  • Handles the async close operation with toast promise feedback
  • Displays terminal warnings after a brief delay to avoid toast conflicts
  • Provides clear error messaging

The pattern of closing the dialog before the operation completes (line 62) is consistent with handleDelete and provides non-blocking UX.


81-106: LGTM: Cleaner, more concise messaging.

The updated toast messages are more succinct while maintaining clarity. The separation of teardown and terminal warnings with appropriate labels helps users understand different failure scenarios.


132-142: LGTM: Warning banner provides clear visual feedback.

The conditional warning banner effectively communicates git status concerns with:

  • Appropriate amber color scheme for non-blocking warnings
  • Clear messaging that differentiates between uncommitted changes and unpushed commits
  • Proper conditional rendering that respects loading and permission states

169-184: LGTM: Delete button properly guarded and well-documented.

The Delete button correctly:

  • Respects both canDelete and isLoading states for proper safety guards
  • Provides a clear tooltip explaining the permanent, destructive nature of the action
  • Uses appropriate destructive variant styling

116-130: LGTM: Compact, informative header design.

The restructured header effectively:

  • Maintains a compact footprint (340px max width) suitable for quick actions
  • Displays workspace name in monospace for easy identification when deletion is possible
  • Shows clear error states when deletion is blocked
  • Provides loading feedback during status checks

39-39: The staleTime and enabled behavior work together correctly.

The concern about stale data is based on a misunderstanding of how React Query's enabled option works. When enabled toggles to true, React Query refetches the query even with high staleTime and gcTime. In this code:

  • When the dialog opens (enabled: open becomes true), the first query refetches automatically
  • When the dialog closes (enabled: open becomes false), the query becomes inactive
  • When reopened, the enabled toggle triggers another refetch

This is the intended design: the git status query uses staleTime: Number.POSITIVE_INFINITY to avoid unnecessary polling (git status rarely changes outside the app), while the terminal count query uses refetchInterval: 2000 because it requires real-time updates. No explicit invalidateQueries calls are needed because the enabled toggle already controls refetch behavior.

Likely an incorrect or invalid review comment.


153-168: The Close and Delete buttons have intentionally different validation requirements, which is the correct approach:

Close button (disabled only on isLoading): Non-destructive operation that hides the workspace from tabs while keeping the worktree on disk. This can proceed even with active terminals or git issues—handleClose gracefully handles terminal warnings by displaying them after the close succeeds.

Delete button (disabled on !canDelete || isLoading): Destructive permanent deletion requiring strict validation from canDelete checks (git status, uncommitted changes, unpushed commits, etc.) before allowing the action.

This distinction is semantically sound. The tooltip implementation is excellent and clearly communicates the behavioral difference.

Comment on lines +785 to +792
close: publicProcedure
.input(z.object({ id: z.string() }))
.mutation(async ({ input }) => {
const workspace = db.data.workspaces.find((w) => w.id === input.id);

if (!workspace) {
throw new Error("Workspace not found");
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Inconsistent error handling pattern with delete procedure.

The close procedure throws an Error when workspace is not found (line 791), while the delete procedure returns { success: false, error: "Workspace not found" } (line 403). This inconsistency may cause different error handling behavior on the frontend.

Consider aligning the approach—either both should throw (letting tRPC handle the error response) or both should return a result object with an error field.

🔎 Option A: Return error object (matches delete)
 		close: publicProcedure
 			.input(z.object({ id: z.string() }))
 			.mutation(async ({ input }) => {
 				const workspace = db.data.workspaces.find((w) => w.id === input.id);
 
 				if (!workspace) {
-					throw new Error("Workspace not found");
+					return { success: false, error: "Workspace not found" };
 				}
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
close: publicProcedure
.input(z.object({ id: z.string() }))
.mutation(async ({ input }) => {
const workspace = db.data.workspaces.find((w) => w.id === input.id);
if (!workspace) {
throw new Error("Workspace not found");
}
close: publicProcedure
.input(z.object({ id: z.string() }))
.mutation(async ({ input }) => {
const workspace = db.data.workspaces.find((w) => w.id === input.id);
if (!workspace) {
return { success: false, error: "Workspace not found" };
}
🤖 Prompt for AI Agents
In apps/desktop/src/lib/trpc/routers/workspaces/workspaces.ts around lines 785
to 792, the close procedure currently throws an Error when the workspace is not
found which is inconsistent with the delete procedure that returns { success:
false, error: "Workspace not found" }; change close to return the same result
object shape as delete (e.g., return { success: false, error: "Workspace not
found" }) instead of throwing, and update the procedure's return type (and any
callers/types) accordingly so both procedures use the same error-handling
pattern.

@AviPeltz AviPeltz merged commit f445e09 into main Dec 18, 2025
10 of 11 checks passed
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

🧹 Nitpick comments (3)
apps/desktop/src/renderer/components/NewWorkspaceModal/NewWorkspaceModal.tsx (3)

150-150: Consider a more neutral dialog title.

The title "Open Workspace" is accurate for "existing" mode but slightly misleading in "new" mode where you're creating a workspace. Consider a more neutral title like "Workspace" or "Manage Workspace" to better reflect the dual functionality.

🔎 View suggested change
-					<DialogTitle className="text-base">Open Workspace</DialogTitle>
+					<DialogTitle className="text-base">Workspace</DialogTitle>

186-210: Enhance mode switcher with ARIA attributes.

The custom mode switcher uses semantic button elements and is keyboard accessible, but would benefit from ARIA attributes to better convey its state to assistive technologies.

🔎 View suggested improvements
 						<div className="px-4 pb-2">
-							<div className="flex p-0.5 bg-muted rounded-md">
+							<div className="flex p-0.5 bg-muted rounded-md" role="tablist">
 								<button
 									type="button"
+									role="tab"
+									aria-selected={mode === "new"}
 									onClick={() => setMode("new")}
 									className={`flex-1 px-3 py-1 text-xs font-medium rounded-sm transition-colors ${
 										mode === "new"
 											? "bg-background text-foreground shadow-sm"
 											: "text-muted-foreground hover:text-foreground"
 									}`}
 								>
 									New Workspace
 								</button>
 								<button
 									type="button"
+									role="tab"
+									aria-selected={mode === "existing"}
 									onClick={() => setMode("existing")}
 									className={`flex-1 px-3 py-1 text-xs font-medium rounded-sm transition-colors ${
 										mode === "existing"
 											? "bg-background text-foreground shadow-sm"
 											: "text-muted-foreground hover:text-foreground"
 									}`}
 								>
 									Existing
 								</button>
 							</div>
 						</div>

215-261: Consider adding Enter key support for form submission.

The form inputs in "new" mode would benefit from standard keyboard navigation where pressing Enter in the Title or Branch input fields triggers workspace creation. This is a common UX pattern that users expect.

🔎 View suggested implementation

Add a keyboard handler to both input fields:

 										<Input
 											id="title"
 											className="h-8 text-sm"
 											placeholder="Feature name"
 											value={title}
 											onChange={(e) => setTitle(e.target.value)}
+											onKeyDown={(e) => {
+												if (e.key === "Enter" && selectedProjectId) {
+													handleCreateWorkspace();
+												}
+											}}
 										/>
 										<Input
 											id="branch"
 											className="h-8 text-sm font-mono"
 											placeholder={
 												title
 													? generateBranchFromTitle(title)
 													: "auto-generated"
 											}
 											value={branchName}
 											onChange={(e) => handleBranchNameChange(e.target.value)}
+											onKeyDown={(e) => {
+												if (e.key === "Enter" && selectedProjectId) {
+													handleCreateWorkspace();
+												}
+											}}
 										/>
📜 Review details

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between b5c0655 and 11ef07c.

📒 Files selected for processing (1)
  • apps/desktop/src/renderer/components/NewWorkspaceModal/NewWorkspaceModal.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/renderer/components/NewWorkspaceModal/NewWorkspaceModal.tsx
apps/desktop/**/*.{ts,tsx}

📄 CodeRabbit inference engine (apps/desktop/AGENTS.md)

apps/desktop/**/*.{ts,tsx}: Please use alias as defined in tsconfig.json when possible
Prefer zustand for state management if it makes sense. Do not use effect unless absolutely necessary

Files:

  • apps/desktop/src/renderer/components/NewWorkspaceModal/NewWorkspaceModal.tsx
**/*.{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/renderer/components/NewWorkspaceModal/NewWorkspaceModal.tsx
**/*.{ts,tsx}

📄 CodeRabbit inference engine (AGENTS.md)

Avoid any type and prioritize type safety in TypeScript code

Files:

  • apps/desktop/src/renderer/components/NewWorkspaceModal/NewWorkspaceModal.tsx
**/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/components/NewWorkspaceModal/NewWorkspaceModal.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/components/NewWorkspaceModal/NewWorkspaceModal.tsx
🧬 Code graph analysis (1)
apps/desktop/src/renderer/components/NewWorkspaceModal/NewWorkspaceModal.tsx (2)
apps/desktop/src/renderer/stores/new-workspace-modal.ts (2)
  • useNewWorkspaceModalOpen (28-29)
  • useCloseNewWorkspaceModal (32-33)
apps/desktop/src/renderer/components/NewWorkspaceModal/components/ExistingWorktreesList/ExistingWorktreesList.tsx (1)
  • ExistingWorktreesList (12-104)
⏰ 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 Admin
  • GitHub Check: Deploy Marketing
  • GitHub Check: Deploy Docs
  • GitHub Check: Deploy Web
  • GitHub Check: Deploy API
  • GitHub Check: Build
🔇 Additional comments (4)
apps/desktop/src/renderer/components/NewWorkspaceModal/NewWorkspaceModal.tsx (4)

10-42: LGTM! Clean imports and type definition.

The Select component additions, icon import, and ExistingWorktreesList integration are well-structured. The Mode type provides clear semantics for the two workflows.


148-181: Clean project selector implementation.

The Select component with the plus button provides a compact, user-friendly interface for project selection. The disabled state handling on the button is appropriate.


44-92: Excellent refactor with clean mode-based workflow.

The state management is well-structured with proper initialization and reset logic. The mode state correctly defaults to "new", and resetForm appropriately clears all state including the new mode field. The auto-select and auto-generate effects are preserved and work correctly with the new mode system.


266-284: Clean conditional rendering and action handling.

The empty state messaging and conditional footer rendering are implemented correctly. The Create Workspace button appropriately shows only when in "new" mode with a project selected, while the "existing" mode handles its own actions through ExistingWorktreesList.

Kitenite pushed a commit that referenced this pull request Dec 18, 2025
* manage worktrees better

* merge in main

* lint typecheck and edge cases

* swap mode
@Kitenite Kitenite deleted the key-muskox-01b285 branch December 18, 2025 22:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant