From 698329eda24954911f5f7edacd4f271aba7a157f Mon Sep 17 00:00:00 2001 From: kbwo Date: Sat, 22 Aug 2026 18:00:38 +0900 Subject: [PATCH] feat: delete a worktree from the Space actions menu Deleting a worktree required the D screen, which lists every worktree and makes you find the one already highlighted in the menu. Space now opens the actions menu for the highlighted row and offers deleting that worktree directly. - Space opens the actions menu for any worktree row, not only rows with a session. Rename/Close are session-specific and stay hidden without one, so the menu title switches to "Worktree Actions". - The delete entry is hidden for worktrees that cannot be deleted (main worktree, and the worktree containing the current working directory) rather than shown and rejected. That rule moves into isDeletableWorktree() so the D screen and the new entry share one source. - Choosing it reuses DeleteConfirmation (uncommitted-changes warning, branch option, Cancel-by-default) and then the existing deletion path, which kills the worktree's sessions first. A failure returns to the menu with the error, since the multi-select screen was never opened here. The Dashboard entry point into the actions menu has only a session, no worktree, so it offers no deletion. Co-Authored-By: Claude Opus 5 (1M context) --- src/components/App.tsx | 78 +++++++++-- src/components/DeleteWorktree.tsx | 18 +-- src/components/Menu.test.tsx | 56 +++++++- src/components/Menu.tsx | 50 +++---- src/components/SessionActions.test.tsx | 172 +++++++++++++++++++++++++ src/components/SessionActions.tsx | 67 +++++++--- src/types/index.ts | 7 +- src/utils/worktreeUtils.test.ts | 50 +++++++ src/utils/worktreeUtils.ts | 30 +++++ 9 files changed, 463 insertions(+), 65 deletions(-) create mode 100644 src/components/SessionActions.test.tsx diff --git a/src/components/App.tsx b/src/components/App.tsx index 6e1b811e..4c5db979 100644 --- a/src/components/App.tsx +++ b/src/components/App.tsx @@ -6,6 +6,7 @@ import Dashboard from './Dashboard.js'; import Session from './Session.js'; import NewWorktree from './NewWorktree.js'; import DeleteWorktree from './DeleteWorktree.js'; +import DeleteConfirmation from './DeleteConfirmation.js'; import MergeWorktree from './MergeWorktree.js'; import Configuration from './Configuration.js'; import PresetSelector from './PresetSelector.js'; @@ -36,7 +37,10 @@ import {ConfigScope} from '../types/index.js'; import {ENV_VARS} from '../constants/env.js'; import {MULTI_PROJECT_ERRORS} from '../constants/error.js'; import {projectManager} from '../services/projectManager.js'; -import {generateWorktreeDirectory} from '../utils/worktreeUtils.js'; +import { + generateWorktreeDirectory, + isDeletableWorktree, +} from '../utils/worktreeUtils.js'; type View = | 'menu' @@ -48,6 +52,7 @@ type View = | 'creating-session' | 'creating-session-preset' | 'delete-worktree' + | 'confirm-delete-worktree' | 'deleting-worktree' | 'merge-worktree' | 'configuration' @@ -96,9 +101,17 @@ const App: React.FC = ({ name?: string; } | null>(null); const [sessionActionsTarget, setSessionActionsTarget] = useState<{ - session: ISession; worktreePath: string; + session?: ISession; + // Present only when the actions menu was opened from a menu row, which is + // the only entry point that knows the worktree; the Dashboard opens it + // from a session alone and therefore offers no worktree deletion. + worktree?: Worktree; } | null>(null); + // Worktree awaiting confirmation of the per-row delete action + const [worktreeToDelete, setWorktreeToDelete] = useState( + null, + ); const [selectedProject, setSelectedProject] = useState( null, ); // Store selected project in multi-project mode @@ -498,8 +511,9 @@ const App: React.FC = ({ return; case 'sessionActions': setSessionActionsTarget({ + worktreePath: action.worktree.path, session: action.session, - worktreePath: action.worktreePath, + worktree: action.worktree, }); navigateWithClear('session-actions'); return; @@ -798,6 +812,11 @@ const App: React.FC = ({ const handleDeleteWorktrees = async ( worktreePaths: string[], deleteBranch: boolean, + options?: { + // Where to send the user when a deletion fails. Defaults to the + // multi-select delete screen, which is where this flow starts. + onError?: () => void; + }, ) => { // Set loading context before showing loading view setLoadingContext({deleteBranch}); @@ -846,7 +865,11 @@ const App: React.FC = ({ handleReturnToMenu(); } else { // Show error - setView('delete-worktree'); + if (options?.onError) { + options.onError(); + } else { + setView('delete-worktree'); + } } }; @@ -1092,10 +1115,16 @@ const App: React.FC = ({ } if (view === 'session-actions' && sessionActionsTarget) { - const {session: targetSession, worktreePath} = sessionActionsTarget; - const label = targetSession.sessionName - ? `${worktreePath} : ${targetSession.sessionName}` - : `${worktreePath} #${targetSession.sessionNumber}`; + const { + session: targetSession, + worktreePath, + worktree: targetWorktree, + } = sessionActionsTarget; + const label = !targetSession + ? worktreePath + : targetSession.sessionName + ? `${worktreePath} : ${targetSession.sessionName}` + : `${worktreePath} #${targetSession.sessionNumber}`; const handleSessionAction = async (action: SessionActionType) => { setSessionActionsTarget(null); @@ -1112,6 +1141,7 @@ const App: React.FC = ({ ); return; case 'rename': + if (!targetSession) return; setRenameTarget({ id: targetSession.id, name: targetSession.sessionName, @@ -1119,15 +1149,25 @@ const App: React.FC = ({ navigateWithClear('rename-session'); return; case 'kill': + if (!targetSession) return; sessionManager.destroySession(targetSession.id); handleReturnToMenu(); return; + case 'deleteWorktree': + if (!targetWorktree) return; + setWorktreeToDelete(targetWorktree); + navigateWithClear('confirm-delete-worktree'); + return; } }; return ( { setSessionActionsTarget(null); @@ -1137,6 +1177,28 @@ const App: React.FC = ({ ); } + if (view === 'confirm-delete-worktree' && worktreeToDelete) { + const target = worktreeToDelete; + + return ( + { + setWorktreeToDelete(null); + void handleDeleteWorktrees([target.path], deleteBranch, { + // The multi-select delete screen was never opened in this flow, + // so surface the failure on the menu instead. + onError: handleReturnToMenu, + }); + }} + onCancel={() => { + setWorktreeToDelete(null); + handleReturnToMenu(); + }} + /> + ); + } + if (view === 'preset-selector') { return ( = ({ ); if (!cancelled) { - // Filter out main worktree and current working directory worktree - const resolvedCwd = path.resolve(process.cwd()); - const deletableWorktrees = allWorktrees.filter(wt => { - if (wt.isMainWorktree) return false; - const resolvedPath = path.resolve(wt.path); - if ( - resolvedCwd === resolvedPath || - resolvedCwd.startsWith(resolvedPath + path.sep) - ) { - return false; - } - return true; - }); + const deletableWorktrees = allWorktrees.filter(wt => + isDeletableWorktree(wt), + ); setWorktrees(deletableWorktrees); setIsLoading(false); } diff --git a/src/components/Menu.test.tsx b/src/components/Menu.test.tsx index ac99ba93..72ed4ac4 100644 --- a/src/components/Menu.test.tsx +++ b/src/components/Menu.test.tsx @@ -473,8 +473,62 @@ describe('Menu component Effect-based error handling', () => { expect(onMenuAction).toHaveBeenCalledWith({ type: 'sessionActions', + worktree: cachedWorktree, session: cachedSession, - worktreePath: '/test/cached', + }); + }); + + it('should open the actions menu with Space on a worktree row that has no session', async () => { + const {Effect} = await import('effect'); + const sessionlessWorktree = { + path: '/test/no-session', + branch: 'feature/no-session', + isMainWorktree: false, + hasSession: false, + }; + + vi.spyOn(sessionManager, 'getAllSessions').mockReturnValue([]); + vi.spyOn(worktreeService, 'getWorktreesEffect').mockReturnValue( + Effect.succeed([sessionlessWorktree]), + ); + vi.spyOn(worktreeService, 'getDefaultBranchEffect').mockReturnValue( + Effect.succeed('main'), + ); + + const onMenuAction = vi.fn(); + vi.mocked(useInput).mockClear(); + + render( + , + ); + + await new Promise(resolve => setTimeout(resolve, 0)); + + // Menu's hotkey handler bails out when raw mode is unavailable. + const origSetRawMode = process.stdin.setRawMode; + process.stdin.setRawMode = vi.fn() as never; + try { + const calls = vi.mocked(useInput).mock.calls; + const handler = calls[calls.length - 1]?.[0]; + expect(handler).toBeDefined(); + handler!(' ', makeKey() as never); + } finally { + process.stdin.setRawMode = origSetRawMode; + } + + expect(onMenuAction).toHaveBeenCalledWith({ + type: 'sessionActions', + worktree: sessionlessWorktree, + session: undefined, }); }); }); diff --git a/src/components/Menu.tsx b/src/components/Menu.tsx index 9bb5e598..b611010d 100644 --- a/src/components/Menu.tsx +++ b/src/components/Menu.tsx @@ -126,15 +126,15 @@ const Menu: React.FC = ({ const worktrees = useGitStatus(baseWorktrees, defaultBranch); // Seed from the in-memory session list so the cached snapshot renders with its // sessions attached. Waiting for the async git load would leave every row - // session-less for that window, disabling the Space session-actions shortcut. + // session-less for that window, hiding the session entries of the Space + // actions menu. const [sessions, setSessions] = useState(() => sessionManager.getAllSessions(), ); const [items, setItems] = useState([]); const [recentProjects, setRecentProjects] = useState([]); - const [highlightedWorktreePath, setHighlightedWorktreePath] = useState< - string | null - >(null); + const [highlightedWorktree, setHighlightedWorktree] = + useState(null); const [highlightedSession, setHighlightedSession] = useState< Session | undefined >(undefined); @@ -420,20 +420,23 @@ const Menu: React.FC = ({ } setItems(menuItems); - // Ensure highlighted worktree path is valid for hotkey support - setHighlightedWorktreePath(prev => { - if ( - prev && - menuItems.some( - item => item.type === 'worktree' && item.worktree.path === prev, - ) - ) { - return prev; + // Ensure highlighted worktree is valid for hotkey support + setHighlightedWorktree(prev => { + const stillListed = prev + ? menuItems.find( + item => + item.type === 'worktree' && item.worktree.path === prev.path, + ) + : undefined; + if (stillListed && stillListed.type === 'worktree') { + // Re-read the item so the highlighted worktree keeps up with + // refreshed git status instead of pinning the stale object. + return stillListed.worktree; } const first = menuItems.find(item => item.type === 'worktree'); if (first && first.type === 'worktree') { setHighlightedSession(first.session); - return first.worktree.path; + return first.worktree; } setHighlightedSession(undefined); return null; @@ -521,18 +524,21 @@ const Menu: React.FC = ({ switch (keyPressed) { case 'a': // Toggle auto-approval for the currently highlighted worktree - if (configReader.isAutoApprovalEnabled() && highlightedWorktreePath) { - sessionManager.toggleAutoApprovalForWorktree(highlightedWorktreePath); + if (configReader.isAutoApprovalEnabled() && highlightedWorktree) { + sessionManager.toggleAutoApprovalForWorktree( + highlightedWorktree.path, + ); setAutoApprovalToggleCounter(c => c + 1); } break; case ' ': - // Open session actions for highlighted session - if (highlightedSession && highlightedWorktreePath) { + // Open the row actions for the highlighted worktree. Rows without a + // session open the same menu with only the worktree-level entries. + if (highlightedWorktree) { onMenuAction({ type: 'sessionActions', + worktree: highlightedWorktree, session: highlightedSession, - worktreePath: highlightedWorktreePath, }); } break; @@ -673,7 +679,7 @@ const Menu: React.FC = ({ const item = items.find(i => i.value === raw?.value); if (!item) return; if (item.type === 'worktree') { - setHighlightedWorktreePath(item.worktree.path); + setHighlightedWorktree(item.worktree); setHighlightedSession(item.session); } }} @@ -712,12 +718,12 @@ const Menu: React.FC = ({ {isSearchMode ? 'Search Mode: Type to filter, Enter to exit search, ESC to exit search' : searchQuery - ? `Controls: ↑↓ Navigate Enter Select | /-Search ESC-Clear 0-9 Quick Select Tab-State Filter Space-Session actions (session rows only) N-New M-Merge D-Delete ${ + ? `Controls: ↑↓ Navigate Enter Select | /-Search ESC-Clear 0-9 Quick Select Tab-State Filter Space-Worktree actions N-New M-Merge D-Delete ${ configReader.isAutoApprovalEnabled() ? 'A-AutoApproval ' : '' }${ multiProject ? 'C-Config' : 'P-ProjConfig C-GlobalConfig' } ${projectName ? 'B-Back' : 'Q-Quit'}` - : `Controls: ↑↓ Navigate Enter Select | Hotkeys: 0-9 Quick Select /-Search Tab-State Filter Space-Session actions (session rows only) N-New M-Merge D-Delete ${ + : `Controls: ↑↓ Navigate Enter Select | Hotkeys: 0-9 Quick Select /-Search Tab-State Filter Space-Worktree actions N-New M-Merge D-Delete ${ configReader.isAutoApprovalEnabled() ? 'A-AutoApproval ' : '' }${ multiProject ? 'C-Config' : 'P-ProjConfig C-GlobalConfig' diff --git a/src/components/SessionActions.test.tsx b/src/components/SessionActions.test.tsx new file mode 100644 index 00000000..7a84796c --- /dev/null +++ b/src/components/SessionActions.test.tsx @@ -0,0 +1,172 @@ +import React from 'react'; +import {render} from 'ink-testing-library'; +import {useInput} from 'ink'; +import {describe, it, expect, vi, beforeEach} from 'vitest'; +import SessionActions from './SessionActions.js'; + +const makeKey = ( + overrides: Record = {}, +): Record => ({ + upArrow: false, + downArrow: false, + leftArrow: false, + rightArrow: false, + pageDown: false, + pageUp: false, + home: false, + end: false, + return: false, + escape: false, + ctrl: false, + shift: false, + tab: false, + backspace: false, + delete: false, + meta: false, + ...overrides, +}); + +// Mock ink to avoid stdin issues and to capture the hotkey handler +vi.mock('ink', async () => { + const actual = await vi.importActual('ink'); + return { + ...actual, + useInput: vi.fn(), + }; +}); + +// Mock SelectInput to render items as simple text +vi.mock('ink-select-input', async () => { + const React = await vi.importActual('react'); + const {Text, Box} = await vi.importActual('ink'); + + return { + default: ({items}: {items: Array<{label: string; value: string}>}) => + React.createElement( + Box, + {flexDirection: 'column'}, + items.map((item: {label: string}, index: number) => + React.createElement(Text, {key: index}, item.label), + ), + ), + }; +}); + +const getLastInputHandler = () => { + const calls = vi.mocked(useInput).mock.calls; + const handler = calls[calls.length - 1]?.[0]; + expect(handler).toBeDefined(); + return handler!; +}; + +describe('SessionActions', () => { + beforeEach(() => { + vi.mocked(useInput).mockClear(); + }); + + it('should show session actions and the delete entry for a deletable worktree', () => { + const {lastFrame} = render( + , + ); + + const frame = lastFrame(); + expect(frame).toContain('Session Actions'); + expect(frame).toContain('New session in this worktree'); + expect(frame).toContain('Rename this session'); + expect(frame).toContain('Close session'); + expect(frame).toContain('Delete this worktree'); + }); + + it('should hide session-specific actions for a worktree without a session', () => { + const {lastFrame} = render( + , + ); + + const frame = lastFrame(); + expect(frame).toContain('Worktree Actions'); + expect(frame).toContain('New session in this worktree'); + expect(frame).toContain('Delete this worktree'); + expect(frame).not.toContain('Rename this session'); + expect(frame).not.toContain('Close session'); + }); + + it('should hide the delete entry when the worktree cannot be deleted', () => { + const {lastFrame} = render( + , + ); + + expect(lastFrame()).not.toContain('Delete this worktree'); + }); + + it('should dispatch deleteWorktree on the D hotkey when deletion is offered', () => { + const onSelect = vi.fn(); + render( + , + ); + + getLastInputHandler()('d', makeKey() as never); + + expect(onSelect).toHaveBeenCalledWith('deleteWorktree'); + }); + + it('should ignore hotkeys of actions that are not offered', () => { + const onSelect = vi.fn(); + render( + , + ); + + const handler = getLastInputHandler(); + handler('d', makeKey() as never); + expect(onSelect).not.toHaveBeenCalled(); + + handler('x', makeKey() as never); + expect(onSelect).toHaveBeenCalledWith('kill'); + }); + + it('should cancel on Escape', () => { + const onCancel = vi.fn(); + render( + , + ); + + getLastInputHandler()('', makeKey({escape: true}) as never); + + expect(onCancel).toHaveBeenCalled(); + }); +}); diff --git a/src/components/SessionActions.tsx b/src/components/SessionActions.tsx index 8496be7b..b64a4d0a 100644 --- a/src/components/SessionActions.tsx +++ b/src/components/SessionActions.tsx @@ -2,48 +2,76 @@ import React from 'react'; import {Box, Text, useInput} from 'ink'; import SelectInput from 'ink-select-input'; -export type SessionActionType = 'newSession' | 'rename' | 'kill'; +export type SessionActionType = + | 'newSession' + | 'rename' + | 'kill' + | 'deleteWorktree'; interface SessionActionsProps { sessionLabel: string; + /** + * Whether the row this menu was opened from has a running session. Session + * specific actions (rename, close) are hidden when it does not. + */ + hasSession?: boolean; + /** + * Whether the worktree of this row may be deleted; see isDeletableWorktree. + * The delete entry is hidden rather than shown-and-rejected so no + * unselectable option appears. + */ + canDeleteWorktree?: boolean; onSelect: (action: SessionActionType) => void; onCancel: () => void; } -const items: Array<{label: string; value: SessionActionType}> = [ - {label: 'S New session in same directory', value: 'newSession'}, - {label: 'R Rename this session', value: 'rename'}, - {label: 'X Close session', value: 'kill'}, -]; +const buildItems = ( + hasSession: boolean, + canDeleteWorktree: boolean, +): Array<{label: string; value: SessionActionType}> => { + const items: Array<{label: string; value: SessionActionType}> = [ + {label: 'S New session in this worktree', value: 'newSession'}, + ]; + + if (hasSession) { + items.push({label: 'R Rename this session', value: 'rename'}); + items.push({label: 'X Close session', value: 'kill'}); + } + + if (canDeleteWorktree) { + items.push({label: 'D Delete this worktree', value: 'deleteWorktree'}); + } + + return items; +}; const SessionActions: React.FC = ({ sessionLabel, + hasSession = true, + canDeleteWorktree = false, onSelect, onCancel, }) => { + const items = buildItems(hasSession, canDeleteWorktree); + useInput((input, key) => { if (key.escape) { onCancel(); return; } - switch (input.toLowerCase()) { - case 's': - onSelect('newSession'); - break; - case 'r': - onSelect('rename'); - break; - case 'x': - onSelect('kill'); - break; + const shortcut = items.find( + item => item.label[0]?.toLowerCase() === input.toLowerCase(), + ); + if (shortcut) { + onSelect(shortcut.value); } }); return ( - Session Actions + {hasSession ? 'Session Actions' : 'Worktree Actions'} {sessionLabel} @@ -52,7 +80,10 @@ const SessionActions: React.FC = ({ onSelect(item.value)} /> - S/R/X or arrow keys + Enter | Escape to cancel + + {items.map(item => item.label[0]).join('/')} or arrow keys + Enter | + Escape to cancel + ); diff --git a/src/types/index.ts b/src/types/index.ts index f1a7e5cb..87ba3f8b 100644 --- a/src/types/index.ts +++ b/src/types/index.ts @@ -85,9 +85,12 @@ export type MenuAction = | {type: 'renameSession'; session: Session} | {type: 'killSession'; sessionId: string} | { + // Row-level actions opened with Space. `session` is absent for a + // worktree row that has no session yet; the actions menu then offers + // only the worktree-level entries. type: 'sessionActions'; - session: Session; - worktreePath: string; + worktree: Worktree; + session?: Session; } | {type: 'deleteWorktree'} | {type: 'mergeWorktree'} diff --git a/src/utils/worktreeUtils.test.ts b/src/utils/worktreeUtils.test.ts index 06fc6e5d..265635a4 100644 --- a/src/utils/worktreeUtils.test.ts +++ b/src/utils/worktreeUtils.test.ts @@ -6,6 +6,7 @@ import { prepareSessionItems, calculateColumnPositions, assembleSessionLabel, + isDeletableWorktree, } from './worktreeUtils.js'; import {Worktree, Session} from '../types/index.js'; import {execSync} from 'child_process'; @@ -466,3 +467,52 @@ describe('column alignment', () => { expect(plain.indexOf('+10 -5')).toBe(21); // Should start at column 21 }); }); + +describe('isDeletableWorktree', () => { + it('should reject the main worktree', () => { + expect( + isDeletableWorktree( + {path: '/repo', isMainWorktree: true}, + '/somewhere/else', + ), + ).toBe(false); + }); + + it('should reject the worktree holding the current working directory', () => { + expect( + isDeletableWorktree( + {path: '/repo/worktrees/feature', isMainWorktree: false}, + '/repo/worktrees/feature', + ), + ).toBe(false); + }); + + it('should reject a worktree that is an ancestor of the current working directory', () => { + expect( + isDeletableWorktree( + {path: '/repo/worktrees/feature', isMainWorktree: false}, + '/repo/worktrees/feature/src/components', + ), + ).toBe(false); + }); + + it('should accept a sibling worktree with a shared path prefix', () => { + // '/repo/worktrees/feature-2' starts with the '/repo/worktrees/feature' + // string but is a different directory, so it stays deletable. + expect( + isDeletableWorktree( + {path: '/repo/worktrees/feature', isMainWorktree: false}, + '/repo/worktrees/feature-2', + ), + ).toBe(true); + }); + + it('should accept an unrelated linked worktree', () => { + expect( + isDeletableWorktree( + {path: '/repo/worktrees/feature', isMainWorktree: false}, + '/repo', + ), + ).toBe(true); + }); +}); diff --git a/src/utils/worktreeUtils.ts b/src/utils/worktreeUtils.ts index 180fc1fe..06647640 100644 --- a/src/utils/worktreeUtils.ts +++ b/src/utils/worktreeUtils.ts @@ -461,3 +461,33 @@ export function assembleSessionLabel( return label; } + +/** + * Whether a worktree may be deleted by CCManager. + * + * Two worktrees are off limits: + * - the main worktree, because git refuses to remove it and the repository + * would be left without a checkout; + * - the worktree that contains the current working directory, because removing + * the directory CCManager is running in breaks the running process. + * + * Single source of truth for the rule, shared by the multi-select delete screen + * and the per-row delete action. + */ +export function isDeletableWorktree( + worktree: Pick, + cwd: string = process.cwd(), +): boolean { + if (worktree.isMainWorktree) return false; + + const resolvedCwd = path.resolve(cwd); + const resolvedPath = path.resolve(worktree.path); + if ( + resolvedCwd === resolvedPath || + resolvedCwd.startsWith(resolvedPath + path.sep) + ) { + return false; + } + + return true; +}