Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
154 changes: 154 additions & 0 deletions apps/desktop/src/lib/trpc/routers/workspaces/procedures/delete.ts
Original file line number Diff line number Diff line change
Expand Up @@ -294,5 +294,159 @@ export const createDeleteProcedures = () => {

return { success: true, terminalWarning };
}),

// Check if a closed worktree (no active workspace) can be deleted
canDeleteWorktree: publicProcedure
.input(
z.object({
worktreeId: z.string(),
skipGitChecks: z.boolean().optional(),
}),
)
.query(async ({ input }) => {
const worktree = getWorktree(input.worktreeId);

if (!worktree) {
return {
canDelete: false,
reason: "Worktree not found",
worktree: null,
hasChanges: false,
hasUnpushedCommits: false,
};
}

const project = getProject(worktree.projectId);

if (!project) {
return {
canDelete: false,
reason: "Project not found",
worktree,
hasChanges: false,
hasUnpushedCommits: false,
};
}

if (input.skipGitChecks) {
return {
canDelete: true,
reason: null,
worktree,
warning: null,
hasChanges: false,
hasUnpushedCommits: false,
};
}

try {
const exists = await worktreeExists(
project.mainRepoPath,
worktree.path,
);

if (!exists) {
return {
canDelete: true,
reason: null,
worktree,
warning:
"Worktree not found in git (may have been manually removed)",
hasChanges: false,
hasUnpushedCommits: false,
};
}

const [hasChanges, unpushedCommits] = await Promise.all([
hasUncommittedChanges(worktree.path),
hasUnpushedCommits(worktree.path),
]);

return {
canDelete: true,
reason: null,
worktree,
warning: null,
hasChanges,
hasUnpushedCommits: unpushedCommits,
};
} catch (error) {
return {
canDelete: false,
reason: `Failed to check worktree status: ${error instanceof Error ? error.message : String(error)}`,
worktree,
hasChanges: false,
hasUnpushedCommits: false,
};
}
}),

// Delete a closed worktree (no active workspace) by worktree ID
deleteWorktree: publicProcedure
.input(z.object({ worktreeId: z.string() }))
.mutation(async ({ input }) => {
const worktree = getWorktree(input.worktreeId);

if (!worktree) {
return { success: false, error: "Worktree not found" };
}

const project = getProject(worktree.projectId);

if (!project) {
return { success: false, error: "Project not found" };
}

// Acquire project lock to prevent racing with concurrent operations
await workspaceInitManager.acquireProjectLock(project.id);

try {
const exists = await worktreeExists(
project.mainRepoPath,
worktree.path,
);

if (exists) {
const teardownResult = await runTeardown(
project.mainRepoPath,
worktree.path,
worktree.branch,
);
if (!teardownResult.success) {
console.error(
`Teardown failed for worktree ${worktree.branch}:`,
teardownResult.error,
);
}
}

try {
if (exists) {
await removeWorktree(project.mainRepoPath, worktree.path);
} else {
console.warn(
`Worktree ${worktree.path} not found in git, skipping removal`,
);
}
} catch (error) {
const errorMessage =
error instanceof Error ? error.message : String(error);
console.error("Failed to remove worktree:", errorMessage);
return {
success: false,
error: `Failed to remove worktree: ${errorMessage}`,
};
}
} finally {
workspaceInitManager.releaseProjectLock(project.id);
}

deleteWorktreeRecord(input.worktreeId);
hideProjectIfNoWorkspaces(worktree.projectId);

track("worktree_deleted", { worktree_id: input.worktreeId });

return { success: true };
}),
});
};
1 change: 1 addition & 0 deletions apps/desktop/src/renderer/react-query/workspaces/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ export { useCloseWorkspace } from "./useCloseWorkspace";
export { useCreateBranchWorkspace } from "./useCreateBranchWorkspace";
export { useCreateWorkspace } from "./useCreateWorkspace";
export { useDeleteWorkspace } from "./useDeleteWorkspace";
export { useDeleteWorktree } from "./useDeleteWorktree";
export { useOpenWorktree } from "./useOpenWorktree";
export { useReorderWorkspaces } from "./useReorderWorkspaces";
export { useSetActiveWorkspace } from "./useSetActiveWorkspace";
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
import { trpc } from "renderer/lib/trpc";

