Skip to content

feat (desktop): allow deleting on workspaces page#713

Merged
AviPeltz merged 2 commits into
mainfrom
right-click-delete-workspaces-page
Jan 12, 2026
Merged

feat (desktop): allow deleting on workspaces page#713
AviPeltz merged 2 commits into
mainfrom
right-click-delete-workspaces-page

Conversation

@AviPeltz
Copy link
Copy Markdown
Collaborator

@AviPeltz AviPeltz commented Jan 12, 2026

Description

Related Issues

Type of Change

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

Testing

Screenshots (if applicable)

Additional Notes

Summary by CodeRabbit

  • New Features
    • Add ability to delete closed worktrees with an eligibility check and clear reasons when deletion is not allowed.
    • New confirmation dialog shows warning flags (uncommitted changes, unpushed commits), disables unsafe deletes, and provides toast feedback for progress/success/errors.
    • Delete action moved into the workspace context menu with contextual labeling and automatic list refresh after deletion.

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

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Jan 12, 2026

📝 Walkthrough

Walkthrough

Adds backend TRPC procedures for validating and deleting closed worktrees, a DeleteWorktreeDialog UI component, a useDeleteWorktree mutation hook, and WorkspaceRow updates to surface the delete action via a context menu and conditional dialogs.

Changes

Cohort / File(s) Summary
Backend: worktree delete procedures
apps/desktop/src/lib/trpc/routers/workspaces/procedures/delete.ts
Adds canDeleteWorktree (query) to validate deletion eligibility (resolves worktree & project, optional git checks, reports hasChanges/hasUnpushedCommits/reason) and deleteWorktree (mutation) to perform deletion with project locking, optional teardown, removal, and record cleanup.
Frontend: Delete dialog component
apps/desktop/src/renderer/screens/main/components/WorkspacesListView/WorkspaceRow/DeleteWorktreeDialog.tsx
New confirmation dialog component that queries canDeleteWorktree on open, displays status/warnings, invokes useDeleteWorktree mutation, and surfaces toast feedback and disabled state when deletion is disallowed.
Frontend: Workspace row integration
apps/desktop/src/renderer/screens/main/components/WorkspacesListView/WorkspaceRow/WorkspaceRow.tsx
Replaces inline delete with a ContextMenu, computes dynamic action label (Close/Delete workspace/worktree), conditionally enables action, and renders DeleteWorktreeDialog alongside existing DeleteWorkspaceDialog.
Frontend: React Query hook
apps/desktop/src/renderer/react-query/workspaces/useDeleteWorktree.ts, apps/desktop/src/renderer/react-query/workspaces/index.ts
Adds useDeleteWorktree mutation hook wrapping trpc.workspaces.deleteWorktree, ensures cache invalidation (getWorktreesByProject.invalidate) and re-exports the hook.

Sequence Diagram

sequenceDiagram
    actor User
    participant UI as DeleteWorktreeDialog
    participant TRPCClient as TRPC Client
    participant Server as Backend Server
    participant Cache as React Query Cache

    User->>UI: Open delete dialog
    UI->>TRPCClient: canDeleteWorktree(worktreeId)
    TRPCClient->>Server: validate deletion (resolve worktree & project, git checks)
    Server-->>TRPCClient: canDelete, reason, hasChanges, hasUnpushedCommits
    TRPCClient-->>UI: display eligibility & warnings

    User->>UI: Click Delete
    UI->>TRPCClient: deleteWorktree(worktreeId)
    TRPCClient->>Server: delete worktree (acquire lock, teardown, remove worktree, delete record)
    Server-->>TRPCClient: success/error
    TRPCClient->>Cache: invalidate worktrees cache
    TRPCClient-->>UI: success/error toast
    UI->>User: update UI / close dialog
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly related PRs

Poem

🐇 I nibbled code and found a tree,
That closed its doors quite quietly.
A dialog popped, a carrot click,
One hop, one delete — gone so quick.
Now fresh branches sprout anew, hooray! 🌿

