From f04d768dd4c3b60fe6bdd82328083518edda1a6b Mon Sep 17 00:00:00 2001 From: Griffen Fargo <3642037+gfargo@users.noreply.github.com> Date: Sun, 3 May 2026 23:47:54 -0400 Subject: [PATCH] feat(log-tui): D on the worktrees view removes worktree + branch (#838) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two delete actions in the worktrees view, with W keeping its existing semantics so muscle memory carries: - W (existing): remove worktree only. Leaves the branch in place for the user to re-attach later or delete separately. - D (new): remove worktree AND delete the branch it was tracking. Chains git worktree remove → git branch -d, with both pre-flight guards inherited from the underlying helpers (refuses current/dirty worktrees; uses safe -d so unmerged branches don't lose work). Per-view scoping in inkInput intercepts D on the worktrees surface before the global workflow-by-key dispatcher would otherwise route it to delete-branch (which would target whatever was last cursored on the branches view rather than acting on the worktree under the cursor here). D from elsewhere keeps doing what it always did. The chained helper handles partial failures with named-step messages so the user knows exactly which step broke if anything does. Detached worktrees and worktrees whose tracked branch isn't in the local ref list both skip the branch step cleanly with a "removed worktree (no branch to delete)" success. Closes #838. Co-Authored-By: Claude Opus 4.7 (1M context) --- src/commands/log/inkInput.test.ts | 64 ++++++++++++++++ src/commands/log/inkInput.ts | 11 +++ src/commands/log/inkRuntime.ts | 28 ++++++- src/commands/log/inkWorkflows.ts | 13 ++++ src/commands/log/worktreeActions.test.ts | 94 ++++++++++++++++++++++++ src/commands/log/worktreeActions.ts | 64 +++++++++++++++- 6 files changed, 272 insertions(+), 2 deletions(-) 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}`, + } +}