/**
* Mutation hook for deleting a closed worktree (one without an active workspace).
* Handles cache invalidation for worktree-related queries.
*/
export function useDeleteWorktree(
options?: Parameters<typeof trpc.workspaces.deleteWorktree.useMutation>[0],
) {
const utils = trpc.useUtils();

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);
},
});
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,120 @@
import {
AlertDialog,
AlertDialogContent,
AlertDialogDescription,
AlertDialogFooter,
AlertDialogHeader,
AlertDialogTitle,
} from "@superset/ui/alert-dialog";
import { Button } from "@superset/ui/button";
import { toast } from "@superset/ui/sonner";
import { Tooltip, TooltipContent, TooltipTrigger } from "@superset/ui/tooltip";
import { trpc } from "renderer/lib/trpc";
import { useDeleteWorktree } from "renderer/react-query/workspaces/useDeleteWorktree";

interface DeleteWorktreeDialogProps {
worktreeId: string;
worktreeName: string;
open: boolean;
onOpenChange: (open: boolean) => void;
}

export function DeleteWorktreeDialog({
worktreeId,
worktreeName,
open,
onOpenChange,
}: DeleteWorktreeDialogProps) {
const deleteWorktree = useDeleteWorktree();

const { data: canDeleteData, isLoading } =
trpc.workspaces.canDeleteWorktree.useQuery(
{ worktreeId },
{
enabled: open,
staleTime: Number.POSITIVE_INFINITY,
},
);

const handleDelete = () => {
onOpenChange(false);

toast.promise(deleteWorktree.mutateAsync({ worktreeId }), {
loading: `Deleting "${worktreeName}"...`,
success: `Deleted "${worktreeName}"`,
error: (error) =>
error instanceof Error ? error.message : "Failed to delete",
});
};

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.

const reason = canDeleteData?.reason;
const hasChanges = canDeleteData?.hasChanges ?? false;
const hasUnpushedCommits = canDeleteData?.hasUnpushedCommits ?? false;
const hasWarnings = hasChanges || hasUnpushedCommits;

return (
<AlertDialog open={open} onOpenChange={onOpenChange}>
<AlertDialogContent className="max-w-[340px] gap-0 p-0">
<AlertDialogHeader className="px-4 pt-4 pb-2">
<AlertDialogTitle className="font-medium">
Delete worktree "{worktreeName}"?
</AlertDialogTitle>
<AlertDialogDescription asChild>
<div className="text-muted-foreground space-y-1.5">
{isLoading ? (
"Checking status..."
) : !canDelete ? (
<span className="text-destructive">{reason}</span>
) : (
<span className="block">
This will permanently delete the worktree and its files from
disk.
</span>
)}
</div>
</AlertDialogDescription>
</AlertDialogHeader>

{!isLoading && canDelete && hasWarnings && (
<div className="px-4 pb-2">
<div className="text-sm text-amber-700 dark:text-amber-300 bg-amber-100 dark:bg-amber-950/50 border border-amber-200 dark:border-amber-800 rounded px-2 py-1.5">
{hasChanges && hasUnpushedCommits
? "Has uncommitted changes and unpushed commits"
: hasChanges
? "Has uncommitted changes"
: "Has unpushed commits"}
</div>
</div>
)}

<AlertDialogFooter className="px-4 pb-4 pt-2 flex-row justify-end gap-2">
<Button
variant="ghost"
size="sm"
className="h-7 px-3 text-xs"
onClick={() => onOpenChange(false)}
>
Cancel
</Button>
<Tooltip delayDuration={400}>
<TooltipTrigger asChild>
<Button
variant="destructive"
size="sm"
className="h-7 px-3 text-xs"
onClick={handleDelete}
disabled={!canDelete || isLoading}
>
Delete
</Button>
Comment on lines +102 to +110
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.

</TooltipTrigger>
<TooltipContent side="top" className="text-xs max-w-[200px]">
Permanently delete worktree from disk.
</TooltipContent>
</Tooltip>
</AlertDialogFooter>
</AlertDialogContent>
</AlertDialog>
);
}
Loading
Loading