🚥 Pre-merge checks | ❌ 3
❌ Failed checks (2 warnings, 1 inconclusive)
Check name Status Explanation Resolution
Description check ⚠️ Warning The PR description contains only empty template sections with no actual content describing the changes, related issues, type of change, testing performed, or any additional context. Fill in all relevant template sections with details about the new worktree deletion feature, related issues, testing verification, and any important implementation notes.
Docstring Coverage ⚠️ Warning Docstring coverage is 33.33% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
Title check ❓ Inconclusive The title 'feat (desktop): allow deleting on workspaces page' is somewhat generic and lacks specificity about the main change; it doesn't clarify that worktree deletion functionality is being added. Consider more specific phrasing like 'feat (desktop): add worktree deletion with confirmation dialog' to better capture the actual implementation scope.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
  • 📝 Generate docstrings

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.

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: 2

🤖 Fix all issues with AI agents
In
@apps/desktop/src/renderer/screens/main/components/WorkspacesListView/WorkspaceRow/DeleteWorktreeDialog.tsx:
- Around line 107-115: The Delete button can still be clicked rapidly because it
only checks canDelete and isLoading; include the mutation's pending state
(isPending) in the disabled condition to block double-clicks. Update the
Button's disabled prop used with the onClick handler (handleDelete) to combine
canDelete, isLoading and isPending (e.g., disabled={!canDelete || isLoading ||
isPending}), and ensure you read isPending from the delete mutation hook or
state (the same place isLoading is sourced) so the button is disabled while the
delete mutation is in progress.
- Line 55: The canDelete fallback currently defaults to true which can briefly
enable deletion before the query resolves; change the assignment of canDelete
(from canDeleteData?.canDelete ?? true) to default to false instead
(canDeleteData?.canDelete ?? false) so the delete action remains disabled until
the query confirms permission; update any related UI logic that relies on
canDelete (e.g., the Delete button enabled/disabled) to use this new default.
🧹 Nitpick comments (4)
apps/desktop/src/renderer/screens/main/components/WorkspacesListView/WorkspaceRow/WorkspaceRow.tsx (1)

187-197: Clarify variable naming for better readability.

The variable isOpenWorkspace checks if workspaceId !== null, but this actually indicates whether the workspace has an associated workspace record (which could be open or closed). Consider renaming to hasWorkspaceId for clarity.

Also, consider extracting these derived values to improve readability:

♻️ Suggested naming improvement
-	// Determine the delete/close action label based on workspace type and state
-	const isOpenWorkspace = workspace.workspaceId !== null;
-	const isClosedWorktree = !isOpenWorkspace && workspace.worktreeId !== null;
+	// Determine the delete/close action label based on workspace type and state
+	const hasWorkspaceId = workspace.workspaceId !== null;
+	const isClosedWorktree = !hasWorkspaceId && workspace.worktreeId !== null;
apps/desktop/src/lib/trpc/routers/workspaces/procedures/delete.ts (3)

342-377: Consider extracting shared worktree status checking logic.

The worktree existence check and git status checking logic (lines 342-377) is duplicated from the canDelete procedure (lines 95-138). Per coding guidelines, consider extracting this into a shared utility function for reuse and easier testing.

♻️ Example extraction
// In a shared utility file, e.g., ../utils/worktree-status.ts
interface WorktreeStatusResult {
  exists: boolean;
  hasChanges: boolean;
  hasUnpushedCommits: boolean;
  error?: string;
}

export async function checkWorktreeStatus(
  mainRepoPath: string,
  worktreePath: string,
): Promise<WorktreeStatusResult> {
  try {
    const exists = await worktreeExists(mainRepoPath, worktreePath);
    if (!exists) {
      return { exists: false, hasChanges: false, hasUnpushedCommits: false };
    }
    const [hasChanges, unpushedCommits] = await Promise.all([
      hasUncommittedChanges(worktreePath),
      hasUnpushedCommits(worktreePath),
    ]);
    return { exists: true, hasChanges, hasUnpushedCommits: unpushedCommits };
  } catch (error) {
    return {
      exists: false,
      hasChanges: false,
      hasUnpushedCommits: false,
      error: error instanceof Error ? error.message : String(error),
    };
  }
}

