diff --git a/src/renderer/src/components/sidebar/DeleteWorktreeDialog.host-context-boundary.test.ts b/src/renderer/src/components/sidebar/DeleteWorktreeDialog.host-context-boundary.test.ts index 30066815370..27a5f100654 100644 --- a/src/renderer/src/components/sidebar/DeleteWorktreeDialog.host-context-boundary.test.ts +++ b/src/renderer/src/components/sidebar/DeleteWorktreeDialog.host-context-boundary.test.ts @@ -2,7 +2,7 @@ import { readFileSync } from 'node:fs' import { join } from 'node:path' import { describe, expect, it } from 'vitest' -const SOURCE = readFileSync(join(__dirname, 'DeleteWorktreeDialog.tsx'), 'utf8') +const SOURCE = readFileSync(join(__dirname, 'use-delete-worktree-status-hydration.ts'), 'utf8') function sourceBetween(source: string, startPattern: string, endPattern: string): string { const start = source.indexOf(startPattern) @@ -14,15 +14,21 @@ function sourceBetween(source: string, startPattern: string, endPattern: string) describe('DeleteWorktreeDialog host-context boundaries', () => { it('preloads git status from the selected worktree owner instead of the focused host', () => { - const effect = sourceBetween( - SOURCE, - 'const statusTargets = deleteTargets.filter(', - 'return () => {' - ) + const effect = sourceBetween(SOURCE, 'const targets = deleteTargets.filter(', 'return () => {') expect(effect).toContain('getSettingsForWorktreeRuntimeOwner') expect(effect).toContain('worktreesByRepo: useAppStore.getState().worktreesByRepo') - expect(effect).toContain('item.id') - expect(effect).not.toContain('settings,\n worktreeId: item.id') + expect(effect).toContain('target.id') + expect(effect).not.toContain('settings,\n worktreeId: target.id') + }) + + it('does not restart pending status requests when one target hydrates', () => { + const effect = sourceBetween(SOURCE, 'useEffect(() => {', '}, [') + + expect(SOURCE).not.toContain('useAppStore((state) => state.gitStatusByWorktree)') + expect(effect).toContain('useAppStore.getState().gitStatusByWorktree') + expect(effect).toContain('const controller = new AbortController()') + expect(effect).toContain('{ signal: controller.signal }') + expect(effect).toContain('controller.abort()') }) }) diff --git a/src/renderer/src/components/sidebar/DeleteWorktreeDialog.test.tsx b/src/renderer/src/components/sidebar/DeleteWorktreeDialog.test.tsx index 3106deda4c7..65700ef574b 100644 --- a/src/renderer/src/components/sidebar/DeleteWorktreeDialog.test.tsx +++ b/src/renderer/src/components/sidebar/DeleteWorktreeDialog.test.tsx @@ -43,7 +43,9 @@ vi.mock('@/store', () => ({ })) vi.mock('@/store/selectors', () => ({ - useAllWorktrees: () => mocks.state.allWorktrees() + useAllWorktrees: () => mocks.state.allWorktrees(), + getWorktreeMapFromState: () => + new Map(mocks.state.allWorktrees().map((worktree) => [worktree.id, worktree])) })) vi.mock('@/components/ui/dialog', () => ({ @@ -91,7 +93,12 @@ vi.mock('./active-worktree-focus-after-delete', () => ({ prepareActiveWorktreeFocusAfterDelete: () => vi.fn() })) +vi.mock('./stale-workspace-list-toast', () => ({ + showWorkspaceListChangedToast: vi.fn() +})) + import { runWorktreeDeletesInParallel } from './delete-worktree-flow' +import { showWorkspaceListChangedToast } from './stale-workspace-list-toast' function makeWorktree(id: string, path: string): Worktree { return { @@ -153,7 +160,11 @@ describe('DeleteWorktreeDialog lineage copy', () => { it('shows child-delete copy and only a delete-all action when the workspace has children', async () => { const parent = makeWorktree('Parent workspace', '/workspaces/parent') const child = makeWorktree('Child workspace', '/workspaces/child') - mocks.state.modalData = { worktreeId: parent.id } + mocks.state.modalData = { + worktreeId: parent.id, + worktreeDeleteIdentities: [{ id: parent.id, instanceId: parent.instanceId }], + lineageDeleteIdentities: [child, parent].map(({ id, instanceId }) => ({ id, instanceId })) + } mocks.state.allWorktrees.mockReturnValue([parent, child]) mocks.state.worktreeLineageById = { [child.id]: makeLineage(child, parent) @@ -295,7 +306,11 @@ describe('DeleteWorktreeDialog lineage copy', () => { it('notifies the dialog caller after a toast force delete succeeds', async () => { const workspace = makeWorktree('Workspace', '/workspaces/workspace') const onDeleted = vi.fn() - mocks.state.modalData = { worktreeId: workspace.id, onDeleted } + mocks.state.modalData = { + worktreeId: workspace.id, + worktreeDeleteIdentities: [{ id: workspace.id, instanceId: workspace.instanceId }], + onDeleted + } mocks.state.allWorktrees.mockReturnValue([workspace]) const { default: DeleteWorktreeDialog } = await import('./DeleteWorktreeDialog') @@ -318,4 +333,57 @@ describe('DeleteWorktreeDialog lineage copy', () => { expect(onDeleted).toHaveBeenCalledWith([workspace.id]) }) + + it('rejects confirmation when the workspace instance changed after the dialog opened', async () => { + const original = makeWorktree('Workspace', '/workspaces/original') + const replacement = { ...original, instanceId: 'replacement-instance' } + mocks.state.modalData = { + worktreeId: original.id, + worktreeDeleteIdentities: [{ id: original.id, instanceId: original.instanceId }] + } + mocks.state.allWorktrees.mockReturnValue([original]) + + const { default: DeleteWorktreeDialog } = await import('./DeleteWorktreeDialog') + renderToStaticMarkup() + mocks.state.allWorktrees.mockReturnValue([replacement]) + + const deleteButton = mocks.buttonProps.find((props) => props.variant === 'destructive') as + | { onClick?: (event: never) => void } + | undefined + deleteButton?.onClick?.(undefined as never) + + expect(showWorkspaceListChangedToast).toHaveBeenCalledOnce() + expect(mocks.state.closeModal).toHaveBeenCalledOnce() + expect(runWorktreeDeletesInParallel).not.toHaveBeenCalled() + expect(mocks.state.removeWorktree).not.toHaveBeenCalled() + }) + + it('rejects lineage confirmation when a descendant instance changed', async () => { + const parent = makeWorktree('Parent workspace', '/workspaces/parent') + const child = makeWorktree('Child workspace', '/workspaces/child') + const replacement = { ...child, instanceId: 'replacement-instance' } + mocks.state.modalData = { + worktreeId: parent.id, + worktreeDeleteIdentities: [{ id: parent.id, instanceId: parent.instanceId }], + lineageDeleteIdentities: [child, parent].map(({ id, instanceId }) => ({ id, instanceId })) + } + mocks.state.allWorktrees.mockReturnValue([parent, child]) + mocks.state.worktreeLineageById = { + [child.id]: makeLineage(child, parent) + } + + const { default: DeleteWorktreeDialog } = await import('./DeleteWorktreeDialog') + renderToStaticMarkup() + mocks.state.allWorktrees.mockReturnValue([parent, replacement]) + + const deleteButton = mocks.buttonProps.find((props) => props.variant === 'destructive') as + | { onClick?: () => void } + | undefined + deleteButton?.onClick?.() + + expect(showWorkspaceListChangedToast).toHaveBeenCalledOnce() + expect(mocks.state.closeModal).toHaveBeenCalledOnce() + expect(runWorktreeDeletesInParallel).not.toHaveBeenCalled() + expect(mocks.state.removeWorktree).not.toHaveBeenCalled() + }) }) diff --git a/src/renderer/src/components/sidebar/DeleteWorktreeDialog.tsx b/src/renderer/src/components/sidebar/DeleteWorktreeDialog.tsx index 6bbe0581c9a..568faf329b4 100644 --- a/src/renderer/src/components/sidebar/DeleteWorktreeDialog.tsx +++ b/src/renderer/src/components/sidebar/DeleteWorktreeDialog.tsx @@ -9,9 +9,6 @@ import { 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' -import { getSettingsForWorktreeRuntimeOwner } from '@/lib/worktree-runtime-owner' import { runWorktreeDeletesInParallel } from './delete-worktree-flow' import { prepareActiveWorktreeFocusAfterDelete } from './active-worktree-focus-after-delete' import { getWorkspaceDeleteLineage } from './workspace-delete-lineage' @@ -30,6 +27,8 @@ import { isFolderWorkspaceDelete as getIsFolderWorkspaceDelete } from './delete-worktree-dialog-copy' import { translate } from '@/i18n/i18n' +import { useDeleteWorktreeStatusHydration } from './use-delete-worktree-status-hydration' +import { useConfirmedWorktreeDeleteTargets } from './use-confirmed-worktree-delete-targets' const DeleteWorktreeDialog = React.memo(function DeleteWorktreeDialog() { const activeModal = useAppStore((s) => s.activeModal) @@ -43,9 +42,7 @@ const DeleteWorktreeDialog = React.memo(function DeleteWorktreeDialog() { const updateSettings = useAppStore((s) => s.updateSettings) const openSettingsTarget = useAppStore((s) => s.openSettingsTarget) const openSettingsPage = useAppStore((s) => s.openSettingsPage) - const settings = useAppStore((s) => s.settings) const gitStatusByWorktree = useAppStore((s) => s.gitStatusByWorktree) - const setGitStatus = useAppStore((s) => s.setGitStatus) const isOpen = activeModal === 'delete-worktree' const worktreeId = typeof modalData.worktreeId === 'string' ? modalData.worktreeId : '' @@ -58,6 +55,12 @@ const DeleteWorktreeDialog = React.memo(function DeleteWorktreeDialog() { : [], [modalData.worktreeIds, worktreeId] ) + const { worktreeDeleteIdentities, lineageDeleteIdentities, resolveConfirmedTargets } = + useConfirmedWorktreeDeleteTargets({ + worktreeIdentityData: modalData.worktreeDeleteIdentities, + lineageIdentityData: modalData.lineageDeleteIdentities, + closeModal + }) const onDeleted = typeof modalData.onDeleted === 'function' ? (modalData.onDeleted as (worktreeIds: string[]) => void) @@ -146,6 +149,7 @@ const DeleteWorktreeDialog = React.memo(function DeleteWorktreeDialog() { repoMap }) }, [deleteStateByWorktreeId, deleteTargets, gitStatusByWorktree, repoMap]) + useDeleteWorktreeStatusHydration({ isOpen, deleteTargets, repoMap }) if (!isOpen && dontAskAgain) { // Why: this checkbox is a one-shot dialog intent; reset it as soon as the @@ -170,47 +174,6 @@ const DeleteWorktreeDialog = React.memo(function DeleteWorktreeDialog() { worktrees.length ]) - useEffect(() => { - if (!isOpen) { - return - } - const statusTargets = deleteTargets.filter( - (item) => - !item.isMainWorktree && - !getIsFolderWorkspaceDelete(repoMap, item) && - gitStatusByWorktree[item.id] === undefined - ) - if (statusTargets.length === 0) { - return - } - let cancelled = false - for (const item of statusTargets) { - void getRuntimeGitStatus({ - // Why: delete warnings inspect git state for the selected workspace; - // a later focused-host switch must not make this preload query another host. - settings: getSettingsForWorktreeRuntimeOwner( - { repos, settings, worktreesByRepo: useAppStore.getState().worktreesByRepo }, - item.id - ), - worktreeId: item.id, - worktreePath: item.path, - connectionId: getConnectionId(item.id) ?? undefined - }) - .then((status) => { - if (!cancelled) { - setGitStatus(item.id, status) - } - }) - .catch(() => { - // Best-effort only: delete itself still performs the authoritative - // backend check and will surface failures through the normal toast. - }) - } - return () => { - cancelled = true - } - }, [deleteTargets, gitStatusByWorktree, isOpen, repoMap, repos, setGitStatus, settings]) - const handleOpenChange = useCallback( (open: boolean) => { if (open) { @@ -254,6 +217,10 @@ const DeleteWorktreeDialog = React.memo(function DeleteWorktreeDialog() { if (worktreeIds.length === 0) { return } + const currentWorktrees = resolveConfirmedTargets(worktreeDeleteIdentities, worktreeIds.length) + if (!currentWorktrees) { + return + } // Why: force-delete is a recovery path taken after a failed first delete. // Saving "don't ask again" from that state would conflate the recovery // action with a broader preference. Only persist the preference on the @@ -303,7 +270,7 @@ const DeleteWorktreeDialog = React.memo(function DeleteWorktreeDialog() { // Why: this modal is the destructive confirmation for the workspace // folder. Running a non-force remove here just turns dirty files into // a redundant Force Delete toast after the user already confirmed. - const deletePromise = runWorktreeDeletesInParallel(worktrees, { + const deletePromise = runWorktreeDeletesInParallel(currentWorktrees, { force: true, onForceDeleted: handleForceDeletedFromToast }) @@ -326,8 +293,9 @@ const DeleteWorktreeDialog = React.memo(function DeleteWorktreeDialog() { persistDontAskAgainPreference, removeWorktree, worktreeIds.length, + worktreeDeleteIdentities, worktreeId, - worktrees + resolveConfirmedTargets ] ) @@ -335,9 +303,16 @@ const DeleteWorktreeDialog = React.memo(function DeleteWorktreeDialog() { if (lineageDelete.deleteAllTargets.length <= 1) { return } + const currentTargets = resolveConfirmedTargets( + lineageDeleteIdentities, + lineageDelete.deleteAllTargets.length + ) + if (!currentTargets) { + return + } // Why: the lineage modal confirms every affected workspace up front, so // dirty child workspaces should not create per-workspace force prompts. - const deletePromise = runWorktreeDeletesInParallel(lineageDelete.deleteAllTargets, { + const deletePromise = runWorktreeDeletesInParallel(currentTargets, { force: true, onForceDeleted: handleForceDeletedFromToast }) @@ -349,7 +324,14 @@ const DeleteWorktreeDialog = React.memo(function DeleteWorktreeDialog() { onDeleted?.(deletedIds) } }) - }, [closeModal, handleForceDeletedFromToast, lineageDelete.deleteAllTargets, onDeleted]) + }, [ + closeModal, + handleForceDeletedFromToast, + lineageDelete.deleteAllTargets.length, + lineageDeleteIdentities, + onDeleted, + resolveConfirmedTargets + ]) return ( diff --git a/src/renderer/src/components/sidebar/WorktreeContextMenu.tsx b/src/renderer/src/components/sidebar/WorktreeContextMenu.tsx index 1063c5145e5..be61dd64d3d 100644 --- a/src/renderer/src/components/sidebar/WorktreeContextMenu.tsx +++ b/src/renderer/src/components/sidebar/WorktreeContextMenu.tsx @@ -40,7 +40,10 @@ import type { WorkspaceStatus, WorkspaceStatusDefinition } from '../../../../shared/types' -import { runWorktreeBatchDelete, runWorktreeDelete } from './delete-worktree-flow' +import { + deferWorktreeContextMenuDeleteIntent, + type WorktreeContextMenuDeleteIntent +} from './worktree-context-menu-delete-intent' import { runSleepWorktrees } from './sleep-worktree-flow' import { activateAndRevealWorktree } from '@/lib/worktree-activation' import { VIRTUALIZED_SCROLL_ANCHOR_RECORD_EVENT } from '@/hooks/useVirtualizedScrollAnchor' @@ -62,11 +65,7 @@ import { import { WorkspaceSleepMenuItems } from './WorkspaceSleepMenuItems' import { isEventTargetInsideCurrentTarget } from './worktree-card-dom-events' import { translate } from '@/i18n/i18n' -import { - folderWorkspaceKey, - parseWorkspaceKey, - worktreeWorkspaceKey -} from '../../../../shared/workspace-scope' +import { parseWorkspaceKey, worktreeWorkspaceKey } from '../../../../shared/workspace-scope' type Props = { worktree: Worktree @@ -332,8 +331,6 @@ const WorktreeContextMenu = React.memo(function WorktreeContextMenu({ const projectGroups = useAppStore((s) => s.projectGroups) const createProjectGroup = useAppStore((s) => s.createProjectGroup) const moveProjectToGroup = useAppStore((s) => s.moveProjectToGroup) - const deleteFolderWorkspace = useAppStore((s) => s.deleteFolderWorkspace) - const setActiveWorktree = useAppStore((s) => s.setActiveWorktree) const repo = useRepoById(worktree.repoId) const deleteState = useAppStore((s) => s.deleteStateByWorktreeId[worktree.id]) const [menuOpen, setMenuOpen] = useState(false) @@ -674,47 +671,30 @@ const WorktreeContextMenu = React.memo(function WorktreeContextMenu({ }, [sleepWorktreesAfterMenuClose, subtreeSleepableWorktrees]) const handleDelete = useCallback(() => { - // Folder mode handled inline because it routes to a different modal; - // standard delete delegates to the shared runWorktreeDelete helper. const restoreSidebarPosition = preserveDeleteSiblingPosition(scopeRef.current) scopeRef.current ?.closest('[data-worktree-sidebar]') ?.dispatchEvent(new Event(VIRTUALIZED_SCROLL_ANCHOR_RECORD_EVENT)) - setMenuOpenState(false) - // Why: Delete can remove the active row and remount the sidebar. Run it - // after menu close for the same reason as Sleep above. - window.setTimeout(() => { - if (isMultiContext) { - runWorktreeBatchDelete(batchDeleteWorktrees.map((item) => item.id)) - restoreSidebarPosition() - return - } - if (folderWorkspaceId) { - void deleteFolderWorkspace(folderWorkspaceId).then((deleted) => { - if ( - deleted && - useAppStore.getState().activeWorktreeId === folderWorkspaceKey(folderWorkspaceId) - ) { - setActiveWorktree(null) + const intent: WorktreeContextMenuDeleteIntent = isMultiContext + ? { + kind: 'batch', + worktrees: batchDeleteWorktrees.map(({ id, instanceId }) => ({ id, instanceId })) + } + : folderWorkspaceId + ? { kind: 'folder', folderWorkspaceId } + : { + kind: 'worktree', + worktree: { id: worktree.id, instanceId: worktree.instanceId } } - }) - restoreSidebarPosition() - return - } - // Why delegate to runWorktreeDelete: keeps the delete-vs-project-removal - // decision tree (and its rationale) in one place shared with command - // surfaces and the memory popover's inline Delete action. - runWorktreeDelete(worktree.id) - restoreSidebarPosition() - }, 50) + deferWorktreeContextMenuDeleteIntent(intent, restoreSidebarPosition) + setMenuOpenState(false) }, [ batchDeleteWorktrees, - deleteFolderWorkspace, folderWorkspaceId, isMultiContext, - setActiveWorktree, setMenuOpenState, - worktree.id + worktree.id, + worktree.instanceId ]) const handleOpenParent = useCallback(() => { 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 941c4d38eaa..9436f67bfcf 100644 --- a/src/renderer/src/components/sidebar/delete-worktree-flow.test.ts +++ b/src/renderer/src/components/sidebar/delete-worktree-flow.test.ts @@ -74,7 +74,11 @@ vi.mock('./delete-worktree-failure-toast', () => ({ import { toast } from 'sonner' import { showDeleteWorktreeFailureToast } from './delete-worktree-failure-toast' -import { runWorktreeBatchDelete, runWorktreeDelete } from './delete-worktree-flow' +import { + runWorktreeBatchDelete, + runWorktreeDelete, + runWorktreeDeletesInParallel +} from './delete-worktree-flow' function setWorktrees( worktrees: { @@ -101,7 +105,7 @@ function setWorktrees( ) } -describe('runWorktreeBatchDelete', () => { +describe('delete worktree flow', () => { beforeEach(() => { mocks.state.settings = { skipDeleteWorktreeConfirm: false } mocks.state.clearWorktreeDeleteState.mockClear() @@ -129,6 +133,10 @@ describe('runWorktreeBatchDelete', () => { expect(mocks.state.clearWorktreeDeleteState).not.toHaveBeenCalledWith('main') expect(mocks.state.openModal).toHaveBeenCalledWith('delete-worktree', { worktreeIds: ['wt-1', 'wt-2'], + worktreeDeleteIdentities: [ + { id: 'wt-1', instanceId: 'wt-1-instance' }, + { id: 'wt-2', instanceId: 'wt-2-instance' } + ], allowSkipConfirm: false }) }) @@ -139,7 +147,10 @@ describe('runWorktreeBatchDelete', () => { const started = runWorktreeBatchDelete(['main', 'wt-1']) expect(started).toBe(true) - expect(mocks.state.openModal).toHaveBeenCalledWith('delete-worktree', { worktreeId: 'wt-1' }) + expect(mocks.state.openModal).toHaveBeenCalledWith('delete-worktree', { + worktreeId: 'wt-1', + worktreeDeleteIdentities: [{ id: 'wt-1', instanceId: 'wt-1-instance' }] + }) }) it('treats duplicate selected ids as one delete target', () => { @@ -149,7 +160,85 @@ describe('runWorktreeBatchDelete', () => { expect(started).toBe(true) expect(mocks.state.clearWorktreeDeleteState).toHaveBeenCalledTimes(1) - expect(mocks.state.openModal).toHaveBeenCalledWith('delete-worktree', { worktreeId: 'wt-1' }) + expect(mocks.state.openModal).toHaveBeenCalledWith('delete-worktree', { + worktreeId: 'wt-1', + worktreeDeleteIdentities: [{ id: 'wt-1', instanceId: 'wt-1-instance' }] + }) + }) + + it('rejects the whole batch when a selected path belongs to a different instance', () => { + setWorktrees([ + { id: 'wt-1', instanceId: 'instance-1' }, + { id: 'wt-2', instanceId: 'instance-2' } + ]) + + const started = runWorktreeBatchDelete([ + { id: 'wt-1', instanceId: 'instance-1' }, + { id: 'wt-2', instanceId: 'replaced-instance' } + ]) + + expect(started).toBe(false) + expect(mocks.state.clearWorktreeDeleteState).not.toHaveBeenCalled() + expect(mocks.state.openModal).not.toHaveBeenCalled() + expect(mocks.state.removeWorktree).not.toHaveBeenCalled() + expect(toast.info).toHaveBeenCalledWith( + 'Workspace list changed', + expect.objectContaining({ + description: 'Refresh Space and try again if the workspace list looks stale.' + }) + ) + }) + + it('opens batch confirmation when every selected instance is still current', () => { + setWorktrees([ + { id: 'wt-1', instanceId: 'instance-1' }, + { id: 'wt-2', instanceId: 'instance-2' } + ]) + + const started = runWorktreeBatchDelete([ + { id: 'wt-1', instanceId: 'instance-1' }, + { id: 'wt-2', instanceId: 'instance-2' } + ]) + + expect(started).toBe(true) + expect(toast.info).not.toHaveBeenCalled() + expect(mocks.state.openModal).toHaveBeenCalledWith('delete-worktree', { + worktreeIds: ['wt-1', 'wt-2'], + worktreeDeleteIdentities: [ + { id: 'wt-1', instanceId: 'instance-1' }, + { id: 'wt-2', instanceId: 'instance-2' } + ], + allowSkipConfirm: false + }) + }) + + it('revalidates each queued instance immediately before execution', async () => { + setWorktrees([ + { id: 'wt-1', instanceId: 'instance-1', path: '/workspaces/first-longer' }, + { id: 'wt-2', instanceId: 'instance-2', path: '/workspaces/second' } + ]) + const targets = Array.from(mocks.state.worktreeMap.values()) + let finishFirst!: (result: { ok: true }) => void + mocks.state.removeWorktree.mockImplementationOnce( + () => new Promise((resolve) => (finishFirst = resolve)) + ) + + const deletion = runWorktreeDeletesInParallel(targets) + await vi.waitFor(() => expect(mocks.state.removeWorktree).toHaveBeenCalledWith('wt-1', false)) + setWorktrees([ + { id: 'wt-1', instanceId: 'instance-1', path: '/workspaces/first-longer' }, + { id: 'wt-2', instanceId: 'replacement-instance', path: '/workspaces/second' } + ]) + finishFirst({ ok: true }) + + await expect(deletion).resolves.toEqual(['wt-1']) + expect(mocks.state.removeWorktree).not.toHaveBeenCalledWith('wt-2', false) + expect(toast.info).toHaveBeenCalledWith( + 'Workspace list changed', + expect.objectContaining({ + description: 'Refresh Space and try again if the workspace list looks stale.' + }) + ) }) it('keeps batch deletes behind confirmation when confirmation is skipped', () => { @@ -166,6 +255,10 @@ describe('runWorktreeBatchDelete', () => { expect(mocks.state.removeWorktree).not.toHaveBeenCalled() expect(mocks.state.openModal).toHaveBeenCalledWith('delete-worktree', { worktreeIds: ['wt-1', 'wt-2'], + worktreeDeleteIdentities: [ + { id: 'wt-1', instanceId: 'wt-1-instance' }, + { id: 'wt-2', instanceId: 'wt-2-instance' } + ], allowSkipConfirm: false, onDeleted }) @@ -271,6 +364,11 @@ describe('runWorktreeBatchDelete', () => { expect(mocks.state.removeWorktree).not.toHaveBeenCalled() expect(mocks.state.openModal).toHaveBeenCalledWith('delete-worktree', { worktreeId: 'parent', + worktreeDeleteIdentities: [{ id: 'parent', instanceId: 'parent-instance' }], + lineageDeleteIdentities: [ + { id: 'child', instanceId: 'child-instance' }, + { id: 'parent', instanceId: 'parent-instance' } + ], allowSkipConfirm: false }) }) @@ -297,10 +395,80 @@ describe('runWorktreeBatchDelete', () => { expect(mocks.state.removeWorktree).not.toHaveBeenCalled() expect(mocks.state.openModal).toHaveBeenCalledWith('delete-worktree', { worktreeId: 'parent', + worktreeDeleteIdentities: [{ id: 'parent', instanceId: 'parent-instance' }], + lineageDeleteIdentities: [ + { id: 'child', instanceId: 'child-instance' }, + { id: 'parent', instanceId: 'parent-instance' } + ], allowSkipConfirm: false }) }) + it('reports a stale list instead of silently dropping a delete whose row vanished', () => { + mocks.state.settings = { skipDeleteWorktreeConfirm: true } + setWorktrees([]) + + runWorktreeDelete('wt-1') + + expect(mocks.state.removeWorktree).not.toHaveBeenCalled() + expect(mocks.state.openModal).not.toHaveBeenCalled() + expect(mocks.state.clearWorktreeDeleteState).not.toHaveBeenCalled() + expect(toast.info).toHaveBeenCalledWith( + 'Workspace list changed', + expect.objectContaining({ + description: 'Refresh Space and try again if the workspace list looks stale.' + }) + ) + }) + + it('rejects a delayed delete when the path now belongs to a different instance', () => { + mocks.state.settings = { skipDeleteWorktreeConfirm: true } + setWorktrees([{ id: 'wt-1', instanceId: 'instance-2' }]) + + runWorktreeDelete('wt-1', { expectedInstanceId: 'instance-1' }) + + expect(mocks.state.removeWorktree).not.toHaveBeenCalled() + expect(mocks.state.openModal).not.toHaveBeenCalled() + expect(toast.info).toHaveBeenCalledWith( + 'Workspace list changed', + expect.objectContaining({ + description: 'Refresh Space and try again if the workspace list looks stale.' + }) + ) + }) + + it('runs a delayed delete when the captured instance is still current', () => { + mocks.state.settings = { skipDeleteWorktreeConfirm: true } + setWorktrees([{ id: 'wt-1', instanceId: 'instance-1', displayName: 'one' }]) + + runWorktreeDelete('wt-1', { expectedInstanceId: 'instance-1' }) + + expect(toast.info).not.toHaveBeenCalled() + expect(mocks.state.removeWorktree).toHaveBeenCalledWith('wt-1', false) + }) + + // Why: the delete-current-workspace shortcut (useIpcEvents) forwards whatever workspace is + // active, and a folder workspace is never in the worktree map — claiming it vanished would lie. + it('stays silent for a folder workspace, which this funnel does not route', () => { + mocks.state.settings = { skipDeleteWorktreeConfirm: true } + setWorktrees([]) + + runWorktreeDelete('folder:11111111-2222-3333-4444-555555555555') + + expect(mocks.state.removeWorktree).not.toHaveBeenCalled() + expect(toast.info).not.toHaveBeenCalled() + }) + + it('does not report a stale list when the workspace is still present', () => { + mocks.state.settings = { skipDeleteWorktreeConfirm: true } + setWorktrees([{ id: 'wt-1', displayName: 'one' }]) + + runWorktreeDelete('wt-1') + + expect(toast.info).not.toHaveBeenCalled() + expect(mocks.state.removeWorktree).toHaveBeenCalledWith('wt-1', false) + }) + it('opens project removal confirmation for a primary workspace', () => { mocks.state.settings = { skipDeleteWorktreeConfirm: true } setWorktrees([ @@ -334,6 +502,7 @@ describe('runWorktreeBatchDelete', () => { expect(mocks.state.removeWorktree).not.toHaveBeenCalled() expect(mocks.state.openModal).toHaveBeenCalledWith('delete-worktree', { worktreeId: 'wt-1', + worktreeDeleteIdentities: [{ id: 'wt-1', instanceId: 'wt-1-instance' }], allowSkipConfirm: false, onDeleted }) diff --git a/src/renderer/src/components/sidebar/delete-worktree-flow.ts b/src/renderer/src/components/sidebar/delete-worktree-flow.ts index 6259789187b..2daca622729 100644 --- a/src/renderer/src/components/sidebar/delete-worktree-flow.ts +++ b/src/renderer/src/components/sidebar/delete-worktree-flow.ts @@ -1,221 +1,46 @@ -import { toast } from 'sonner' import { useAppStore } from '@/store' 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' -import { showDeleteWorktreeFailureToast } from './delete-worktree-failure-toast' +import { + showNoDeletableWorkspacesToast, + showWorkspaceListChangedToast +} from './stale-workspace-list-toast' import { getWorkspaceDeleteLineage } from './workspace-delete-lineage' import { resolveSshWorkspaceForget } from './ssh-workspace-forget-resolution' import { isPairedWebClientWindow } from '@/lib/desktop-window-chrome' +import { parseWorkspaceKey } from '../../../../shared/workspace-scope' import { - isPathInsideOrEqual, - normalizeRuntimePathForComparison -} from '../../../../shared/cross-platform-path' -import type { Worktree } from '../../../../shared/types' -import { translate } from '@/i18n/i18n' + resolveWorktreeBatchDeleteTargets, + toWorktreeDeleteIdentities, + type WorktreeBatchDeleteOptions, + type WorktreeDeleteIdentity, + type WorktreeDeleteOptions +} from './worktree-delete-request' +import { + runWorktreeDeletesInParallel, + runWorktreeDeleteWithToast +} from './worktree-delete-execution' -type WorktreeBatchDeleteOptions = { - forceConfirm?: boolean - onDeleted?: (worktreeIds: string[]) => void -} - -type WorktreeDeleteWithToastOptions = { - force?: boolean - onForceDeleted?: (worktreeId: string) => void - // Why: batch deletes suppress the per-delete focus handoff to focus one survivor after the batch (see runWorktreeDeletesInParallel). - focusSuccessorOnDelete?: boolean -} - -// Why: a failed delete usually means unresolved changes, so land on the diff panel, not just focus the worktree. -function viewWorktreeDiff(worktreeId: string): void { - activateAndRevealWorktree(worktreeId) - const state = useAppStore.getState() - state.setRightSidebarTab('source-control') - state.setRightSidebarOpen(true) -} - -function isStrictDescendantPath(parentPath: string, childPath: string): boolean { - return ( - normalizeRuntimePathForComparison(parentPath) !== - normalizeRuntimePathForComparison(childPath) && isPathInsideOrEqual(parentPath, childPath) - ) -} - -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(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 uniqueTargets) { - const group = groups.get(target.repoId) - if (group) { - group.push(target) - } else { - groups.set(target.repoId, [target]) - } - } - for (const group of groups.values()) { - // Why: delete nested children first — else the parent delete is rejected while it still contains a registered worktree. - group.sort((a, b) => b.path.length - a.path.length) - } - const groupResults = await Promise.all( - Array.from(groups.values()).map(async (group) => { - const deletedInGroup: string[] = [] - const failedInGroup: (typeof group)[number][] = [] - for (const target of group) { - if (failedInGroup.some((failed) => isStrictDescendantPath(target.path, failed.path))) { - useAppStore.getState().clearWorktreeDeleteState(target.id) - continue - } - const deleted = await runWorktreeDeleteWithToast(target.id, target.displayName, { - ...options, - focusSuccessorOnDelete: false - }) - if (deleted) { - deletedInGroup.push(target.id) - } else { - // Why: after a descendant delete fails, deleting an ancestor can still remove that child from disk (it lives under the parent). - failedInGroup.push(target) - } - } - return deletedInGroup - }) - ) - const deletedSet = new Set(groupResults.flat()) - // Why: focus a survivor once after the batch settles — an intermediate focus could spawn a terminal in a to-be-deleted workspace. - if (activeWorktreeIdBefore && deletedSet.has(activeWorktreeIdBefore)) { - commitBatchFocus?.() - } - return uniqueTargets.filter((target) => deletedSet.has(target.id)).map((target) => target.id) -} - -/** - * Shared delete-with-toast flow for both DeleteWorktreeDialog (confirm) and - * WorktreeContextMenu (skip-confirm), so both entry points behave identically. - * - * A renderer-layer helper (not a store action) to keep UI concerns out of the store slice. - */ -export function runWorktreeDeleteWithToast( - worktreeId: string, - worktreeName: string, - options: WorktreeDeleteWithToastOptions = {} -): Promise { - const removeWorktree = useAppStore.getState().removeWorktree - const commitFocus = prepareActiveWorktreeFocusAfterDelete(worktreeId) - const focusSuccessor = options.focusSuccessorOnDelete !== false - - return removeWorktree(worktreeId, options.force === true) - .then((result) => { - if (result.ok) { - // Why: keep the user on a live workspace instead of the Landing screen when they delete the one they were viewing. - if (focusSuccessor) { - commitFocus() - } - return true - } - const state = useAppStore.getState().deleteStateByWorktreeId[worktreeId] - const canForceDelete = state?.canForceDelete ?? false - const hasKnownChanges = - (useAppStore.getState().gitStatusByWorktree[worktreeId]?.length ?? 0) > 0 - showDeleteWorktreeFailureToast({ - error: result.error, - canForceDelete, - forceDeleteReason: state?.forceDeleteReason ?? null, - lockReason: state?.lockReason ?? null, - hasKnownChanges, - onViewChanges: () => viewWorktreeDiff(worktreeId), - onForceDelete: () => { - // Why: recapture at click time — the user may have navigated away while the toast was open, so focus only hands off if still viewed. - const commitForceFocus = prepareActiveWorktreeFocusAfterDelete(worktreeId) - // Why (#11960): the user clicked Force Delete on a failure toast, so this - // retry may waive the PTY-stop proof the first attempt could not satisfy. - const forceRemoval = useAppStore - .getState() - .removeWorktree(worktreeId, true, { allowUnverifiedPtyStop: true }) - forceRemoval - .then((forceResult) => { - if (!forceResult.ok) { - toast.error( - translate( - 'auto.components.sidebar.delete.worktree.flow.4f3876c0f5', - 'Force delete failed' - ), - { - description: forceResult.error, - action: { - label: translate( - 'auto.components.sidebar.delete.worktree.flow.7488ed8711', - 'View' - ), - onClick: () => viewWorktreeDiff(worktreeId) - } - } - ) - return - } - commitForceFocus() - options.onForceDeleted?.(worktreeId) - }) - .catch((err: unknown) => { - toast.error( - translate( - 'auto.components.sidebar.delete.worktree.flow.ae57cbf6e4', - 'Failed to delete workspace' - ), - { - description: err instanceof Error ? err.message : String(err), - action: { - label: translate( - 'auto.components.sidebar.delete.worktree.flow.7488ed8711', - 'View' - ), - onClick: () => viewWorktreeDiff(worktreeId) - } - } - ) - }) - }, - worktreeId, - worktreeName - }) - return false - }) - .catch((err: unknown) => { - toast.error( - translate( - 'auto.components.sidebar.delete.worktree.flow.ae57cbf6e4', - 'Failed to delete workspace' - ), - { - description: err instanceof Error ? err.message : String(err) - } - ) - return false - }) -} +export { runWorktreeDeletesInParallel, runWorktreeDeleteWithToast } /** * Shared funnel for the standard (non-folder) delete decision tree (WorktreeContextMenu, * MemoryStatusSegment); branches on the `skipDeleteWorktreeConfirm` preference. * - * The missing-record guard is defense-in-depth: refuse to act if the record vanished - * between render and click (concurrent delete or state reset). + * The missing-record and instance guards reject stale actions after concurrent state changes. */ -export function runWorktreeDelete(worktreeId: string): void { +export function runWorktreeDelete(worktreeId: string, options: WorktreeDeleteOptions = {}): void { const state = useAppStore.getState() const target = getWorktreeMapFromState(state).get(worktreeId) ?? null - if (!target) { + const instanceChanged = + Object.hasOwn(options, 'expectedInstanceId') && + target?.instanceId !== options.expectedInstanceId + if (!target || instanceChanged) { + // Why: folder workspaces are never in the worktree map — their callers own that route, so a + // miss there is a routing gap, not a stale list, and must not claim the workspace is gone. + if (parseWorkspaceKey(worktreeId)?.type !== 'folder') { + showWorkspaceListChangedToast() + } return } if (target.isMainWorktree) { @@ -254,9 +79,12 @@ export function runWorktreeDelete(worktreeId: string): void { return } - const hasLineageChildren = - getWorkspaceDeleteLineage(target, getAllWorktreesFromState(state), state.worktreeLineageById) - .descendants.length > 0 + const deleteLineage = getWorkspaceDeleteLineage( + target, + getAllWorktreesFromState(state), + state.worktreeLineageById + ) + const hasLineageChildren = deleteLineage.descendants.length > 0 const skipConfirm = state.settings?.skipDeleteWorktreeConfirm ?? false if (skipConfirm && !hasLineageChildren) { void runWorktreeDeleteWithToast(worktreeId, target.displayName) @@ -264,33 +92,32 @@ export function runWorktreeDelete(worktreeId: string): void { } state.openModal('delete-worktree', { worktreeId, + worktreeDeleteIdentities: toWorktreeDeleteIdentities([target]), + ...(hasLineageChildren + ? { + lineageDeleteIdentities: toWorktreeDeleteIdentities(deleteLineage.deleteAllTargets) + } + : {}), ...(hasLineageChildren ? { allowSkipConfirm: false } : {}) }) } export function runWorktreeBatchDelete( - worktreeIds: readonly string[], + requestedWorktrees: readonly string[] | readonly WorktreeDeleteIdentity[], options: WorktreeBatchDeleteOptions = {} ): boolean { const state = useAppStore.getState() - const worktreeMap = getWorktreeMapFromState(state) - const targets = Array.from(new Set(worktreeIds)) - .map((id) => worktreeMap.get(id) ?? null) - .filter((worktree): worktree is Worktree => worktree != null && !worktree.isMainWorktree) + const targets = resolveWorktreeBatchDeleteTargets( + requestedWorktrees, + getWorktreeMapFromState(state) + ) + if (!targets) { + showWorkspaceListChangedToast() + return false + } if (targets.length === 0) { - toast.info( - translate( - 'auto.components.sidebar.delete.worktree.flow.7243145cd6', - 'No deletable workspaces selected' - ), - { - description: translate( - 'auto.components.sidebar.delete.worktree.flow.b81b4e40ca', - 'Refresh Space and try again if the workspace list looks stale.' - ) - } - ) + showNoDeletableWorkspacesToast() return false } @@ -299,13 +126,15 @@ 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], - getAllWorktreesFromState(state), - state.worktreeLineageById - ).descendants.length > 0 + const singleTargetLineage = + targets.length === 1 + ? getWorkspaceDeleteLineage( + targets[0], + getAllWorktreesFromState(state), + state.worktreeLineageById + ) + : null + const singleTargetHasLineageChildren = (singleTargetLineage?.descendants.length ?? 0) > 0 const skipConfirm = !options.forceConfirm && targets.length === 1 && @@ -325,6 +154,14 @@ export function runWorktreeBatchDelete( if (targets.length === 1) { state.openModal('delete-worktree', { worktreeId: targets[0].id, + worktreeDeleteIdentities: toWorktreeDeleteIdentities(targets), + ...(singleTargetHasLineageChildren && singleTargetLineage + ? { + lineageDeleteIdentities: toWorktreeDeleteIdentities( + singleTargetLineage.deleteAllTargets + ) + } + : {}), ...(options.forceConfirm || singleTargetHasLineageChildren ? { allowSkipConfirm: false } : {}), @@ -335,6 +172,7 @@ export function runWorktreeBatchDelete( state.openModal('delete-worktree', { worktreeIds: targets.map((target) => target.id), + worktreeDeleteIdentities: toWorktreeDeleteIdentities(targets), allowSkipConfirm: false, ...(options.onDeleted ? { onDeleted: options.onDeleted } : {}) }) 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 817a02f2040..8def3dedde2 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 @@ -49,6 +49,14 @@ vi.mock('sonner', () => ({ import { toast } from 'sonner' import { runWorktreeDeletesInParallel } from './delete-worktree-flow' +function runDeletesForCurrentWorktrees( + targets: Parameters[0], + options?: Parameters[1] +) { + mocks.state.worktreeMap = new Map(targets.map((target) => [target.id, target])) + return runWorktreeDeletesInParallel(targets, options) +} + function deferredDeleteResult(): { promise: Promise<{ ok: true }> resolve: (value: { ok: true }) => void @@ -65,6 +73,7 @@ describe('runWorktreeDeletesInParallel', () => { mocks.state.removeWorktree.mockClear().mockResolvedValue({ ok: true }) mocks.state.clearWorktreeDeleteState.mockClear() mocks.state.markWorktreesDeleting.mockClear() + mocks.state.worktreeMap = new Map() mocks.state.deleteStateByWorktreeId = {} vi.mocked(toast.error).mockClear() vi.mocked(toast.info).mockClear() @@ -77,7 +86,7 @@ describe('runWorktreeDeletesInParallel', () => { .mockReturnValueOnce(first.promise) .mockReturnValueOnce(second.promise) - const deleted = runWorktreeDeletesInParallel([ + const deleted = runDeletesForCurrentWorktrees([ { id: 'wt-1', displayName: 'one', repoId: 'repo-a', path: '/workspaces/one' }, { id: 'wt-2', displayName: 'two', repoId: 'repo-b', path: '/workspaces/two' } ]) @@ -98,7 +107,7 @@ describe('runWorktreeDeletesInParallel', () => { const childDelete = deferredDeleteResult() mocks.state.removeWorktree.mockReturnValueOnce(childDelete.promise) - const deleted = runWorktreeDeletesInParallel([ + const deleted = runDeletesForCurrentWorktrees([ { id: 'parent', displayName: 'parent', repoId: 'repo-a', path: '/workspaces/parent' }, { id: 'child', displayName: 'child', repoId: 'repo-a', path: '/workspaces/parent/child' } ]) @@ -124,7 +133,7 @@ describe('runWorktreeDeletesInParallel', () => { }) it('deletes nested workspaces before their parent within the same repo', async () => { - await runWorktreeDeletesInParallel([ + await runDeletesForCurrentWorktrees([ { id: 'parent', displayName: 'parent', repoId: 'repo-a', path: '/workspaces/parent' }, { id: 'child', displayName: 'child', repoId: 'repo-a', path: '/workspaces/parent/child' } ]) @@ -134,7 +143,7 @@ describe('runWorktreeDeletesInParallel', () => { }) it('passes confirmed force to each delete', async () => { - await runWorktreeDeletesInParallel( + await runDeletesForCurrentWorktrees( [ { id: 'wt-1', displayName: 'one', repoId: 'repo-a', path: '/workspaces/one' }, { id: 'wt-2', displayName: 'two', repoId: 'repo-b', path: '/workspaces/two' } @@ -157,7 +166,7 @@ describe('runWorktreeDeletesInParallel', () => { .mockResolvedValueOnce({ ok: true }) .mockResolvedValueOnce({ ok: false, error: 'selector_not_found' }) - await expect(runWorktreeDeletesInParallel([target, target])).resolves.toEqual(['wt-1']) + await expect(runDeletesForCurrentWorktrees([target, target])).resolves.toEqual(['wt-1']) expect(mocks.state.markWorktreesDeleting).toHaveBeenCalledWith(['wt-1']) expect(mocks.state.removeWorktree).toHaveBeenCalledTimes(1) @@ -176,7 +185,7 @@ describe('runWorktreeDeletesInParallel', () => { }) await expect( - runWorktreeDeletesInParallel([ + runDeletesForCurrentWorktrees([ { id: 'parent', displayName: 'parent', repoId: 'repo-a', path: '/workspaces/parent' }, { id: 'child', displayName: 'child', repoId: 'repo-a', path: '/workspaces/parent/child' } ]) diff --git a/src/renderer/src/components/sidebar/stale-workspace-list-toast.ts b/src/renderer/src/components/sidebar/stale-workspace-list-toast.ts new file mode 100644 index 00000000000..890d4c91a60 --- /dev/null +++ b/src/renderer/src/components/sidebar/stale-workspace-list-toast.ts @@ -0,0 +1,33 @@ +import { toast } from 'sonner' +import { translate } from '@/i18n/i18n' + +// Why: keys keep their original `delete.worktree.flow` namespace so the existing +// translations are not orphaned by the move out of that module. +function staleWorkspaceListToast(title: string): void { + toast.info(title, { + description: translate( + 'auto.components.sidebar.delete.worktree.flow.b81b4e40ca', + 'Refresh Space and try again if the workspace list looks stale.' + ) + }) +} + +/** A delete target changed or vanished after the action was selected. */ +export function showWorkspaceListChangedToast(): void { + staleWorkspaceListToast( + translate( + 'auto.components.sidebar.delete.worktree.flow.workspaceListChanged', + 'Workspace list changed' + ) + ) +} + +/** A multi-select delete whose selection resolved to nothing deletable. */ +export function showNoDeletableWorkspacesToast(): void { + staleWorkspaceListToast( + translate( + 'auto.components.sidebar.delete.worktree.flow.7243145cd6', + 'No deletable workspaces selected' + ) + ) +} diff --git a/src/renderer/src/components/sidebar/use-confirmed-worktree-delete-targets.ts b/src/renderer/src/components/sidebar/use-confirmed-worktree-delete-targets.ts new file mode 100644 index 00000000000..f2e0dd7929b --- /dev/null +++ b/src/renderer/src/components/sidebar/use-confirmed-worktree-delete-targets.ts @@ -0,0 +1,45 @@ +import { useCallback, useMemo } from 'react' +import { useAppStore } from '@/store' +import { getWorktreeMapFromState } from '@/store/selectors' +import { + readWorktreeDeleteIdentities, + resolveWorktreeBatchDeleteTargets, + type WorktreeDeleteIdentity +} from './worktree-delete-request' +import { showWorkspaceListChangedToast } from './stale-workspace-list-toast' + +export function useConfirmedWorktreeDeleteTargets({ + worktreeIdentityData, + lineageIdentityData, + closeModal +}: { + worktreeIdentityData: unknown + lineageIdentityData: unknown + closeModal: () => void +}) { + const worktreeDeleteIdentities = useMemo( + () => readWorktreeDeleteIdentities(worktreeIdentityData), + [worktreeIdentityData] + ) + const lineageDeleteIdentities = useMemo( + () => readWorktreeDeleteIdentities(lineageIdentityData), + [lineageIdentityData] + ) + const resolveConfirmedTargets = useCallback( + (identities: readonly WorktreeDeleteIdentity[], expectedCount: number) => { + const targets = resolveWorktreeBatchDeleteTargets( + identities, + getWorktreeMapFromState(useAppStore.getState()) + ) + if (!targets || targets.length !== expectedCount) { + showWorkspaceListChangedToast() + closeModal() + return null + } + return targets + }, + [closeModal] + ) + + return { worktreeDeleteIdentities, lineageDeleteIdentities, resolveConfirmedTargets } +} diff --git a/src/renderer/src/components/sidebar/use-delete-worktree-status-hydration.ts b/src/renderer/src/components/sidebar/use-delete-worktree-status-hydration.ts new file mode 100644 index 00000000000..2a88a430c50 --- /dev/null +++ b/src/renderer/src/components/sidebar/use-delete-worktree-status-hydration.ts @@ -0,0 +1,60 @@ +import { useEffect } from 'react' +import { useAppStore } from '@/store' +import { getConnectionId } from '@/lib/connection-context' +import { getSettingsForWorktreeRuntimeOwner } from '@/lib/worktree-runtime-owner' +import { getRuntimeGitStatus } from '@/runtime/runtime-git-client' +import type { Repo, Worktree } from '../../../../shared/types' +import { isFolderWorkspaceDelete } from './delete-worktree-dialog-copy' + +export function useDeleteWorktreeStatusHydration({ + isOpen, + deleteTargets, + repoMap +}: { + isOpen: boolean + deleteTargets: readonly Worktree[] + repoMap: ReadonlyMap +}): void { + const repos = useAppStore((state) => state.repos) + const settings = useAppStore((state) => state.settings) + const setGitStatus = useAppStore((state) => state.setGitStatus) + + useEffect(() => { + if (!isOpen) { + return + } + const gitStatusByWorktree = useAppStore.getState().gitStatusByWorktree + const targets = deleteTargets.filter( + (target) => + !target.isMainWorktree && + !isFolderWorkspaceDelete(repoMap, target) && + gitStatusByWorktree[target.id] === undefined + ) + const controller = new AbortController() + for (const target of targets) { + void getRuntimeGitStatus( + { + settings: getSettingsForWorktreeRuntimeOwner( + { repos, settings, worktreesByRepo: useAppStore.getState().worktreesByRepo }, + target.id + ), + worktreeId: target.id, + worktreePath: target.path, + connectionId: getConnectionId(target.id) ?? undefined + }, + { signal: controller.signal } + ) + .then((status) => { + if (!controller.signal.aborted) { + setGitStatus(target.id, status) + } + }) + .catch(() => { + // Best effort only; deletion performs the authoritative backend check. + }) + } + return () => { + controller.abort() + } + }, [deleteTargets, isOpen, repoMap, repos, setGitStatus, settings]) +} diff --git a/src/renderer/src/components/sidebar/worktree-context-menu-delete-intent.test.ts b/src/renderer/src/components/sidebar/worktree-context-menu-delete-intent.test.ts new file mode 100644 index 00000000000..20d28499be9 --- /dev/null +++ b/src/renderer/src/components/sidebar/worktree-context-menu-delete-intent.test.ts @@ -0,0 +1,76 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' + +const mocks = vi.hoisted(() => ({ runDelete: vi.fn(), runBatchDelete: vi.fn() })) + +vi.mock('@/store', () => ({ useAppStore: { getState: vi.fn() } })) +vi.mock('./delete-worktree-flow', () => ({ + runWorktreeDelete: mocks.runDelete, + runWorktreeBatchDelete: mocks.runBatchDelete +})) + +import { deferWorktreeContextMenuDeleteIntent } from './worktree-context-menu-delete-intent' + +describe('deferWorktreeContextMenuDeleteIntent', () => { + beforeEach(() => { + vi.clearAllMocks() + }) + + afterEach(() => { + vi.useRealTimers() + vi.unstubAllGlobals() + }) + + it('dispatches the selected workspace identity after the menu event completes', () => { + const defer = vi.fn<(callback: () => void) => void>() + const intent = { + kind: 'worktree' as const, + worktree: { id: 'repo::/work/wt', instanceId: 'instance-1' } + } + const onDispatched = vi.fn() + + deferWorktreeContextMenuDeleteIntent(intent, onDispatched, defer) + + expect(mocks.runDelete).not.toHaveBeenCalled() + expect(onDispatched).not.toHaveBeenCalled() + expect(defer).toHaveBeenCalledOnce() + + const [deferred] = defer.mock.calls[0] + deferred() + + expect(mocks.runDelete).toHaveBeenCalledWith('repo::/work/wt', { + expectedInstanceId: 'instance-1' + }) + expect(onDispatched).toHaveBeenCalledOnce() + }) + + it('preserves every selected workspace identity for batch validation', () => { + const intent = { + kind: 'batch' as const, + worktrees: [ + { id: 'wt-1', instanceId: 'instance-1' }, + { id: 'wt-2', instanceId: 'instance-2' } + ] + } + + deferWorktreeContextMenuDeleteIntent(intent, undefined, (callback) => callback()) + + expect(mocks.runBatchDelete).toHaveBeenCalledWith(intent.worktrees) + }) + + it('dispatches on the next macrotask by default', () => { + vi.useFakeTimers() + vi.stubGlobal('window', { setTimeout }) + const intent = { + kind: 'worktree' as const, + worktree: { id: 'wt-1', instanceId: 'instance-1' } + } + + deferWorktreeContextMenuDeleteIntent(intent) + + expect(mocks.runDelete).not.toHaveBeenCalled() + vi.runAllTimers() + expect(mocks.runDelete).toHaveBeenCalledWith('wt-1', { + expectedInstanceId: 'instance-1' + }) + }) +}) diff --git a/src/renderer/src/components/sidebar/worktree-context-menu-delete-intent.ts b/src/renderer/src/components/sidebar/worktree-context-menu-delete-intent.ts new file mode 100644 index 00000000000..7662af6d247 --- /dev/null +++ b/src/renderer/src/components/sidebar/worktree-context-menu-delete-intent.ts @@ -0,0 +1,38 @@ +import { useAppStore } from '@/store' +import { folderWorkspaceKey } from '../../../../shared/workspace-scope' +import { runWorktreeBatchDelete, runWorktreeDelete } from './delete-worktree-flow' +import type { WorktreeDeleteIdentity } from './worktree-delete-request' + +export type WorktreeContextMenuDeleteIntent = + | { kind: 'worktree'; worktree: WorktreeDeleteIdentity } + | { kind: 'batch'; worktrees: readonly WorktreeDeleteIdentity[] } + | { kind: 'folder'; folderWorkspaceId: string } + +export function runWorktreeContextMenuDeleteIntent(intent: WorktreeContextMenuDeleteIntent): void { + if (intent.kind === 'batch') { + runWorktreeBatchDelete(intent.worktrees) + return + } + if (intent.kind === 'worktree') { + runWorktreeDelete(intent.worktree.id, { expectedInstanceId: intent.worktree.instanceId }) + return + } + const state = useAppStore.getState() + void state.deleteFolderWorkspace(intent.folderWorkspaceId).then((deleted) => { + const current = useAppStore.getState() + if (deleted && current.activeWorktreeId === folderWorkspaceKey(intent.folderWorkspaceId)) { + current.setActiveWorktree(null) + } + }) +} + +export function deferWorktreeContextMenuDeleteIntent( + intent: WorktreeContextMenuDeleteIntent, + onDispatched?: () => void, + defer: (callback: () => void) => void = (callback) => window.setTimeout(callback, 0) +): void { + defer(() => { + runWorktreeContextMenuDeleteIntent(intent) + onDispatched?.() + }) +} diff --git a/src/renderer/src/components/sidebar/worktree-delete-execution.ts b/src/renderer/src/components/sidebar/worktree-delete-execution.ts new file mode 100644 index 00000000000..df556f4e5cb --- /dev/null +++ b/src/renderer/src/components/sidebar/worktree-delete-execution.ts @@ -0,0 +1,194 @@ +import { toast } from 'sonner' +import { useAppStore } from '@/store' +import { getWorktreeMapFromState } from '@/store/selectors' +import { activateAndRevealWorktree } from '@/lib/worktree-activation' +import { translate } from '@/i18n/i18n' +import { + isPathInsideOrEqual, + normalizeRuntimePathForComparison +} from '../../../../shared/cross-platform-path' +import type { Worktree } from '../../../../shared/types' +import { prepareActiveWorktreeFocusAfterDelete } from './active-worktree-focus-after-delete' +import { showDeleteWorktreeFailureToast } from './delete-worktree-failure-toast' +import { showWorkspaceListChangedToast } from './stale-workspace-list-toast' +import type { WorktreeDeleteWithToastOptions } from './worktree-delete-request' + +// A failed delete usually means unresolved changes, so land on the diff panel. +function viewWorktreeDiff(worktreeId: string): void { + activateAndRevealWorktree(worktreeId) + const state = useAppStore.getState() + state.setRightSidebarTab('source-control') + state.setRightSidebarOpen(true) +} + +function isStrictDescendantPath(parentPath: string, childPath: string): boolean { + return ( + normalizeRuntimePathForComparison(parentPath) !== + normalizeRuntimePathForComparison(childPath) && isPathInsideOrEqual(parentPath, childPath) + ) +} + +export async function runWorktreeDeletesInParallel( + targets: readonly Pick[], + options: WorktreeDeleteWithToastOptions = {} +): Promise { + // A destructive command must run once per identity even if a refresh duplicated rows. + const uniqueTargets = Array.from(new Map(targets.map((target) => [target.id, target])).values()) + // Batch focus is committed once after every target settles. + const activeWorktreeIdBefore = useAppStore.getState().activeWorktreeId + const commitBatchFocus = activeWorktreeIdBefore + ? prepareActiveWorktreeFocusAfterDelete(activeWorktreeIdBefore) + : null + // Mark all targets up front so the sidebar shows immediate progress. + useAppStore.getState().markWorktreesDeleting(uniqueTargets.map((target) => target.id)) + // Git worktree removal shares repo locks, while separate repos can proceed in parallel. + const groups = new Map() + for (const target of uniqueTargets) { + const group = groups.get(target.repoId) + if (group) { + group.push(target) + } else { + groups.set(target.repoId, [target]) + } + } + for (const group of groups.values()) { + // Children must leave first or Git rejects their registered ancestor. + group.sort((a, b) => b.path.length - a.path.length) + } + let listChanged = false + const groupResults = await Promise.all( + Array.from(groups.values()).map(async (group) => { + const deletedInGroup: string[] = [] + const failedInGroup: (typeof group)[number][] = [] + for (const target of group) { + // A queued target may be recreated while an earlier repo sibling is deleting. + const currentTarget = getWorktreeMapFromState(useAppStore.getState()).get(target.id) + if (!currentTarget || currentTarget.instanceId !== target.instanceId) { + useAppStore.getState().clearWorktreeDeleteState(target.id) + listChanged = true + continue + } + if (failedInGroup.some((failed) => isStrictDescendantPath(target.path, failed.path))) { + useAppStore.getState().clearWorktreeDeleteState(target.id) + continue + } + const deleted = await runWorktreeDeleteWithToast(target.id, target.displayName, { + ...options, + focusSuccessorOnDelete: false + }) + if (deleted) { + deletedInGroup.push(target.id) + } else { + // A failed child makes deleting its ancestor unsafe because the child lives below it. + failedInGroup.push(target) + } + } + return deletedInGroup + }) + ) + if (listChanged) { + showWorkspaceListChangedToast() + } + const deletedSet = new Set(groupResults.flat()) + // Intermediate focus can spawn a terminal in another target that is still queued. + if (activeWorktreeIdBefore && deletedSet.has(activeWorktreeIdBefore)) { + commitBatchFocus?.() + } + return uniqueTargets.filter((target) => deletedSet.has(target.id)).map((target) => target.id) +} + +/** Shared confirmed and skip-confirm execution with consistent failure recovery. */ +export function runWorktreeDeleteWithToast( + worktreeId: string, + worktreeName: string, + options: WorktreeDeleteWithToastOptions = {} +): Promise { + const removeWorktree = useAppStore.getState().removeWorktree + const commitFocus = prepareActiveWorktreeFocusAfterDelete(worktreeId) + const focusSuccessor = options.focusSuccessorOnDelete !== false + + return removeWorktree(worktreeId, options.force === true) + .then((result) => { + if (result.ok) { + if (focusSuccessor) { + commitFocus() + } + return true + } + const state = useAppStore.getState().deleteStateByWorktreeId[worktreeId] + const canForceDelete = state?.canForceDelete ?? false + const hasKnownChanges = + (useAppStore.getState().gitStatusByWorktree[worktreeId]?.length ?? 0) > 0 + showDeleteWorktreeFailureToast({ + error: result.error, + canForceDelete, + forceDeleteReason: state?.forceDeleteReason ?? null, + lockReason: state?.lockReason ?? null, + hasKnownChanges, + onViewChanges: () => viewWorktreeDiff(worktreeId), + onForceDelete: () => { + // Recapture focus because the user may have navigated while the toast was open. + const commitForceFocus = prepareActiveWorktreeFocusAfterDelete(worktreeId) + // The explicit Force Delete retry may waive an unverified PTY-stop proof. + const forceRemoval = useAppStore + .getState() + .removeWorktree(worktreeId, true, { allowUnverifiedPtyStop: true }) + forceRemoval + .then((forceResult) => { + if (!forceResult.ok) { + toast.error( + translate( + 'auto.components.sidebar.delete.worktree.flow.4f3876c0f5', + 'Force delete failed' + ), + { + description: forceResult.error, + action: { + label: translate( + 'auto.components.sidebar.delete.worktree.flow.7488ed8711', + 'View' + ), + onClick: () => viewWorktreeDiff(worktreeId) + } + } + ) + return + } + commitForceFocus() + options.onForceDeleted?.(worktreeId) + }) + .catch((err: unknown) => { + toast.error( + translate( + 'auto.components.sidebar.delete.worktree.flow.ae57cbf6e4', + 'Failed to delete workspace' + ), + { + description: err instanceof Error ? err.message : String(err), + action: { + label: translate( + 'auto.components.sidebar.delete.worktree.flow.7488ed8711', + 'View' + ), + onClick: () => viewWorktreeDiff(worktreeId) + } + } + ) + }) + }, + worktreeId, + worktreeName + }) + return false + }) + .catch((err: unknown) => { + toast.error( + translate( + 'auto.components.sidebar.delete.worktree.flow.ae57cbf6e4', + 'Failed to delete workspace' + ), + { description: err instanceof Error ? err.message : String(err) } + ) + return false + }) +} diff --git a/src/renderer/src/components/sidebar/worktree-delete-request.ts b/src/renderer/src/components/sidebar/worktree-delete-request.ts new file mode 100644 index 00000000000..4092c5cde51 --- /dev/null +++ b/src/renderer/src/components/sidebar/worktree-delete-request.ts @@ -0,0 +1,65 @@ +import type { Worktree } from '../../../../shared/types' + +export type WorktreeBatchDeleteOptions = { + forceConfirm?: boolean + onDeleted?: (worktreeIds: string[]) => void +} + +export type WorktreeDeleteIdentity = Pick + +export type WorktreeDeleteOptions = { + expectedInstanceId?: string +} + +export type WorktreeDeleteWithToastOptions = { + force?: boolean + onForceDeleted?: (worktreeId: string) => void + // Batch deletion commits one focus handoff after all targets settle. + focusSuccessorOnDelete?: boolean +} + +export function toWorktreeDeleteIdentities( + worktrees: readonly Pick[] +): WorktreeDeleteIdentity[] { + return worktrees.map(({ id, instanceId }) => ({ id, instanceId })) +} + +export function resolveWorktreeBatchDeleteTargets( + requestedWorktrees: readonly string[] | readonly WorktreeDeleteIdentity[], + worktreeMap: ReadonlyMap +): Worktree[] | null { + const uniqueRequests = Array.from( + new Map( + requestedWorktrees.map( + (request) => [typeof request === 'string' ? request : request.id, request] as const + ) + ).values() + ) + const targets: Worktree[] = [] + for (const request of uniqueRequests) { + const worktreeId = typeof request === 'string' ? request : request.id + const target = worktreeMap.get(worktreeId) ?? null + if (typeof request !== 'string' && (!target || target.instanceId !== request.instanceId)) { + return null + } + if (target && !target.isMainWorktree) { + targets.push(target) + } + } + return targets +} + +export function readWorktreeDeleteIdentities(value: unknown): WorktreeDeleteIdentity[] { + if (!Array.isArray(value)) { + return [] + } + return value.flatMap((entry) => { + if (!entry || typeof entry !== 'object' || !('id' in entry) || typeof entry.id !== 'string') { + return [] + } + const instanceId = 'instanceId' in entry ? entry.instanceId : undefined + return instanceId === undefined || typeof instanceId === 'string' + ? [{ id: entry.id, instanceId }] + : [] + }) +} diff --git a/src/renderer/src/i18n/locales/en.json b/src/renderer/src/i18n/locales/en.json index 1aaead6e8f0..34911f6f372 100644 --- a/src/renderer/src/i18n/locales/en.json +++ b/src/renderer/src/i18n/locales/en.json @@ -4910,7 +4910,8 @@ "ae57cbf6e4": "Failed to delete workspace", "7488ed8711": "View", "2b20ce87b3": "Force Delete", - "4f3876c0f5": "Force delete failed" + "4f3876c0f5": "Force delete failed", + "workspaceListChanged": "Workspace list changed" }, "toast": { "1d0fa5c0a5": "Failed to delete workspace {{value0}}",