From dc8d3e459b17d2d33c322e3b4f91379f190c2a18 Mon Sep 17 00:00:00 2001 From: Jinwoo-H Date: Thu, 9 Jul 2026 15:08:41 -0400 Subject: [PATCH] Handle locked worktree force deletion Co-authored-by: Orca --- src/main/git/remove-worktree.test.ts | 10 ++--- src/main/git/worktree-list-paths.test.ts | 42 +++++++++++++++++++ src/main/git/worktree.ts | 4 +- src/relay/git-handler-worktree-ops.test.ts | 4 ++ src/relay/git-handler-worktree-remove.ts | 4 +- .../src/store/slices/store-cascades.test.ts | 27 ++++++++++++ src/renderer/src/store/slices/worktrees.ts | 1 + 7 files changed, 85 insertions(+), 7 deletions(-) diff --git a/src/main/git/remove-worktree.test.ts b/src/main/git/remove-worktree.test.ts index f2f356a8734..e2838c6734e 100644 --- a/src/main/git/remove-worktree.test.ts +++ b/src/main/git/remove-worktree.test.ts @@ -247,7 +247,7 @@ branch refs/heads/main ]) }) - it('passes --force before the worktree path when forced removal is requested', async () => { + it('passes double --force before the worktree path when forced removal is requested', async () => { mockGitCommands({ 'git worktree list --porcelain': { stdout: `worktree /repo @@ -269,7 +269,7 @@ branch refs/heads/main await removeWorktree('/repo', '/repo-feature', true) - expect(getGitCalls()).toContain('git worktree remove --force /repo-feature') + expect(getGitCalls()).toContain('git worktree remove --force --force /repo-feature') }) it('matches Windows worktree paths before deleting the branch', async () => { @@ -1070,7 +1070,7 @@ branch refs/heads/main 'git sparse-checkout init --cone', 'git sparse-checkout set -- packages/web', 'git config --local --unset-all branch.feature/test.base', - 'git worktree remove --force /repo-feature', + 'git worktree remove --force --force /repo-feature', 'git branch -D -- feature/test' ]) ) @@ -1078,11 +1078,11 @@ branch refs/heads/main expectGitCallOrder( calls, 'git sparse-checkout set -- packages/web', - 'git worktree remove --force /repo-feature' + 'git worktree remove --force --force /repo-feature' ) expectGitCallOrder( calls, - 'git worktree remove --force /repo-feature', + 'git worktree remove --force --force /repo-feature', 'git branch -D -- feature/test' ) }) diff --git a/src/main/git/worktree-list-paths.test.ts b/src/main/git/worktree-list-paths.test.ts index dc792712077..cf375e26015 100644 --- a/src/main/git/worktree-list-paths.test.ts +++ b/src/main/git/worktree-list-paths.test.ts @@ -33,6 +33,39 @@ async function createRepoWithNewlineWorktree(): Promise<{ } } +async function createRepoWithLockedDeletedWorktree(): Promise<{ + repoPath: string + worktreePath: string +}> { + const root = await mkdtemp(path.join(tmpdir(), 'orca-worktree-locked-delete-')) + tempRoots.push(root) + const repoPath = path.join(root, 'repo') + const requestedWorktreePath = path.join(root, 'locked deleted worktree') + + execFileSync('git', ['init', '--quiet', repoPath]) + git(repoPath, ['symbolic-ref', 'HEAD', 'refs/heads/main']) + git(repoPath, ['config', 'user.email', 'test@example.com']) + git(repoPath, ['config', 'user.name', 'Test User']) + git(repoPath, ['commit', '--allow-empty', '--quiet', '-m', 'initial']) + git(repoPath, [ + 'worktree', + 'add', + '--quiet', + '-b', + 'feature/locked-delete', + requestedWorktreePath + ]) + git(repoPath, ['worktree', 'lock', '--reason', 'orca locked stale repro', requestedWorktreePath]) + + const worktreePath = await realpath(requestedWorktreePath) + await rm(worktreePath, { recursive: true, force: true }) + + return { + repoPath: await realpath(repoPath), + worktreePath + } +} + function branchExists(repoPath: string, branchName: string): boolean { try { git(repoPath, ['show-ref', '--verify', '--quiet', `refs/heads/${branchName}`]) @@ -68,4 +101,13 @@ describe('git worktree paths', () => { expect(branchExists(repoPath, 'feature/newline')).toBe(false) } ) + + it('force-removes a locked worktree whose directory was deleted manually', async () => { + const { repoPath, worktreePath } = await createRepoWithLockedDeletedWorktree() + + await expect(removeWorktree(repoPath, worktreePath, true)).resolves.toEqual({}) + + expect(git(repoPath, ['worktree', 'list', '--porcelain'])).not.toContain(worktreePath) + expect(branchExists(repoPath, 'feature/locked-delete')).toBe(false) + }) }) diff --git a/src/main/git/worktree.ts b/src/main/git/worktree.ts index eb30e13ad0a..f4965eda76b 100644 --- a/src/main/git/worktree.ts +++ b/src/main/git/worktree.ts @@ -1076,7 +1076,9 @@ async function performRemoveWorktree( const args = ['worktree', 'remove'] if (force) { - args.push('--force') + // Why: Git requires force twice to remove a locked worktree; Orca's + // force path already passed the explicit destructive confirmation gate. + args.push('--force', '--force') } args.push(worktreePath) await gitExecFileAsync(args, gitExecOptions(repoPath, options)) diff --git a/src/relay/git-handler-worktree-ops.test.ts b/src/relay/git-handler-worktree-ops.test.ts index 52cc1236054..f04c2b0adba 100644 --- a/src/relay/git-handler-worktree-ops.test.ts +++ b/src/relay/git-handler-worktree-ops.test.ts @@ -224,6 +224,10 @@ describe('removeWorktreeOp', () => { forceBranchDelete: true }) + expect(git).toHaveBeenCalledWith( + ['worktree', 'remove', '--force', '--force', '/repo-feature'], + expect.any(String) + ) expect(git).toHaveBeenCalledWith(['branch', '-D', '--', 'feature/test'], expect.any(String)) expect(git).not.toHaveBeenCalledWith(['branch', '-d', '--', 'feature/test'], expect.any(String)) }) diff --git a/src/relay/git-handler-worktree-remove.ts b/src/relay/git-handler-worktree-remove.ts index edd3932ef3b..f45571cf565 100644 --- a/src/relay/git-handler-worktree-remove.ts +++ b/src/relay/git-handler-worktree-remove.ts @@ -175,7 +175,9 @@ export async function removeWorktreeOp( const args = ['worktree', 'remove'] if (force) { - args.push('--force') + // Why: mirror local deletion semantics; Git requires force twice to + // override worktree locks after the user confirmed destructive removal. + args.push('--force', '--force') } args.push(worktreePath) await git(args, repoPath) diff --git a/src/renderer/src/store/slices/store-cascades.test.ts b/src/renderer/src/store/slices/store-cascades.test.ts index dd4b531fbd8..03e24b639f7 100644 --- a/src/renderer/src/store/slices/store-cascades.test.ts +++ b/src/renderer/src/store/slices/store-cascades.test.ts @@ -324,6 +324,33 @@ describe('removeWorktree cascade', () => { }) }) + it('offers force delete for locked worktree removal errors', async () => { + const store = createTestStore() + const worktreeId = 'repo1::/workspace/feature-wt' + const error = + "fatal: cannot remove a locked working tree, lock reason: claude session\nuse 'remove -f -f' to override or unlock first" + + mockApi.worktrees.remove.mockRejectedValueOnce(new Error(error)) + + seedStore(store, { + worktreesByRepo: { + repo1: [makeWorktree({ id: worktreeId, repoId: 'repo1' })] + }, + tabsByWorktree: {}, + ptyIdsByTabId: {}, + terminalLayoutsByTabId: {} + }) + + const result = await store.getState().removeWorktree(worktreeId) + + expect(result).toEqual({ ok: false, error }) + expect(store.getState().deleteStateByWorktreeId[worktreeId]).toEqual({ + isDeleting: false, + error, + canForceDelete: true + }) + }) + it('offers force delete when Git already removed an unregistered worktree', async () => { const store = createTestStore() const worktreeId = 'repo1::/workspace/deleted-wt' diff --git a/src/renderer/src/store/slices/worktrees.ts b/src/renderer/src/store/slices/worktrees.ts index 36d70b5d319..2d148be68bd 100644 --- a/src/renderer/src/store/slices/worktrees.ts +++ b/src/renderer/src/store/slices/worktrees.ts @@ -293,6 +293,7 @@ function toVisibleTabType(contentType: string): WorkspaceVisibleTabType { const FORCE_RETRYABLE_WORKTREE_REMOVAL_MESSAGES = [ 'Worktree has uncommitted or untracked changes', 'contains modified or untracked files', + 'cannot remove a locked working tree', 'Worktree is no longer registered with Git but its directory remains', 'Worktree is no longer registered with Git and its directory is already gone' ] as const