Based on learnings, business logic exceeding ~50 lines or used by multiple procedures should be extracted.


399-432: Optional: Consider extracting worktree removal logic.

The teardown-and-remove pattern (lines 399-432) is similar to the logic in the delete mutation (lines 202-243). A shared removeWorktreeWithTeardown utility could reduce duplication. This is optional since the delete mutation has additional workspace-specific concerns.


409-427: Use prefixed logging pattern for consistency.

Per coding guidelines, use the [domain/operation] prefix pattern for console logs, consistent with other logging in this file (e.g., line 166: [workspace/delete]).

♻️ Proposed fix
 					if (!teardownResult.success) {
 						console.error(
-							`Teardown failed for worktree ${worktree.branch}:`,
+							`[worktree/delete] Teardown failed for ${worktree.branch}:`,
 							teardownResult.error,
 						);
 					}
 ...
 					} else {
 						console.warn(
-							`Worktree ${worktree.path} not found in git, skipping removal`,
+							`[worktree/delete] Worktree ${worktree.path} not found in git, skipping removal`,
 						);
 					}
 ...
 					const errorMessage =
 						error instanceof Error ? error.message : String(error);
-					console.error("Failed to remove worktree:", errorMessage);
+					console.error("[worktree/delete] Failed to remove worktree:", errorMessage);
📜 Review details

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between a77cba2 and 80e6c91.

📒 Files selected for processing (3)
  • apps/desktop/src/lib/trpc/routers/workspaces/procedures/delete.ts
  • apps/desktop/src/renderer/screens/main/components/WorkspacesListView/WorkspaceRow/DeleteWorktreeDialog.tsx
  • apps/desktop/src/renderer/screens/main/components/WorkspacesListView/WorkspaceRow/WorkspaceRow.tsx
🧰 Additional context used
📓 Path-based instructions (6)
apps/desktop/**/*.{ts,tsx}

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

apps/desktop/**/*.{ts,tsx}: For Electron interprocess communication, ALWAYS use tRPC as defined in src/lib/trpc
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.
For tRPC subscriptions with trpc-electron, ALWAYS use the observable pattern from @trpc/server/observable instead of async generators, as the library explicitly checks isObservable(result) and throws an error otherwise

Files:

  • apps/desktop/src/lib/trpc/routers/workspaces/procedures/delete.ts
  • apps/desktop/src/renderer/screens/main/components/WorkspacesListView/WorkspaceRow/WorkspaceRow.tsx
  • apps/desktop/src/renderer/screens/main/components/WorkspacesListView/WorkspaceRow/DeleteWorktreeDialog.tsx
**/*.{ts,tsx}

📄 CodeRabbit inference engine (AGENTS.md)

**/*.{ts,tsx}: Use object parameters for functions with 2+ parameters instead of positional arguments
Functions with 2+ parameters should accept a single params object with named properties for self-documentation and extensibility
Use prefixed console logging with context pattern: [domain/operation] message
Extract magic numbers and hardcoded values to named constants at module top
Use lookup objects/maps instead of repeated if (type === ...) conditionals
Avoid using any type - maintain type safety in TypeScript code
Never swallow errors silently - at minimum log them with context
Import from concrete files directly when possible - avoid barrel file abuse that creates circular dependencies
Avoid deep nesting (4+ levels) - use early returns, extract functions, and invert conditions
Use named properties in options objects instead of boolean parameters to avoid boolean blindness

Files:

  • apps/desktop/src/lib/trpc/routers/workspaces/procedures/delete.ts
  • apps/desktop/src/renderer/screens/main/components/WorkspacesListView/WorkspaceRow/WorkspaceRow.tsx
  • apps/desktop/src/renderer/screens/main/components/WorkspacesListView/WorkspaceRow/DeleteWorktreeDialog.tsx
apps/**/*.{ts,tsx}

📄 CodeRabbit inference engine (AGENTS.md)

Use Drizzle ORM for all database operations - never use raw SQL

Files:

  • apps/desktop/src/lib/trpc/routers/workspaces/procedures/delete.ts
  • apps/desktop/src/renderer/screens/main/components/WorkspacesListView/WorkspaceRow/WorkspaceRow.tsx
  • apps/desktop/src/renderer/screens/main/components/WorkspacesListView/WorkspaceRow/DeleteWorktreeDialog.tsx
