diff --git a/src/renderer/src/components/right-sidebar/PRActions.tsx b/src/renderer/src/components/right-sidebar/PRActions.tsx index 725a5b1625e..84cba563e75 100644 --- a/src/renderer/src/components/right-sidebar/PRActions.tsx +++ b/src/renderer/src/components/right-sidebar/PRActions.tsx @@ -4,7 +4,9 @@ import { useAppStore } from '@/store' import { cn } from '@/lib/utils' import { Button } from '@/components/ui/button' import { Tooltip, TooltipTrigger, TooltipContent, TooltipProvider } from '@/components/ui/tooltip' +import { isFolderRepo } from '../../../../shared/repo-kind' import type { PRInfo, Repo, Worktree } from '../../../../shared/types' +import { getWorktreeRemovalAction, openWorktreeRemovalModal } from '../sidebar/worktree-removal' const MERGE_METHODS = ['squash', 'merge', 'rebase'] as const @@ -26,10 +28,15 @@ export default function PRActions({ onRefreshPR: () => Promise }): React.JSX.Element | null { const openModal = useAppStore((s) => s.openModal) + const clearWorktreeDeleteState = useAppStore((s) => s.clearWorktreeDeleteState) + const deleteState = useAppStore((s) => s.deleteStateByWorktreeId[worktree.id]) + const isDeleting = deleteState?.isDeleting ?? false const [merging, setMerging] = useState(false) const [mergeError, setMergeError] = useState(null) const [mergeMenuOpen, setMergeMenuOpen] = useState(false) const mergeMenuRef = useRef(null) + const isFolder = isFolderRepo(repo) + const removalAction = getWorktreeRemovalAction(worktree, isFolder) const handleMerge = useCallback( async (method: 'merge' | 'squash' | 'rebase' = 'squash') => { @@ -70,8 +77,13 @@ export default function PRActions({ }, [mergeMenuOpen]) const handleDeleteWorktree = useCallback(() => { - openModal('delete-worktree', { worktreeId: worktree.id }) - }, [worktree.id, openModal]) + // Why: guard against double-clicks while a delete is already in progress, + // matching the pattern in WorktreeCard and WorktreeContextMenu. + if (isDeleting) { + return + } + openWorktreeRemovalModal(worktree, isFolder, openModal, clearWorktreeDeleteState) + }, [clearWorktreeDeleteState, isDeleting, isFolder, openModal, worktree]) // Why: merging a PR with unresolved conflicts would fail on GitHub anyway; // disabling the button prevents a confusing error and signals the user must @@ -164,9 +176,11 @@ export default function PRActions({ size="xs" className="w-full text-[11px]" onClick={handleDeleteWorktree} + disabled={isDeleting || removalAction.disabled} + title={removalAction.disabledReason} > - Delete Worktree + {isDeleting ? 'Deleting\u2026' : removalAction.label} ) } diff --git a/src/renderer/src/components/sidebar/DeleteWorktreeDialog.tsx b/src/renderer/src/components/sidebar/DeleteWorktreeDialog.tsx index b15858ad766..cafe45215ef 100644 --- a/src/renderer/src/components/sidebar/DeleteWorktreeDialog.tsx +++ b/src/renderer/src/components/sidebar/DeleteWorktreeDialog.tsx @@ -8,7 +8,7 @@ import { DialogTitle } from '@/components/ui/dialog' import { Button } from '@/components/ui/button' -import { AlertTriangle, LoaderCircle, Trash2 } from 'lucide-react' +import { AlertTriangle, LoaderCircle, Trash } from 'lucide-react' import { useAppStore } from '@/store' import { toast } from 'sonner' import { getDeleteWorktreeToastCopy } from './delete-worktree-toast' @@ -179,7 +179,7 @@ const DeleteWorktreeDialog = React.memo(function DeleteWorktreeDialog() { onClick={() => handleDelete(true)} disabled={isDeleting} > - {isDeleting ? : } + {isDeleting ? : } {isDeleting ? 'Force Deleting…' : 'Force Delete'} ) : ( @@ -189,7 +189,7 @@ const DeleteWorktreeDialog = React.memo(function DeleteWorktreeDialog() { onClick={() => handleDelete(false)} disabled={isDeleting} > - {isDeleting ? : } + {isDeleting ? : } {isDeleting ? 'Deleting…' : 'Delete'} ))} diff --git a/src/renderer/src/components/sidebar/WorktreeCard.tsx b/src/renderer/src/components/sidebar/WorktreeCard.tsx index c1b602bd434..725ffd19bb3 100644 --- a/src/renderer/src/components/sidebar/WorktreeCard.tsx +++ b/src/renderer/src/components/sidebar/WorktreeCard.tsx @@ -4,11 +4,12 @@ import { useAppStore } from '@/store' import { Badge } from '@/components/ui/badge' import { HoverCard, HoverCardTrigger, HoverCardContent } from '@/components/ui/hover-card' import { Tooltip, TooltipTrigger, TooltipContent } from '@/components/ui/tooltip' -import { Bell, GitMerge, LoaderCircle, CircleDot, CircleCheck, CircleX } from 'lucide-react' +import { Bell, GitMerge, LoaderCircle, CircleDot, CircleCheck, CircleX, Trash } from 'lucide-react' import StatusIndicator from './StatusIndicator' import CacheTimer from './CacheTimer' import CommentMarkdown from './CommentMarkdown' import WorktreeContextMenu from './WorktreeContextMenu' +import { getWorktreeRemovalAction, openWorktreeRemovalModal } from './worktree-removal' import { cn } from '@/lib/utils' import { getWorktreeStatus, type WorktreeStatus } from '@/lib/worktree-status' import { getRepoKindLabel, isFolderRepo } from '../../../../shared/repo-kind' @@ -96,6 +97,7 @@ const WorktreeCard = React.memo(function WorktreeCard({ }: WorktreeCardProps) { const setActiveWorktree = useAppStore((s) => s.setActiveWorktree) const openModal = useAppStore((s) => s.openModal) + const clearWorktreeDeleteState = useAppStore((s) => s.clearWorktreeDeleteState) const updateWorktreeMeta = useAppStore((s) => s.updateWorktreeMeta) const fetchPRForBranch = useAppStore((s) => s.fetchPRForBranch) const fetchIssue = useAppStore((s) => s.fetchIssue) @@ -152,6 +154,7 @@ const WorktreeCard = React.memo(function WorktreeCard({ : null const isDeleting = deleteState?.isDeleting ?? false + const removalAction = getWorktreeRemovalAction(worktree, isFolder) // Derive status const status: WorktreeStatus = useMemo( @@ -217,6 +220,17 @@ const WorktreeCard = React.memo(function WorktreeCard({ ) const unreadTooltip = worktree.isUnread ? 'Mark read' : 'Mark unread' + const handleQuickDelete = useCallback( + (event: React.MouseEvent) => { + event.preventDefault() + event.stopPropagation() + if (isDeleting || removalAction.disabled) { + return + } + openWorktreeRemovalModal(worktree, isFolder, openModal, clearWorktreeDeleteState) + }, + [clearWorktreeDeleteState, isDeleting, isFolder, openModal, removalAction.disabled, worktree] + ) return ( @@ -293,7 +307,7 @@ const WorktreeCard = React.memo(function WorktreeCard({ {/* Content area */}
{/* Header row: Title and Checks */} -
+
{worktree.displayName} @@ -320,9 +334,9 @@ const WorktreeCard = React.memo(function WorktreeCard({ )}
- {/* CI Checks & PR state on the right */} - {cardProps.includes('ci') && pr && pr.checksStatus !== 'neutral' && ( -
+
+ {/* CI Checks & PR state on the right */} + {cardProps.includes('ci') && pr && pr.checksStatus !== 'neutral' && ( @@ -341,8 +355,41 @@ const WorktreeCard = React.memo(function WorktreeCard({ CI checks {checksLabel(pr.checksStatus).toLowerCase()} -
- )} + )} + + + + + + + {removalAction.disabledReason ?? removalAction.label} + + +
{/* Subtitle row: Repo badge + Branch */} diff --git a/src/renderer/src/components/sidebar/WorktreeContextMenu.tsx b/src/renderer/src/components/sidebar/WorktreeContextMenu.tsx index c1b8869890b..f4e7deb3322 100644 --- a/src/renderer/src/components/sidebar/WorktreeContextMenu.tsx +++ b/src/renderer/src/components/sidebar/WorktreeContextMenu.tsx @@ -15,11 +15,12 @@ import { MessageSquare, Pencil, XCircle, - Trash2 + Trash } from 'lucide-react' import { useAppStore } from '@/store' import type { Worktree } from '../../../../shared/types' import { isFolderRepo } from '../../../../shared/repo-kind' +import { getWorktreeRemovalAction, openWorktreeRemovalModal } from './worktree-removal' type Props = { worktree: Worktree @@ -42,6 +43,7 @@ const WorktreeContextMenu = React.memo(function WorktreeContextMenu({ worktree, const isDeleting = deleteState?.isDeleting ?? false const repo = repos.find((entry) => entry.id === worktree.repoId) const isFolder = repo ? isFolderRepo(repo) : false + const removalAction = getWorktreeRemovalAction(worktree, isFolder) useEffect(() => { const closeMenu = (): void => setMenuOpen(false) @@ -100,26 +102,8 @@ const WorktreeContextMenu = React.memo(function WorktreeContextMenu({ worktree, const handleDelete = useCallback(() => { setMenuOpen(false) - if (isFolder) { - // Why: folder mode reuses the worktree row UI for a synthetic root entry, - // but users still expect "remove" to disconnect the folder from Orca, - // not to run git-style delete semantics against the real folder on disk. - openModal('confirm-remove-folder', { - repoId: worktree.repoId, - displayName: worktree.displayName - }) - return - } - clearWorktreeDeleteState(worktree.id) - openModal('delete-worktree', { worktreeId: worktree.id }) - }, [ - worktree.id, - worktree.repoId, - worktree.displayName, - clearWorktreeDeleteState, - isFolder, - openModal - ]) + openWorktreeRemovalModal(worktree, isFolder, openModal, clearWorktreeDeleteState) + }, [clearWorktreeDeleteState, isFolder, openModal, worktree]) return ( <> @@ -183,15 +167,11 @@ const WorktreeContextMenu = React.memo(function WorktreeContextMenu({ worktree, - - {isDeleting ? 'Deleting…' : isFolder ? 'Remove Folder from Orca' : 'Delete'} + + {isDeleting ? 'Deleting…' : removalAction.label} diff --git a/src/renderer/src/components/sidebar/worktree-removal.test.ts b/src/renderer/src/components/sidebar/worktree-removal.test.ts new file mode 100644 index 00000000000..0671a8116d2 --- /dev/null +++ b/src/renderer/src/components/sidebar/worktree-removal.test.ts @@ -0,0 +1,86 @@ +import { describe, expect, it, vi } from 'vitest' +import type { Worktree } from '../../../../shared/types' +import { getWorktreeRemovalAction, openWorktreeRemovalModal } from './worktree-removal' + +function makeWorktree(overrides: Partial = {}): Worktree { + return { + id: 'repo-1::/tmp/feature-123', + repoId: 'repo-1', + path: '/tmp/feature-123', + head: 'abc123', + branch: 'refs/heads/feature-123', + isBare: false, + isMainWorktree: false, + displayName: 'feature-123', + comment: '', + linkedIssue: null, + linkedPR: null, + isArchived: false, + isUnread: false, + sortOrder: 0, + lastActivityAt: 0, + ...overrides + } +} + +describe('getWorktreeRemovalAction', () => { + it('uses delete worktree copy for normal git worktrees', () => { + expect(getWorktreeRemovalAction(makeWorktree(), false)).toEqual({ + disabled: false, + label: 'Delete worktree' + }) + }) + + it('uses folder removal copy for folder rows', () => { + expect(getWorktreeRemovalAction(makeWorktree(), true)).toEqual({ + disabled: false, + label: 'Remove folder from Orca' + }) + }) + + it('disables deletion for the main worktree', () => { + expect(getWorktreeRemovalAction(makeWorktree({ isMainWorktree: true }), false)).toEqual({ + disabled: true, + disabledReason: 'The main worktree cannot be deleted', + label: 'Delete worktree' + }) + }) +}) + +describe('openWorktreeRemovalModal', () => { + it('opens the delete worktree modal for git worktrees after clearing stale state', () => { + const clearWorktreeDeleteState = vi.fn() + const openModal = vi.fn() + const worktree = makeWorktree() + + openWorktreeRemovalModal(worktree, false, openModal, clearWorktreeDeleteState) + + expect(clearWorktreeDeleteState).toHaveBeenCalledWith(worktree.id) + expect(openModal).toHaveBeenCalledWith('delete-worktree', { worktreeId: worktree.id }) + }) + + it('opens the folder removal modal without clearing git delete state', () => { + const clearWorktreeDeleteState = vi.fn() + const openModal = vi.fn() + const worktree = makeWorktree({ displayName: 'workspace-folder' }) + + openWorktreeRemovalModal(worktree, true, openModal, clearWorktreeDeleteState) + + expect(clearWorktreeDeleteState).not.toHaveBeenCalled() + expect(openModal).toHaveBeenCalledWith('confirm-remove-folder', { + repoId: worktree.repoId, + displayName: worktree.displayName + }) + }) + + it('does not open any modal for the main worktree', () => { + const clearWorktreeDeleteState = vi.fn() + const openModal = vi.fn() + const worktree = makeWorktree({ isMainWorktree: true }) + + openWorktreeRemovalModal(worktree, false, openModal, clearWorktreeDeleteState) + + expect(clearWorktreeDeleteState).not.toHaveBeenCalled() + expect(openModal).not.toHaveBeenCalled() + }) +}) diff --git a/src/renderer/src/components/sidebar/worktree-removal.ts b/src/renderer/src/components/sidebar/worktree-removal.ts new file mode 100644 index 00000000000..82edf140d1b --- /dev/null +++ b/src/renderer/src/components/sidebar/worktree-removal.ts @@ -0,0 +1,67 @@ +import type { Worktree } from '../../../../shared/types' +import type { UISlice } from '@/store/slices/ui' + +type OpenModal = (modal: UISlice['activeModal'], data?: Record) => void +type ClearWorktreeDeleteState = (worktreeId: string) => void + +export type WorktreeRemovalAction = { + disabled: boolean + disabledReason?: string + label: string +} + +export function getWorktreeRemovalAction( + worktree: Pick, + isFolder: boolean +): WorktreeRemovalAction { + if (isFolder) { + return { + disabled: false, + label: 'Remove folder from Orca' + } + } + + if (worktree.isMainWorktree) { + return { + disabled: true, + disabledReason: 'The main worktree cannot be deleted', + label: 'Delete worktree' + } + } + + return { + disabled: false, + label: 'Delete worktree' + } +} + +export function openWorktreeRemovalModal( + worktree: Pick, + isFolder: boolean, + openModal: OpenModal, + clearWorktreeDeleteState: ClearWorktreeDeleteState +): void { + if (!isFolder && worktree.isMainWorktree) { + // Why: the shared helper is the boundary between row-level affordances and + // the destructive modal flow. Guarding the main-worktree invariant here + // keeps future callers from reintroducing a modal path Git can never honor. + return + } + + if (isFolder) { + // Why: folder mode reuses the worktree row UI for a synthetic root entry, + // but removal there only disconnects Orca from that folder. Reusing the + // git delete dialog would imply filesystem deletion semantics we do not do. + openModal('confirm-remove-folder', { + repoId: worktree.repoId, + displayName: worktree.displayName + }) + return + } + + // Why: both the context menu and the new hover button lead into the same + // confirmation dialog. Clearing stale delete state here ensures either entry + // point starts with a clean dialog instead of re-showing an old error. + clearWorktreeDeleteState(worktree.id) + openModal('delete-worktree', { worktreeId: worktree.id }) +}