From 8fef6db29266f57a450fafaa9d2d26554d099113 Mon Sep 17 00:00:00 2001
From: Brennan Benson <79079362+brennanb2025@users.noreply.github.com>
Date: Thu, 30 Jul 2026 20:32:40 -0700
Subject: [PATCH] Fix duplicate remote worktree delete failures (#11623)
* Fix duplicate worktree delete attempts
* Complete delete flow selector mock
* Normalize duplicate delete selections
---
.../sidebar/DeleteWorktreeDialog.test.tsx | 4 +++
.../sidebar/DeleteWorktreeDialog.tsx | 9 ++++---
.../sidebar/delete-worktree-flow.test.ts | 11 ++++++++
.../sidebar/delete-worktree-flow.ts | 25 +++++++++++--------
.../delete-worktree-parallel-flow.test.ts | 20 +++++++++++++++
5 files changed, 55 insertions(+), 14 deletions(-)
diff --git a/src/renderer/src/components/sidebar/DeleteWorktreeDialog.test.tsx b/src/renderer/src/components/sidebar/DeleteWorktreeDialog.test.tsx
index 7f1576c72..3106deda4 100644
--- a/src/renderer/src/components/sidebar/DeleteWorktreeDialog.test.tsx
+++ b/src/renderer/src/components/sidebar/DeleteWorktreeDialog.test.tsx
@@ -42,6 +42,10 @@ vi.mock('@/store', () => ({
)
}))
+vi.mock('@/store/selectors', () => ({
+ useAllWorktrees: () => mocks.state.allWorktrees()
+}))
+
vi.mock('@/components/ui/dialog', () => ({
Dialog: ({ open, children }: { open: boolean; children: ReactNode }) =>
open ? {children}
: null,
diff --git a/src/renderer/src/components/sidebar/DeleteWorktreeDialog.tsx b/src/renderer/src/components/sidebar/DeleteWorktreeDialog.tsx
index 598f5bedd..b89f3fef3 100644
--- a/src/renderer/src/components/sidebar/DeleteWorktreeDialog.tsx
+++ b/src/renderer/src/components/sidebar/DeleteWorktreeDialog.tsx
@@ -7,6 +7,7 @@ import {
DialogTitle
} from '@/components/ui/dialog'
import { useAppStore } from '@/store'
+import { useAllWorktrees } from '@/store/selectors'
import { toast } from 'sonner'
import { getConnectionId } from '@/lib/connection-context'
import { getRuntimeGitStatus } from '@/runtime/runtime-git-client'
@@ -36,7 +37,7 @@ const DeleteWorktreeDialog = React.memo(function DeleteWorktreeDialog() {
const closeModal = useAppStore((s) => s.closeModal)
const removeWorktree = useAppStore((s) => s.removeWorktree)
const clearWorktreeDeleteState = useAppStore((s) => s.clearWorktreeDeleteState)
- const allWorktrees = useAppStore((s) => s.allWorktrees)
+ const allWorktrees = useAllWorktrees()
const repos = useAppStore((s) => s.repos)
const worktreeLineageById = useAppStore((s) => s.worktreeLineageById)
const updateSettings = useAppStore((s) => s.updateSettings)
@@ -62,7 +63,7 @@ const DeleteWorktreeDialog = React.memo(function DeleteWorktreeDialog() {
? (modalData.onDeleted as (worktreeIds: string[]) => void)
: null
const worktree = useMemo(
- () => (worktreeId ? (allWorktrees().find((item) => item.id === worktreeId) ?? null) : null),
+ () => (worktreeId ? (allWorktrees.find((item) => item.id === worktreeId) ?? null) : null),
[allWorktrees, worktreeId]
)
const worktrees = useMemo(() => {
@@ -70,7 +71,7 @@ const DeleteWorktreeDialog = React.memo(function DeleteWorktreeDialog() {
return []
}
const selected = new Set(worktreeIds)
- return allWorktrees().filter((item) => selected.has(item.id))
+ return allWorktrees.filter((item) => selected.has(item.id))
}, [allWorktrees, worktreeIds])
const repoMap = useMemo(() => new Map(repos.map((repo) => [repo.id, repo])), [repos])
const isBatchDelete = worktreeIds.length > 1
@@ -90,7 +91,7 @@ const DeleteWorktreeDialog = React.memo(function DeleteWorktreeDialog() {
const lineageDelete = useMemo(
() =>
!isBatchDelete && worktree
- ? getWorkspaceDeleteLineage(worktree, allWorktrees(), worktreeLineageById)
+ ? getWorkspaceDeleteLineage(worktree, allWorktrees, worktreeLineageById)
: { descendants: [], deleteAllTargets: [] },
[allWorktrees, isBatchDelete, worktree, worktreeLineageById]
)
diff --git a/src/renderer/src/components/sidebar/delete-worktree-flow.test.ts b/src/renderer/src/components/sidebar/delete-worktree-flow.test.ts
index 69771ef31..a188f16c8 100644
--- a/src/renderer/src/components/sidebar/delete-worktree-flow.test.ts
+++ b/src/renderer/src/components/sidebar/delete-worktree-flow.test.ts
@@ -53,6 +53,7 @@ vi.mock('@/store', () => ({
}))
vi.mock('@/store/selectors', () => ({
+ getAllWorktreesFromState: () => Array.from(mocks.state.worktreeMap.values()),
getWorktreeMapFromState: () => mocks.state.worktreeMap
}))
@@ -141,6 +142,16 @@ describe('runWorktreeBatchDelete', () => {
expect(mocks.state.openModal).toHaveBeenCalledWith('delete-worktree', { worktreeId: 'wt-1' })
})
+ it('treats duplicate selected ids as one delete target', () => {
+ setWorktrees([{ id: 'wt-1' }])
+
+ const started = runWorktreeBatchDelete(['wt-1', 'wt-1'])
+
+ expect(started).toBe(true)
+ expect(mocks.state.clearWorktreeDeleteState).toHaveBeenCalledTimes(1)
+ expect(mocks.state.openModal).toHaveBeenCalledWith('delete-worktree', { worktreeId: 'wt-1' })
+ })
+
it('keeps batch deletes behind confirmation when confirmation is skipped', () => {
mocks.state.settings = { skipDeleteWorktreeConfirm: true }
setWorktrees([
diff --git a/src/renderer/src/components/sidebar/delete-worktree-flow.ts b/src/renderer/src/components/sidebar/delete-worktree-flow.ts
index f5a653a42..b2dc1dbc1 100644
--- a/src/renderer/src/components/sidebar/delete-worktree-flow.ts
+++ b/src/renderer/src/components/sidebar/delete-worktree-flow.ts
@@ -1,6 +1,6 @@
import { toast } from 'sonner'
import { useAppStore } from '@/store'
-import { getWorktreeMapFromState } from '@/store/selectors'
+import { getAllWorktreesFromState, getWorktreeMapFromState } from '@/store/selectors'
import { findRepoForHost } from '@/store/slices/repo-host-identity'
import { activateAndRevealWorktree } from '@/lib/worktree-activation'
import { prepareActiveWorktreeFocusAfterDelete } from './active-worktree-focus-after-delete'
@@ -46,16 +46,18 @@ export async function runWorktreeDeletesInParallel(
targets: readonly Pick[],
options: WorktreeDeleteWithToastOptions = {}
): Promise {
+ // Why: refresh races can leave duplicate rows, but a destructive command must run once per identity.
+ const uniqueTargets = Array.from(new Map(targets.map((target) => [target.id, target])).values())
// Why: capture the viewed workspace before any delete so we can focus one survivor after the batch settles, not per delete.
const activeWorktreeIdBefore = useAppStore.getState().activeWorktreeId
const commitBatchFocus = activeWorktreeIdBefore
? prepareActiveWorktreeFocusAfterDelete(activeWorktreeIdBefore)
: null
// Why: mark every target deleting up front for immediate in-flight feedback, even though deletes serialize per repo.
- useAppStore.getState().markWorktreesDeleting(targets.map((target) => target.id))
+ useAppStore.getState().markWorktreesDeleting(uniqueTargets.map((target) => target.id))
// Why: worktree remove/prune/branch -D race on shared ref locks; group by repoId to serialize per repo (cross-repo stays parallel).
- const groups = new Map()
- for (const target of targets) {
+ const groups = new Map()
+ for (const target of uniqueTargets) {
const group = groups.get(target.repoId)
if (group) {
group.push(target)
@@ -95,7 +97,7 @@ export async function runWorktreeDeletesInParallel(
if (activeWorktreeIdBefore && deletedSet.has(activeWorktreeIdBefore)) {
commitBatchFocus?.()
}
- return targets.filter((target) => deletedSet.has(target.id)).map((target) => target.id)
+ return uniqueTargets.filter((target) => deletedSet.has(target.id)).map((target) => target.id)
}
/**
@@ -249,8 +251,8 @@ export function runWorktreeDelete(worktreeId: string): void {
}
const hasLineageChildren =
- getWorkspaceDeleteLineage(target, state.allWorktrees(), state.worktreeLineageById).descendants
- .length > 0
+ getWorkspaceDeleteLineage(target, getAllWorktreesFromState(state), state.worktreeLineageById)
+ .descendants.length > 0
const skipConfirm = state.settings?.skipDeleteWorktreeConfirm ?? false
if (skipConfirm && !hasLineageChildren) {
void runWorktreeDeleteWithToast(worktreeId, target.displayName)
@@ -268,7 +270,7 @@ export function runWorktreeBatchDelete(
): boolean {
const state = useAppStore.getState()
const worktreeMap = getWorktreeMapFromState(state)
- const targets = worktreeIds
+ const targets = Array.from(new Set(worktreeIds))
.map((id) => worktreeMap.get(id) ?? null)
.filter((worktree): worktree is Worktree => worktree != null && !worktree.isMainWorktree)
@@ -295,8 +297,11 @@ export function runWorktreeBatchDelete(
// Why: bulk cleanup can destroy many directories at once, so batch/Space deletes keep an explicit confirmation step.
const singleTargetHasLineageChildren =
targets.length === 1 &&
- getWorkspaceDeleteLineage(targets[0], state.allWorktrees(), state.worktreeLineageById)
- .descendants.length > 0
+ getWorkspaceDeleteLineage(
+ targets[0],
+ getAllWorktreesFromState(state),
+ state.worktreeLineageById
+ ).descendants.length > 0
const skipConfirm =
!options.forceConfirm &&
targets.length === 1 &&
diff --git a/src/renderer/src/components/sidebar/delete-worktree-parallel-flow.test.ts b/src/renderer/src/components/sidebar/delete-worktree-parallel-flow.test.ts
index e20214d66..817a02f20 100644
--- a/src/renderer/src/components/sidebar/delete-worktree-parallel-flow.test.ts
+++ b/src/renderer/src/components/sidebar/delete-worktree-parallel-flow.test.ts
@@ -31,6 +31,7 @@ vi.mock('@/store', () => ({
}))
vi.mock('@/store/selectors', () => ({
+ getAllWorktreesFromState: () => Array.from(mocks.state.worktreeMap.values()),
getWorktreeMapFromState: () => mocks.state.worktreeMap
}))
@@ -145,6 +146,25 @@ describe('runWorktreeDeletesInParallel', () => {
expect(mocks.state.removeWorktree).toHaveBeenNthCalledWith(2, 'wt-2', true)
})
+ it('deletes a duplicated target identity only once', async () => {
+ const target = {
+ id: 'wt-1',
+ displayName: 'one',
+ repoId: 'repo-a',
+ path: '/workspaces/one'
+ }
+ mocks.state.removeWorktree
+ .mockResolvedValueOnce({ ok: true })
+ .mockResolvedValueOnce({ ok: false, error: 'selector_not_found' })
+
+ await expect(runWorktreeDeletesInParallel([target, target])).resolves.toEqual(['wt-1'])
+
+ expect(mocks.state.markWorktreesDeleting).toHaveBeenCalledWith(['wt-1'])
+ expect(mocks.state.removeWorktree).toHaveBeenCalledTimes(1)
+ expect(mocks.state.removeWorktree).toHaveBeenCalledWith('wt-1', false)
+ expect(toast.error).not.toHaveBeenCalled()
+ })
+
it('clears a pending ancestor when a nested descendant delete fails', async () => {
mocks.state.removeWorktree.mockImplementationOnce(async (worktreeId: string) => {
mocks.state.deleteStateByWorktreeId[worktreeId] = {