**/*.{ts,tsx,js,jsx}

📄 CodeRabbit inference engine (AGENTS.md)

Use Biome for formatting and linting - run at root level with bun run lint:fix or biome check --write

Files:

  • apps/desktop/src/lib/trpc/routers/workspaces/procedures/delete.ts
  • apps/desktop/src/renderer/screens/main/components/WorkspacesListView/WorkspaceRow/WorkspaceRow.tsx
  • apps/desktop/src/renderer/screens/main/components/WorkspacesListView/WorkspaceRow/DeleteWorktreeDialog.tsx
apps/desktop/src/renderer/**/*.{ts,tsx}

📄 CodeRabbit inference engine (AGENTS.md)

Never import Node.js modules (fs, path, os, net) in renderer process or shared code - they are externalized for browser compatibility

Files:

  • apps/desktop/src/renderer/screens/main/components/WorkspacesListView/WorkspaceRow/WorkspaceRow.tsx
  • apps/desktop/src/renderer/screens/main/components/WorkspacesListView/WorkspaceRow/DeleteWorktreeDialog.tsx
**/*.tsx

📄 CodeRabbit inference engine (AGENTS.md)

One component per file - do not create multi-component files

Files:

  • apps/desktop/src/renderer/screens/main/components/WorkspacesListView/WorkspaceRow/WorkspaceRow.tsx
  • apps/desktop/src/renderer/screens/main/components/WorkspacesListView/WorkspaceRow/DeleteWorktreeDialog.tsx
🧠 Learnings (1)
📚 Learning: 2026-01-02T06:50:28.671Z
Learnt from: CR
Repo: superset-sh/superset PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-02T06:50:28.671Z
Learning: Applies to apps/api/**/*.{ts,tsx} : Extract business logic from tRPC procedures into utility functions when logic exceeds ~50 lines, is used by multiple procedures, or needs independent testing

Applied to files:

  • apps/desktop/src/lib/trpc/routers/workspaces/procedures/delete.ts
🧬 Code graph analysis (2)
apps/desktop/src/lib/trpc/routers/workspaces/procedures/delete.ts (6)
apps/desktop/src/lib/trpc/index.ts (1)
  • publicProcedure (49-49)
apps/desktop/src/lib/trpc/routers/workspaces/utils/db-helpers.ts (4)
  • getWorktree (178-184)
  • getProject (167-173)
  • deleteWorktreeRecord (259-261)
  • hideProjectIfNoWorkspaces (98-107)
apps/desktop/src/lib/trpc/routers/workspaces/utils/git.ts (4)
  • worktreeExists (244-259)
  • hasUncommittedChanges (401-407)
  • hasUnpushedCommits (409-434)
  • removeWorktree (213-232)
apps/desktop/src/main/lib/workspace-init-manager.ts (1)
  • workspaceInitManager (302-302)
apps/desktop/src/lib/trpc/routers/workspaces/utils/teardown.ts (1)
  • runTeardown (41-80)
apps/desktop/src/main/lib/analytics/index.ts (1)
  • track (27-46)
apps/desktop/src/renderer/screens/main/components/WorkspacesListView/WorkspaceRow/DeleteWorktreeDialog.tsx (5)
packages/ui/src/components/ui/sonner.tsx (1)
  • toast (40-40)
apps/desktop/src/lib/trpc/routers/workspaces/utils/git.ts (1)
  • hasUnpushedCommits (409-434)
packages/ui/src/components/ui/alert-dialog.tsx (6)
  • AlertDialog (146-146)
  • AlertDialogContent (150-150)
  • AlertDialogHeader (151-151)
  • AlertDialogTitle (153-153)
  • AlertDialogDescription (154-154)
  • AlertDialogFooter (152-152)
packages/ui/src/components/ui/button.tsx (1)
  • Button (60-60)
packages/ui/src/components/ui/tooltip.tsx (3)
  • Tooltip (76-76)
  • TooltipTrigger (76-76)
  • TooltipContent (76-76)
