From 1f1feba98a5d74e8ca8be87a98733aef01697d25 Mon Sep 17 00:00:00 2001 From: AviPeltz Date: Tue, 6 Jan 2026 15:08:51 -0800 Subject: [PATCH 1/2] fix delete taking so long --- .../workspaces/useCloseWorkspace.ts | 115 +++++++++++++- .../workspaces/useDeleteWorkspace.ts | 115 +++++++++++++- plans/optimistic-workspace-delete.md | 140 ++++++++++++++++++ 3 files changed, 366 insertions(+), 4 deletions(-) create mode 100644 plans/optimistic-workspace-delete.md diff --git a/apps/desktop/src/renderer/react-query/workspaces/useCloseWorkspace.ts b/apps/desktop/src/renderer/react-query/workspaces/useCloseWorkspace.ts index e4c96b0fc86..b7ca5ed574d 100644 --- a/apps/desktop/src/renderer/react-query/workspaces/useCloseWorkspace.ts +++ b/apps/desktop/src/renderer/react-query/workspaces/useCloseWorkspace.ts @@ -1,8 +1,27 @@ import { trpc } from "renderer/lib/trpc"; +type CloseContext = { + previousGrouped: ReturnType< + typeof trpc.useUtils + >["workspaces"]["getAllGrouped"]["getData"] extends () => infer R + ? R + : never; + previousAll: ReturnType< + typeof trpc.useUtils + >["workspaces"]["getAll"]["getData"] extends () => infer R + ? R + : never; + previousActive: ReturnType< + typeof trpc.useUtils + >["workspaces"]["getActive"]["getData"] extends () => infer R + ? R + : never; +}; + /** * Mutation hook for closing a workspace without deleting the worktree - * Automatically invalidates all workspace queries on success + * Uses optimistic updates to immediately remove workspace from UI, + * then performs actual close in background. */ export function useCloseWorkspace( options?: Parameters[0], @@ -11,8 +30,100 @@ export function useCloseWorkspace( return trpc.workspaces.close.useMutation({ ...options, + onMutate: async ({ id }) => { + // Cancel outgoing refetches to avoid overwriting optimistic update + await Promise.all([ + utils.workspaces.getAll.cancel(), + utils.workspaces.getAllGrouped.cancel(), + utils.workspaces.getActive.cancel(), + ]); + + // Snapshot previous values for rollback + const previousGrouped = utils.workspaces.getAllGrouped.getData(); + const previousAll = utils.workspaces.getAll.getData(); + const previousActive = utils.workspaces.getActive.getData(); + + // Optimistically remove workspace from getAllGrouped cache + if (previousGrouped) { + utils.workspaces.getAllGrouped.setData( + undefined, + previousGrouped + .map((group) => ({ + ...group, + workspaces: group.workspaces.filter((w) => w.id !== id), + })) + .filter((group) => group.workspaces.length > 0), + ); + } + + // Optimistically remove workspace from getAll cache + if (previousAll) { + utils.workspaces.getAll.setData( + undefined, + previousAll.filter((w) => w.id !== id), + ); + } + + // If closing the active workspace, switch to another workspace optimistically + // This prevents a flash of "no workspace" state while the backend processes + if (previousActive?.id === id) { + // Find the next workspace to switch to (matches backend logic: most recently opened) + const remainingWorkspaces = previousAll + ?.filter((w) => w.id !== id) + .sort((a, b) => b.lastOpenedAt - a.lastOpenedAt); + + if (remainingWorkspaces && remainingWorkspaces.length > 0) { + const nextWorkspace = remainingWorkspaces[0]; + // Find the project info for the next workspace from grouped data + const projectGroup = previousGrouped?.find((g) => + g.workspaces.some((w) => w.id === nextWorkspace.id), + ); + const workspaceFromGrouped = projectGroup?.workspaces.find( + (w) => w.id === nextWorkspace.id, + ); + + if (projectGroup && workspaceFromGrouped) { + utils.workspaces.getActive.setData(undefined, { + ...nextWorkspace, + type: workspaceFromGrouped.type, + worktreePath: workspaceFromGrouped.worktreePath, + project: { + id: projectGroup.project.id, + name: projectGroup.project.name, + mainRepoPath: projectGroup.project.mainRepoPath, + }, + worktree: null, // Will be populated on invalidate + }); + } else { + // Fallback: just clear it and let invalidate handle it + utils.workspaces.getActive.setData(undefined, null); + } + } else { + // No remaining workspaces + utils.workspaces.getActive.setData(undefined, null); + } + } + + // Return context for rollback + return { previousGrouped, previousAll, previousActive } as CloseContext; + }, + onError: (_err, _variables, context) => { + // Rollback to previous state on error + if (context?.previousGrouped !== undefined) { + utils.workspaces.getAllGrouped.setData( + undefined, + context.previousGrouped, + ); + } + if (context?.previousAll !== undefined) { + utils.workspaces.getAll.setData(undefined, context.previousAll); + } + if (context?.previousActive !== undefined) { + utils.workspaces.getActive.setData(undefined, context.previousActive); + } + }, onSuccess: async (...args) => { - // Auto-invalidate all workspace queries + // Invalidate to ensure consistency with backend state await utils.workspaces.invalidate(); // Invalidate project queries since close updates project metadata await utils.projects.getRecents.invalidate(); diff --git a/apps/desktop/src/renderer/react-query/workspaces/useDeleteWorkspace.ts b/apps/desktop/src/renderer/react-query/workspaces/useDeleteWorkspace.ts index bb65e810272..a201be93eea 100644 --- a/apps/desktop/src/renderer/react-query/workspaces/useDeleteWorkspace.ts +++ b/apps/desktop/src/renderer/react-query/workspaces/useDeleteWorkspace.ts @@ -1,8 +1,27 @@ import { trpc } from "renderer/lib/trpc"; +type DeleteContext = { + previousGrouped: ReturnType< + typeof trpc.useUtils + >["workspaces"]["getAllGrouped"]["getData"] extends () => infer R + ? R + : never; + previousAll: ReturnType< + typeof trpc.useUtils + >["workspaces"]["getAll"]["getData"] extends () => infer R + ? R + : never; + previousActive: ReturnType< + typeof trpc.useUtils + >["workspaces"]["getActive"]["getData"] extends () => infer R + ? R + : never; +}; + /** * Mutation hook for deleting a workspace - * Automatically invalidates all workspace queries on success + * Uses optimistic updates to immediately remove workspace from UI, + * then performs actual deletion in background. */ export function useDeleteWorkspace( options?: Parameters[0], @@ -11,8 +30,100 @@ export function useDeleteWorkspace( return trpc.workspaces.delete.useMutation({ ...options, + onMutate: async ({ id }) => { + // Cancel outgoing refetches to avoid overwriting optimistic update + await Promise.all([ + utils.workspaces.getAll.cancel(), + utils.workspaces.getAllGrouped.cancel(), + utils.workspaces.getActive.cancel(), + ]); + + // Snapshot previous values for rollback + const previousGrouped = utils.workspaces.getAllGrouped.getData(); + const previousAll = utils.workspaces.getAll.getData(); + const previousActive = utils.workspaces.getActive.getData(); + + // Optimistically remove workspace from getAllGrouped cache + if (previousGrouped) { + utils.workspaces.getAllGrouped.setData( + undefined, + previousGrouped + .map((group) => ({ + ...group, + workspaces: group.workspaces.filter((w) => w.id !== id), + })) + .filter((group) => group.workspaces.length > 0), + ); + } + + // Optimistically remove workspace from getAll cache + if (previousAll) { + utils.workspaces.getAll.setData( + undefined, + previousAll.filter((w) => w.id !== id), + ); + } + + // If deleting the active workspace, switch to another workspace optimistically + // This prevents a flash of "no workspace" state while the backend processes + if (previousActive?.id === id) { + // Find the next workspace to switch to (matches backend logic: most recently opened) + const remainingWorkspaces = previousAll + ?.filter((w) => w.id !== id) + .sort((a, b) => b.lastOpenedAt - a.lastOpenedAt); + + if (remainingWorkspaces && remainingWorkspaces.length > 0) { + const nextWorkspace = remainingWorkspaces[0]; + // Find the project info for the next workspace from grouped data + const projectGroup = previousGrouped?.find((g) => + g.workspaces.some((w) => w.id === nextWorkspace.id), + ); + const workspaceFromGrouped = projectGroup?.workspaces.find( + (w) => w.id === nextWorkspace.id, + ); + + if (projectGroup && workspaceFromGrouped) { + utils.workspaces.getActive.setData(undefined, { + ...nextWorkspace, + type: workspaceFromGrouped.type, + worktreePath: workspaceFromGrouped.worktreePath, + project: { + id: projectGroup.project.id, + name: projectGroup.project.name, + mainRepoPath: projectGroup.project.mainRepoPath, + }, + worktree: null, // Will be populated on invalidate + }); + } else { + // Fallback: just clear it and let invalidate handle it + utils.workspaces.getActive.setData(undefined, null); + } + } else { + // No remaining workspaces + utils.workspaces.getActive.setData(undefined, null); + } + } + + // Return context for rollback + return { previousGrouped, previousAll, previousActive } as DeleteContext; + }, + onError: (_err, _variables, context) => { + // Rollback to previous state on error + if (context?.previousGrouped !== undefined) { + utils.workspaces.getAllGrouped.setData( + undefined, + context.previousGrouped, + ); + } + if (context?.previousAll !== undefined) { + utils.workspaces.getAll.setData(undefined, context.previousAll); + } + if (context?.previousActive !== undefined) { + utils.workspaces.getActive.setData(undefined, context.previousActive); + } + }, onSuccess: async (...args) => { - // Auto-invalidate all workspace queries + // Invalidate to ensure consistency with backend state await utils.workspaces.invalidate(); // Call user's onSuccess if provided diff --git a/plans/optimistic-workspace-delete.md b/plans/optimistic-workspace-delete.md new file mode 100644 index 00000000000..544d94515ae --- /dev/null +++ b/plans/optimistic-workspace-delete.md @@ -0,0 +1,140 @@ +# Optimistic Workspace Deletion + +## Problem + +Deleting a workspace feels slow because the UI waits for the entire deletion process to complete before updating. The backend deletion involves: + +1. Waiting for init to complete (up to 30s if workspace is initializing) +2. Killing terminal processes (2-3s) +3. Acquiring project lock +4. Running teardown scripts (fire-and-forget) +5. `git worktree remove --force` (can take several seconds for large directories) +6. Database cleanup + +## Solution + +Implement optimistic UI updates so the workspace is removed from the UI immediately, while the actual deletion happens in the background. + +## Implementation Plan + +### 1. Identify the delete trigger point + +**File:** `apps/desktop/src/renderer/react-query/workspaces/useDeleteWorkspace.ts` + +This hook wraps `trpc.workspaces.delete.useMutation`. We need to add optimistic update logic here. + +### 2. Add optimistic update to useDeleteWorkspace + +```typescript +export function useDeleteWorkspace( + options?: Parameters[0], +) { + const utils = trpc.useUtils(); + + return trpc.workspaces.delete.useMutation({ + ...options, + onMutate: async ({ id }) => { + // Cancel outgoing refetches + await utils.workspaces.getAll.cancel(); + await utils.workspaces.getAllGrouped.cancel(); + await utils.workspaces.getActive.cancel(); + + // Snapshot previous value + const previousGrouped = utils.workspaces.getAllGrouped.getData(); + const previousAll = utils.workspaces.getAll.getData(); + const previousActive = utils.workspaces.getActive.getData(); + + // Optimistically remove workspace from cache + if (previousGrouped) { + utils.workspaces.getAllGrouped.setData(undefined, + previousGrouped.map(group => ({ + ...group, + workspaces: group.workspaces.filter(w => w.id !== id) + })).filter(group => group.workspaces.length > 0) + ); + } + + if (previousAll) { + utils.workspaces.getAll.setData(undefined, + previousAll.filter(w => w.id !== id) + ); + } + + // If deleting active workspace, clear it + if (previousActive?.id === id) { + utils.workspaces.getActive.setData(undefined, null); + } + + // Return context for rollback + return { previousGrouped, previousAll, previousActive }; + }, + onError: (err, { id }, context) => { + // Rollback on error + if (context?.previousGrouped) { + utils.workspaces.getAllGrouped.setData(undefined, context.previousGrouped); + } + if (context?.previousAll) { + utils.workspaces.getAll.setData(undefined, context.previousAll); + } + if (context?.previousActive) { + utils.workspaces.getActive.setData(undefined, context.previousActive); + } + + // Show error toast + // toast.error(`Failed to delete workspace: ${err.message}`); + + // Call user's onError if provided + options?.onError?.(err, { id }, context); + }, + onSuccess: async (...args) => { + // Invalidate to ensure consistency (in background) + await utils.workspaces.invalidate(); + await options?.onSuccess?.(...args); + }, + }); +} +``` + +### 3. Handle active workspace switching + +When deleting the currently active workspace, we need to switch to another workspace immediately. Check: + +**File:** `apps/desktop/src/renderer/react-query/workspaces/useWorkspaceDeleteHandler.ts` + +This likely handles the UX around deletion. May need to: +- Pre-select the next workspace before deletion +- Update UI state optimistically + +### 4. Close dialog immediately + +**File:** `apps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/WorkspaceListItem/components/DeleteWorkspaceDialog/DeleteWorkspaceDialog.tsx` + +Ensure the delete confirmation dialog closes immediately when user confirms, not after mutation completes. + +### 5. Add error handling UI + +If the optimistic delete fails, we need to: +- Show a toast notification with the error +- The workspace will reappear in the list (rollback) + +## Files to Modify + +1. `apps/desktop/src/renderer/react-query/workspaces/useDeleteWorkspace.ts` - Add optimistic updates +2. `apps/desktop/src/renderer/react-query/workspaces/useWorkspaceDeleteHandler.ts` - Review for any blocking logic +3. `apps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/WorkspaceListItem/components/DeleteWorkspaceDialog/DeleteWorkspaceDialog.tsx` - Ensure immediate dialog close + +## Testing + +1. Delete a workspace with large node_modules - should disappear instantly +2. Delete the active workspace - should switch to another workspace instantly +3. Simulate a failure (e.g., by temporarily breaking the backend) - workspace should reappear with error toast +4. Delete while workspace is initializing - should still feel instant + +## Risks + +- **Low:** If deletion fails, workspace reappears (could be slightly confusing but correct) +- **Mitigation:** Show clear error toast explaining what happened + +## Alternative Considered + +Speeding up the actual deletion (async file removal, etc.) was considered but adds complexity and edge cases around orphaned files, race conditions with branch reuse, and teardown script failures. Optimistic UI is simpler and achieves the same perceived performance. From 8e30af56c93e60555852f8afd04a7e46c1a082ef Mon Sep 17 00:00:00 2001 From: AviPeltz Date: Tue, 6 Jan 2026 17:22:30 -0800 Subject: [PATCH 2/2] make this ish more robusticles --- .../react-query/workspaces/useCloseWorkspace.ts | 17 ++++++++++++++++- .../workspaces/useDeleteWorkspace.ts | 17 ++++++++++++++++- 2 files changed, 32 insertions(+), 2 deletions(-) diff --git a/apps/desktop/src/renderer/react-query/workspaces/useCloseWorkspace.ts b/apps/desktop/src/renderer/react-query/workspaces/useCloseWorkspace.ts index b7ca5ed574d..c67674c4324 100644 --- a/apps/desktop/src/renderer/react-query/workspaces/useCloseWorkspace.ts +++ b/apps/desktop/src/renderer/react-query/workspaces/useCloseWorkspace.ts @@ -83,6 +83,21 @@ export function useCloseWorkspace( ); if (projectGroup && workspaceFromGrouped) { + // For worktree-type workspaces, provide minimal worktree data to prevent + // hasIncompleteInit from triggering the initialization view + const worktreeData = + workspaceFromGrouped.type === "worktree" + ? { + branch: nextWorkspace.branch, + baseBranch: null, + gitStatus: { + branch: nextWorkspace.branch, + needsRebase: false, + lastRefreshed: Date.now(), + }, + } + : null; + utils.workspaces.getActive.setData(undefined, { ...nextWorkspace, type: workspaceFromGrouped.type, @@ -92,7 +107,7 @@ export function useCloseWorkspace( name: projectGroup.project.name, mainRepoPath: projectGroup.project.mainRepoPath, }, - worktree: null, // Will be populated on invalidate + worktree: worktreeData, }); } else { // Fallback: just clear it and let invalidate handle it diff --git a/apps/desktop/src/renderer/react-query/workspaces/useDeleteWorkspace.ts b/apps/desktop/src/renderer/react-query/workspaces/useDeleteWorkspace.ts index a201be93eea..331be9b895e 100644 --- a/apps/desktop/src/renderer/react-query/workspaces/useDeleteWorkspace.ts +++ b/apps/desktop/src/renderer/react-query/workspaces/useDeleteWorkspace.ts @@ -83,6 +83,21 @@ export function useDeleteWorkspace( ); if (projectGroup && workspaceFromGrouped) { + // For worktree-type workspaces, provide minimal worktree data to prevent + // hasIncompleteInit from triggering the initialization view + const worktreeData = + workspaceFromGrouped.type === "worktree" + ? { + branch: nextWorkspace.branch, + baseBranch: null, + gitStatus: { + branch: nextWorkspace.branch, + needsRebase: false, + lastRefreshed: Date.now(), + }, + } + : null; + utils.workspaces.getActive.setData(undefined, { ...nextWorkspace, type: workspaceFromGrouped.type, @@ -92,7 +107,7 @@ export function useDeleteWorkspace( name: projectGroup.project.name, mainRepoPath: projectGroup.project.mainRepoPath, }, - worktree: null, // Will be populated on invalidate + worktree: worktreeData, }); } else { // Fallback: just clear it and let invalidate handle it