diff --git a/src/commands/log/inkInput.test.ts b/src/commands/log/inkInput.test.ts index 49706c4..3f83dea 100644 --- a/src/commands/log/inkInput.test.ts +++ b/src/commands/log/inkInput.test.ts @@ -2314,4 +2314,68 @@ describe('log Ink input interactions', () => { ]) }) }) + + // #838 — `D` on the worktrees view fires the chained worktree + // removal + branch delete instead of the global delete-branch + // workflow. Scoped per-view so `D` from elsewhere keeps doing what + // it always did. + describe('worktree D-for-delete-with-branch', () => { + function worktreesViewState(overrides: Partial = {}) { + const base = createLogInkState(rows) + return { + ...base, + focus: 'commits' as const, + activeView: 'worktrees' as const, + viewStack: ['worktrees'] as LogInkState['viewStack'], + ...overrides, + } + } + + it('D on the worktrees view fires the remove-worktree-and-branch confirm', () => { + const events = getLogInkInputEvents( + worktreesViewState(), + 'D', + {}, + { worktreeListCount: 2 }, + ) + expect(events).toEqual([ + { type: 'action', action: { type: 'setPendingConfirmation', value: 'remove-worktree-and-branch' } }, + ]) + }) + + it('D from anywhere else still fires the global delete-branch workflow', () => { + // History view: no per-view interception; global workflow-by-key + // path takes over and routes to delete-branch. + const events = getLogInkInputEvents(createLogInkState(rows), 'D', {}) + expect(events).toEqual([ + { type: 'action', action: { type: 'setPendingConfirmation', value: 'delete-branch' } }, + ]) + }) + + it('W on the worktrees view still fires plain remove-worktree (existing behavior)', () => { + const events = getLogInkInputEvents( + worktreesViewState(), + 'W', + {}, + { worktreeListCount: 2 }, + ) + expect(events).toEqual([ + { type: 'action', action: { type: 'setPendingConfirmation', value: 'remove-worktree' } }, + ]) + }) + + it('D on the worktrees view with no worktrees falls through (no interception)', () => { + const events = getLogInkInputEvents( + worktreesViewState(), + 'D', + {}, + { worktreeListCount: 0 }, + ) + // worktreeListCount=0 means the per-view guard doesn't fire; the + // global workflow-by-key path then claims D for delete-branch. + expect(events).toEqual([ + { type: 'action', action: { type: 'setPendingConfirmation', value: 'delete-branch' } }, + ]) + }) + }) }) diff --git a/src/commands/log/inkInput.ts b/src/commands/log/inkInput.ts index 40aa57d..12776d1 100644 --- a/src/commands/log/inkInput.ts +++ b/src/commands/log/inkInput.ts @@ -1663,6 +1663,17 @@ export function getLogInkInputEvents( return [action({ type: 'setPendingConfirmation', value: 'delete-remote-tag' })] } + // Per-view worktree action: `D` removes the worktree AND deletes + // the branch it was tracking (#838). Scoped to the worktrees + // surface so it intercepts BEFORE the global workflow-by-key + // dispatcher would otherwise route `D` to delete-branch (which + // would silently target whatever was last cursored on the branches + // surface instead of acting on the worktree under the cursor here). + // `W` keeps its existing "remove worktree only" semantics. + if (inputValue === 'D' && isWorktreeActionTarget(state) && context.worktreeListCount) { + return [action({ type: 'setPendingConfirmation', value: 'remove-worktree-and-branch' })] + } + // #783 — full PR action panel keys, scoped to the pull-request view. // All five wrap a `gh pr ` invocation; merge / request-changes / // comment open prompts first, the rest route through the y-confirm diff --git a/src/commands/log/inkRuntime.ts b/src/commands/log/inkRuntime.ts index 48caaf3..e699f45 100644 --- a/src/commands/log/inkRuntime.ts +++ b/src/commands/log/inkRuntime.ts @@ -176,7 +176,7 @@ import { } from './historyActions' import { applyStash, checkoutFileFromStash, createStash, dropStash, popStash } from './stashActions' import { ApplyHunkTarget, applyHunkPatch } from './hunkActions' -import { removeWorktree } from './worktreeActions' +import { removeWorktree, removeWorktreeAndBranch } from './worktreeActions' import { abortOperation } from './operationActions' import { PullRequestOverview, getPullRequestOverview } from './pullRequestData' import { @@ -1863,6 +1863,32 @@ function LogInkApp(deps: LogInkComponentDeps): ReactTypes.ReactElement { } return removeWorktree(git, cursorTarget) }, + 'remove-worktree-and-branch': async () => { + const all = context.worktreeList?.worktrees || [] + const visible = state.filter + ? all.filter((w) => matchesPromotedFilter([w.path, w.branch || ''], state.filter)) + : all + const cursorTarget = visible.length + ? visible[Math.min(state.selectedWorktreeListIndex, visible.length - 1)] + : all[Math.min(state.selectedWorktreeListIndex, Math.max(0, all.length - 1))] + if (!cursorTarget) return { ok: false, message: 'No worktree selected' } + if (cursorTarget.current) { + return { + ok: false, + message: 'Cannot remove the current worktree — switch to another worktree first.', + } + } + // The chained helper handles the worktree removal AND the + // safe branch delete in one call. Branch refs come from the + // live context so the underlying deleteBranch helper sees + // the current/local flags it needs to refuse the destructive + // path on the wrong target. + return removeWorktreeAndBranch( + git, + cursorTarget, + context.branches?.localBranches || [] + ) + }, 'abort-operation': async () => { const operation = context.operation?.operation if (!operation) { diff --git a/src/commands/log/inkWorkflows.ts b/src/commands/log/inkWorkflows.ts index 53f3d71..1df7ce9 100644 --- a/src/commands/log/inkWorkflows.ts +++ b/src/commands/log/inkWorkflows.ts @@ -313,6 +313,19 @@ export function getLogInkWorkflowActions(): LogInkWorkflowAction[] { kind: 'destructive', requiresConfirmation: true, }, + { + // Per-view-only — the inkInput handler scopes this to the + // worktrees surface so the global `D` keystroke (delete-branch) + // keeps working from elsewhere. The empty `key` keeps the + // workflow palette-discoverable but does not register a global + // hotkey that would collide with delete-branch. + id: 'remove-worktree-and-branch', + key: '', + label: 'Remove worktree + delete branch', + description: 'Remove the selected worktree and delete the branch it was tracking after confirmation.', + kind: 'destructive', + requiresConfirmation: true, + }, { id: 'abort-operation', key: 'A', diff --git a/src/commands/log/worktreeActions.test.ts b/src/commands/log/worktreeActions.test.ts index 93b7bae..c90da29 100644 --- a/src/commands/log/worktreeActions.test.ts +++ b/src/commands/log/worktreeActions.test.ts @@ -1,7 +1,9 @@ +import { BranchRef } from './branchData' import { createBranchWorktree, createWorktree, removeWorktree, + removeWorktreeAndBranch, worktreePathAction, } from './worktreeActions' import { WorktreeEntry } from './worktreeData' @@ -67,4 +69,96 @@ describe('log worktree actions', () => { }) expect(git.raw).not.toHaveBeenCalled() }) + + // #838 — `D` on a worktree chains the worktree removal AND the + // branch delete in one action so users don't have to remove the + // worktree, navigate to the branches view, then delete the branch + // separately. + describe('removeWorktreeAndBranch', () => { + const branchRef: BranchRef = { + type: 'local', + name: 'refs/heads/feature/log', + shortName: 'feature/log', + hash: 'abc123', + current: false, + date: '2026-05-03', + subject: 'feat: log', + ahead: 0, + behind: 0, + } + + it('removes the worktree then deletes the matching branch', async () => { + const git = { raw: jest.fn().mockResolvedValue('') } + + await expect( + removeWorktreeAndBranch(git as never, worktree, [branchRef]) + ).resolves.toEqual({ + ok: true, + message: 'Removed worktree /repo-feature and deleted branch feature/log', + }) + + expect(git.raw).toHaveBeenNthCalledWith(1, ['worktree', 'remove', '/repo-feature']) + expect(git.raw).toHaveBeenNthCalledWith(2, ['branch', '-d', 'feature/log']) + }) + + it('aborts before the branch delete when the worktree removal fails', async () => { + const git = { raw: jest.fn() } + + await expect( + removeWorktreeAndBranch(git as never, { ...worktree, dirty: true }, [branchRef]) + ).resolves.toEqual({ + ok: false, + message: 'Cannot remove dirty worktree /repo-feature. Clean or stash it first.', + }) + // Worktree pre-flight rejected; git.raw was never called for + // either step. + expect(git.raw).not.toHaveBeenCalled() + }) + + it('reports a partial failure when the branch delete fails after a successful worktree removal', async () => { + const git = { + raw: jest.fn() + .mockResolvedValueOnce('') // worktree remove ok + .mockRejectedValueOnce(new Error('branch feature/log not fully merged')), + } + + await expect( + removeWorktreeAndBranch(git as never, worktree, [branchRef]) + ).resolves.toEqual({ + ok: false, + message: 'Removed worktree /repo-feature, but branch delete failed: branch feature/log not fully merged', + }) + }) + + it('skips the branch delete when the worktree had no branch (detached HEAD)', async () => { + const git = { raw: jest.fn().mockResolvedValue('') } + + await expect( + removeWorktreeAndBranch( + git as never, + { ...worktree, branch: undefined, detached: true }, + [branchRef] + ) + ).resolves.toEqual({ + ok: true, + message: 'Removed worktree /repo-feature (no branch to delete)', + }) + + expect(git.raw).toHaveBeenCalledTimes(1) + expect(git.raw).toHaveBeenCalledWith(['worktree', 'remove', '/repo-feature']) + }) + + it('skips the branch delete when the named branch is not in the local ref list', async () => { + const git = { raw: jest.fn().mockResolvedValue('') } + + await expect( + removeWorktreeAndBranch(git as never, worktree, []) + ).resolves.toEqual({ + ok: true, + message: 'Removed worktree /repo-feature (branch feature/log not found in local branches)', + }) + + expect(git.raw).toHaveBeenCalledTimes(1) + }) + }) }) diff --git a/src/commands/log/worktreeActions.ts b/src/commands/log/worktreeActions.ts index 528270e..6f1a706 100644 --- a/src/commands/log/worktreeActions.ts +++ b/src/commands/log/worktreeActions.ts @@ -1,5 +1,6 @@ import { SimpleGit } from 'simple-git' -import { BranchActionResult } from './branchActions' +import { BranchActionResult, deleteBranch } from './branchActions' +import { BranchRef } from './branchData' import { WorktreeEntry } from './worktreeData' async function runAction(action: () => Promise, successMessage: string): Promise { @@ -87,3 +88,64 @@ export function worktreePathAction(worktree: WorktreeEntry): BranchActionResult message: `Worktree path: ${worktree.path}`, } } + +/** + * Remove a worktree AND delete the branch it was tracking (#838). The + * canonical "I'm done with this side branch" wind-down: removes the + * worktree directory, then runs `git branch -d` on the previously + * checked-out branch. + * + * Both pre-flight guards inherit from the underlying helpers: + * - removeWorktree refuses the current worktree and dirty worktrees + * - deleteBranch refuses the current branch and uses `-d` (safe + * delete, refuses unmerged commits) + * + * Aborts cleanly at any failure point and surfaces a message that + * names which step broke. When the worktree had no branch (detached + * HEAD) the branch step is silently skipped — there's nothing to + * delete and the worktree removal alone counts as success. + */ +export async function removeWorktreeAndBranch( + git: SimpleGit, + worktree: WorktreeEntry, + branchRefs: BranchRef[] +): Promise { + const removeResult = await removeWorktree(git, worktree) + if (!removeResult.ok) { + return removeResult + } + + const branchName = worktree.branch + if (!branchName) { + return { + ok: true, + message: `Removed worktree ${worktree.path} (no branch to delete)`, + } + } + + // Look up the local BranchRef for the branch this worktree was on. + // deleteBranch needs the full ref (not just the name) so its + // current-branch and local-only guards apply correctly. + const branch = branchRefs.find((entry) => + entry.type === 'local' && entry.shortName === branchName + ) + if (!branch) { + return { + ok: true, + message: `Removed worktree ${worktree.path} (branch ${branchName} not found in local branches)`, + } + } + + const deleteResult = await deleteBranch(git, branch) + if (!deleteResult.ok) { + return { + ok: false, + message: `Removed worktree ${worktree.path}, but branch delete failed: ${deleteResult.message}`, + } + } + + return { + ok: true, + message: `Removed worktree ${worktree.path} and deleted branch ${branchName}`, + } +}