⏰ 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 (7)
apps/desktop/src/renderer/screens/main/components/WorkspacesListView/WorkspaceRow/WorkspaceRow.tsx (2)

214-233: LGTM on dialog rendering logic.

The conditions for rendering DeleteWorkspaceDialog (when workspaceId exists) and DeleteWorktreeDialog (when isClosedWorktree with worktreeId) are mutually exclusive, so sharing showDeleteDialog state is safe.


199-212: Context menu implementation looks good.

The ContextMenu wraps the button correctly using asChild, the destructive action is properly styled, and the canDelete state controls the disabled state appropriately.

apps/desktop/src/renderer/screens/main/components/WorkspacesListView/WorkspaceRow/DeleteWorktreeDialog.tsx (3)

44-53: LGTM on the delete handler pattern.

Closing the dialog immediately and using toast.promise for feedback is a good UX pattern - the user doesn't have to wait for the deletion to complete, and errors are clearly communicated via toast notifications.


35-42: LGTM on query configuration.

Using staleTime: Number.POSITIVE_INFINITY is appropriate here since the query re-fetches each time the dialog opens (via enabled: open), and the worktree status is unlikely to change while the dialog remains open.


84-94: LGTM on the warning UI implementation.

The amber-colored warning banner clearly communicates potential data loss risks (uncommitted changes, unpushed commits) while still allowing the user to proceed with deletion.

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

297-378: LGTM on canDeleteWorktree procedure.

The procedure correctly:

  • Validates worktree and project existence
  • Supports skipGitChecks for optimization
  • Checks worktree existence in git and handles the "manually removed" case
  • Runs git status checks in parallel for efficiency
  • Returns consistent response shape with appropriate error messages

396-443: Lock handling and cleanup sequence looks correct.

The lock is properly acquired before filesystem operations and released in a finally block. The DB cleanup happening after lock release is acceptable since the critical filesystem operations are protected, and the pattern matches the existing delete mutation.

});
};

const canDelete = canDeleteData?.canDelete ?? true;
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

Default canDelete to false to prevent premature deletion.

Defaulting canDelete to true before the query completes could allow the user to click "Delete" during the brief window before the query returns. Since the button is also disabled when isLoading, this is mitigated, but the default should still be false for defense-in-depth.

🐛 Proposed fix
-	const canDelete = canDeleteData?.canDelete ?? true;
+	const canDelete = canDeleteData?.canDelete ?? false;
📝 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
const canDelete = canDeleteData?.canDelete ?? true;
const canDelete = canDeleteData?.canDelete ?? false;
🤖 Prompt for AI Agents
In
@apps/desktop/src/renderer/screens/main/components/WorkspacesListView/WorkspaceRow/DeleteWorktreeDialog.tsx
at line 55, The canDelete fallback currently defaults to true which can briefly
enable deletion before the query resolves; change the assignment of canDelete
(from canDeleteData?.canDelete ?? true) to default to false instead
(canDeleteData?.canDelete ?? false) so the delete action remains disabled until
the query confirms permission; update any related UI logic that relies on
canDelete (e.g., the Delete button enabled/disabled) to use this new default.

Comment on lines +107 to +115
<Button
variant="destructive"
size="sm"
className="h-7 px-3 text-xs"
onClick={handleDelete}
disabled={!canDelete || isLoading}
>
Delete
</Button>
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

Add isPending check to prevent double-clicks.

The delete button should also be disabled while the mutation is in progress to prevent multiple deletion attempts if the user clicks rapidly before the dialog closes.

🐛 Proposed fix
 							<Button
 								variant="destructive"
 								size="sm"
 								className="h-7 px-3 text-xs"
 								onClick={handleDelete}
-								disabled={!canDelete || isLoading}
+								disabled={!canDelete || isLoading || deleteWorktree.isPending}
 							>
 								Delete
 							</Button>
🤖 Prompt for AI Agents
In
@apps/desktop/src/renderer/screens/main/components/WorkspacesListView/WorkspaceRow/DeleteWorktreeDialog.tsx
around lines 107 - 115, The Delete button can still be clicked rapidly because
it only checks canDelete and isLoading; include the mutation's pending state
(isPending) in the disabled condition to block double-clicks. Update the
Button's disabled prop used with the onClick handler (handleDelete) to combine
canDelete, isLoading and isPending (e.g., disabled={!canDelete || isLoading ||
isPending}), and ensure you read isPending from the delete mutation hook or
state (the same place isLoading is sourced) so the button is disabled while the
delete mutation is in progress.

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 (4)
apps/desktop/src/renderer/react-query/workspaces/useDeleteWorktree.ts (1)

19-24: Consider removing redundant callback wrappers.

The onSuccess and onError handlers only forward to the options callbacks without adding any additional logic. Since ...options is already spread, these callbacks would be passed through automatically. Only onSettled needs to be overridden because it adds cache invalidation logic.

♻️ Suggested simplification
 	return trpc.workspaces.deleteWorktree.useMutation({
 		...options,
 		onSettled: async (...args) => {
 			// Invalidate worktree queries to refresh the list
 			await utils.workspaces.getWorktreesByProject.invalidate();
 			await options?.onSettled?.(...args);
 		},
-		onSuccess: async (...args) => {
-			await options?.onSuccess?.(...args);
-		},
-		onError: async (...args) => {
-			await options?.onError?.(...args);
-		},
 	});
apps/desktop/src/renderer/screens/main/components/WorkspacesListView/WorkspaceRow/DeleteWorktreeDialog.tsx (1)

30-37: Consider reducing staleTime for fresher git status.

Using Number.POSITIVE_INFINITY for staleTime means the canDeleteWorktree data never refreshes once fetched. If a user opens the dialog, closes it, makes commits or changes, then reopens the dialog, the warnings (hasChanges, hasUnpushedCommits) will be stale.

Consider using a shorter staleTime (e.g., 0 or a few seconds) to ensure fresh git status checks when the dialog opens.

♻️ Suggested change
 	const { data: canDeleteData, isLoading } =
 		trpc.workspaces.canDeleteWorktree.useQuery(
 			{ worktreeId },
 			{
 				enabled: open,
-				staleTime: Number.POSITIVE_INFINITY,
+				staleTime: 0, // Always fetch fresh git status when dialog opens
 			},
 		);
apps/desktop/src/lib/trpc/routers/workspaces/procedures/delete.ts (2)

360-362: Variable shadows imported function name.

The local variable unpushedCommits on line 362 is assigned from hasUnpushedCommits(), which is also the name of the imported function (line 20). While this works, it could cause confusion. The existing pattern in canDelete (line 117) uses the same shadowing pattern, so this is consistent but worth noting.

♻️ Alternative naming for clarity
 				const [hasChanges, unpushedCommits] = await Promise.all([
 					hasUncommittedChanges(worktree.path),
-					hasUnpushedCommits(worktree.path),
+					hasUnpushedCommits(worktree.path), // consider: checkHasUnpushedCommits or getHasUnpushedCommits for the import
 				]);

Alternatively, rename the import to avoid shadowing:

import {
	hasUncommittedChanges,
	hasUnpushedCommits as checkUnpushedCommits,
	// ...
} from "../utils/git";

298-450: Consider extracting shared git-check logic (optional).

The git status checking logic in canDeleteWorktree (lines 342-381) is similar to the worktree checks in canDelete (lines 90-139). Per coding guidelines, consider extracting to a shared utility if this pattern is used elsewhere or grows. For now, the duplication is acceptable given the slightly different contexts and return types.

📜 Review details

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 80e6c91 and c3cfcb7.

📒 Files selected for processing (5)
  • apps/desktop/src/lib/trpc/routers/workspaces/procedures/delete.ts
  • apps/desktop/src/renderer/react-query/workspaces/index.ts
  • apps/desktop/src/renderer/react-query/workspaces/useDeleteWorktree.ts
  • apps/desktop/src/renderer/screens/main/components/WorkspacesListView/WorkspaceRow/DeleteWorktreeDialog.tsx
  • apps/desktop/src/renderer/screens/main/components/WorkspacesListView/WorkspaceRow/WorkspaceRow.tsx
🧰 Additional context used
📓 Path-based instructions (6)
apps/desktop/**/*.{ts,tsx}

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

apps/desktop/**/*.{ts,tsx}: For Electron interprocess communication, ALWAYS use tRPC as defined in src/lib/trpc
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.
For tRPC subscriptions with trpc-electron, ALWAYS use the observable pattern from @trpc/server/observable instead of async generators, as the library explicitly checks isObservable(result) and throws an error otherwise

Files:

  • apps/desktop/src/renderer/react-query/workspaces/useDeleteWorktree.ts
  • apps/desktop/src/renderer/screens/main/components/WorkspacesListView/WorkspaceRow/WorkspaceRow.tsx
  • apps/desktop/src/lib/trpc/routers/workspaces/procedures/delete.ts
  • apps/desktop/src/renderer/screens/main/components/WorkspacesListView/WorkspaceRow/DeleteWorktreeDialog.tsx
  • apps/desktop/src/renderer/react-query/workspaces/index.ts
**/*.{ts,tsx}

📄 CodeRabbit inference engine (AGENTS.md)

**/*.{ts,tsx}: Use object parameters for functions with 2+ parameters instead of positional arguments
Functions with 2+ parameters should accept a single params object with named properties for self-documentation and extensibility
Use prefixed console logging with context pattern: [domain/operation] message
Extract magic numbers and hardcoded values to named constants at module top
Use lookup objects/maps instead of repeated if (type === ...) conditionals
Avoid using any type - maintain type safety in TypeScript code
Never swallow errors silently - at minimum log them with context
Import from concrete files directly when possible - avoid barrel file abuse that creates circular dependencies
Avoid deep nesting (4+ levels) - use early returns, extract functions, and invert conditions
Use named properties in options objects instead of boolean parameters to avoid boolean blindness

Files:

  • apps/desktop/src/renderer/react-query/workspaces/useDeleteWorktree.ts
  • apps/desktop/src/renderer/screens/main/components/WorkspacesListView/WorkspaceRow/WorkspaceRow.tsx
  • apps/desktop/src/lib/trpc/routers/workspaces/procedures/delete.ts
  • apps/desktop/src/renderer/screens/main/components/WorkspacesListView/WorkspaceRow/DeleteWorktreeDialog.tsx
  • apps/desktop/src/renderer/react-query/workspaces/index.ts
apps/desktop/src/renderer/**/*.{ts,tsx}

📄 CodeRabbit inference engine (AGENTS.md)

Never import Node.js modules (fs, path, os, net) in renderer process or shared code - they are externalized for browser compatibility

Files:

  • apps/desktop/src/renderer/react-query/workspaces/useDeleteWorktree.ts
  • apps/desktop/src/renderer/screens/main/components/WorkspacesListView/WorkspaceRow/WorkspaceRow.tsx
  • apps/desktop/src/renderer/screens/main/components/WorkspacesListView/WorkspaceRow/DeleteWorktreeDialog.tsx
  • apps/desktop/src/renderer/react-query/workspaces/index.ts
apps/**/*.{ts,tsx}

📄 CodeRabbit inference engine (AGENTS.md)

Use Drizzle ORM for all database operations - never use raw SQL

Files:

  • apps/desktop/src/renderer/react-query/workspaces/useDeleteWorktree.ts
  • apps/desktop/src/renderer/screens/main/components/WorkspacesListView/WorkspaceRow/WorkspaceRow.tsx
  • apps/desktop/src/lib/trpc/routers/workspaces/procedures/delete.ts
  • apps/desktop/src/renderer/screens/main/components/WorkspacesListView/WorkspaceRow/DeleteWorktreeDialog.tsx
  • apps/desktop/src/renderer/react-query/workspaces/index.ts
**/*.{ts,tsx,js,jsx}

📄 CodeRabbit inference engine (AGENTS.md)

Use Biome for formatting and linting - run at root level with bun run lint:fix or biome check --write

Files:

  • apps/desktop/src/renderer/react-query/workspaces/useDeleteWorktree.ts
  • apps/desktop/src/renderer/screens/main/components/WorkspacesListView/WorkspaceRow/WorkspaceRow.tsx
  • apps/desktop/src/lib/trpc/routers/workspaces/procedures/delete.ts
  • apps/desktop/src/renderer/screens/main/components/WorkspacesListView/WorkspaceRow/DeleteWorktreeDialog.tsx
  • apps/desktop/src/renderer/react-query/workspaces/index.ts
**/*.tsx

📄 CodeRabbit inference engine (AGENTS.md)

One component per file - do not create multi-component files

Files:

  • apps/desktop/src/renderer/screens/main/components/WorkspacesListView/WorkspaceRow/WorkspaceRow.tsx
  • apps/desktop/src/renderer/screens/main/components/WorkspacesListView/WorkspaceRow/DeleteWorktreeDialog.tsx
🧠 Learnings (2)
📚 Learning: 2026-01-02T06:50:28.671Z
Learnt from: CR
Repo: superset-sh/superset PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-02T06:50:28.671Z
Learning: Applies to apps/api/**/*.{ts,tsx} : Extract business logic from tRPC procedures into utility functions when logic exceeds ~50 lines, is used by multiple procedures, or needs independent testing

Applied to files:

  • apps/desktop/src/lib/trpc/routers/workspaces/procedures/delete.ts
📚 Learning: 2026-01-02T06:50:28.671Z
Learnt from: CR
Repo: superset-sh/superset PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-02T06:50:28.671Z
Learning: Applies to apps/desktop/src/renderer/**/*.{ts,tsx} : Never import Node.js modules (fs, path, os, net) in renderer process or shared code - they are externalized for browser compatibility

Applied to files:

  • apps/desktop/src/renderer/react-query/workspaces/index.ts
🧬 Code graph analysis (1)
apps/desktop/src/renderer/react-query/workspaces/useDeleteWorktree.ts (1)
apps/desktop/src/renderer/react-query/workspaces/index.ts (1)
  • useDeleteWorktree (5-5)
⏰ 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 (7)
apps/desktop/src/renderer/react-query/workspaces/index.ts (1)

5-5: LGTM!

The new useDeleteWorktree export follows the established pattern and maintains alphabetical ordering.

apps/desktop/src/renderer/screens/main/components/WorkspacesListView/WorkspaceRow/WorkspaceRow.tsx (2)

187-197: LGTM! Clear state derivation logic.

The actionLabel and canDelete logic correctly handles the three workspace states (branch, closed worktree, open workspace) with mutually exclusive conditions.


214-233: Dialog rendering relies on implicit mutual exclusivity.

The conditions for rendering DeleteWorkspaceDialog (workspace has workspaceId) and DeleteWorktreeDialog (isClosedWorktree) are mutually exclusive by definition, so sharing showDeleteDialog state is safe. The logic is correct.

apps/desktop/src/renderer/screens/main/components/WorkspacesListView/WorkspaceRow/DeleteWorktreeDialog.tsx (2)

39-48: LGTM! Good UX pattern.

Closing the dialog immediately and using toast.promise for async feedback keeps the UI responsive while providing clear progress/success/error states.


79-89: LGTM! Clear warning presentation.

The warning box properly handles all three warning scenarios with appropriate conditional messaging and uses semantic amber colors for visibility.

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

298-382: Code structure follows established patterns.

The canDeleteWorktree procedure appropriately mirrors the existing canDelete procedure's structure and error handling. The git status checks with skipGitChecks optimization and comprehensive return types are well-designed.


384-450: LGTM! Proper lock management and cleanup sequence.

The deleteWorktree mutation correctly:

  • Acquires project lock before git operations
  • Uses try/finally to ensure lock release
  • Handles worktree non-existence gracefully
  • Cleans up DB records after git operations
  • Tracks analytics

@AviPeltz AviPeltz merged commit aead79a into main Jan 12, 2026
5 checks passed
@github-actions
Copy link
Copy Markdown
Contributor

🧹 Preview Cleanup Complete

The following preview resources have been cleaned up:

  • ⚠️ Neon database branch
  • ⚠️ Electric Fly.io app

Thank you for your contribution! 🎉

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