From 64f5c1c195f461b4884e9b77d8f5e3e082dc81b9 Mon Sep 17 00:00:00 2001 From: Bo Zhang Date: Sat, 10 Oct 2026 08:06:33 +0800 Subject: [PATCH] Remove locked worktrees, including ones whose directory is gone Claude Code's worktree isolation locks the worktrees it creates in its scratchpad. Once that scratchpad is cleaned up, git keeps a locked entry whose directory is missing. git doesn't flag it prunable and `git worktree prune` skips it, so Ness showed it as a normal worktree and deleting it failed with "cannot remove a locked working tree". - Parse `locked [reason]` from `git worktree list --porcelain`. A locked entry whose directory is missing is reported as prunable, so it gets the Stale badge and no tabs are spawned against it. - pruneWorktrees unlocks missing-directory locked entries before pruning. Locked entries whose directory still exists are left alone. - removeWorktree passes --force twice when the directory is gone. - A locked worktree whose directory still exists asks for confirmation (and shows the lock reason) before it's force removed. The deletion FSM refuses to remove it without that override, and unlocks it before trashing so the prune after the rename doesn't skip it. --- src/main/index.ts | 8 +- src/main/worktree-deletion-fsm.test.ts | 70 +++++++++++++++- src/main/worktree-deletion-fsm.ts | 38 ++++++--- src/main/worktree-locked.integration.test.ts | 87 ++++++++++++++++++++ src/main/worktree.test.ts | 40 +++++++++ src/main/worktree.ts | 36 +++++++- src/renderer/build-backend.ts | 5 +- src/renderer/hooks/useWorktreeHandlers.ts | 13 ++- src/renderer/types.ts | 3 +- src/shared/state/worktrees.ts | 12 ++- 10 files changed, 290 insertions(+), 22 deletions(-) create mode 100644 src/main/worktree-locked.integration.test.ts diff --git a/src/main/index.ts b/src/main/index.ts index 6550db887..19b539080 100644 --- a/src/main/index.ts +++ b/src/main/index.ts @@ -1776,7 +1776,8 @@ function registerIpcHandlers(): void { repoRoot: string, path: string, force?: boolean, - removeMeta?: { prNumber?: number; prState?: PRState } + removeMeta?: { prNumber?: number; prState?: PRState }, + opts?: { overrideLock?: boolean } ) => { if (!repoRoot) throw new Error('No repo root provided') // Drop any locally-merged flag for the branch at this path. We still @@ -1807,7 +1808,10 @@ function registerIpcHandlers(): void { repoRoot, path, branch: wt?.branch || '', - force + force, + locked: wt?.locked, + lockedReason: wt?.lockedReason, + overrideLock: opts?.overrideLock }) return { queued: true } }) diff --git a/src/main/worktree-deletion-fsm.test.ts b/src/main/worktree-deletion-fsm.test.ts index 91e75eded..90da44521 100644 --- a/src/main/worktree-deletion-fsm.test.ts +++ b/src/main/worktree-deletion-fsm.test.ts @@ -1,4 +1,7 @@ import { describe, it, expect, vi, beforeEach } from 'vitest' +import { mkdtempSync, rmSync } from 'node:fs' +import { tmpdir } from 'node:os' +import { join } from 'node:path' vi.mock('./debug', () => ({ log: () => {} })) @@ -9,6 +12,7 @@ vi.mock('./repo-config', () => ({ vi.mock('./worktree', () => ({ removeWorktree: vi.fn(async () => {}), pruneWorktrees: vi.fn(async () => {}), + unlockWorktree: vi.fn(async () => {}), runWorktreeScript: vi.fn(async (_kind, _cmd, _ctx, onChunk) => { if (onChunk) onChunk('stdout', 'teardown-output') return { ok: true, exitCode: 0, stdout: '', stderr: '' } @@ -25,7 +29,7 @@ vi.mock('./worktree-trash', () => ({ import { Store } from './store' import { WorktreeDeletionFSM } from './worktree-deletion-fsm' -import { removeWorktree, pruneWorktrees, runWorktreeScript } from './worktree' +import { removeWorktree, pruneWorktrees, runWorktreeScript, unlockWorktree } from './worktree' import { deleteWorktreeDirectory, isSameVolume, @@ -182,3 +186,67 @@ describe('WorktreeDeletionFSM — failure surface', () => { expect(failed).toBeTruthy() }) }) + +describe('WorktreeDeletionFSM — locked worktrees', () => { + beforeEach(() => { + vi.clearAllMocks() + ;(loadRepoConfig as unknown as ReturnType).mockReturnValue({}) + ;(isSameVolume as unknown as ReturnType).mockReturnValue(true) + ;(moveWorktreeToTrash as unknown as ReturnType).mockResolvedValue( + '/trash/uuid' + ) + }) + + function withDir(fn: (dir: string) => Promise): Promise { + const dir = mkdtempSync(join(tmpdir(), 'wt-fsm-locked-')) + return fn(dir).finally(() => rmSync(dir, { recursive: true, force: true })) + } + + it('refuses a locked worktree whose directory exists without an override, before teardown', () => + withDir(async (dir) => { + ;(loadRepoConfig as unknown as ReturnType).mockReturnValue({ + teardownCommand: 'echo bye' + }) + const { fsm, events } = makeFsm() + + fsm.enqueue({ repoRoot: '/repo', path: dir, branch: 'x', locked: true, lockedReason: 'claude agent' }) + await new Promise((r) => setImmediate(r)) + + const failed = events.find( + (e) => e.type === 'worktrees/pendingDeletionUpdated' && e.payload.patch.phase === 'failed' + ) + expect(failed && failed.type === 'worktrees/pendingDeletionUpdated' && failed.payload.patch.error).toMatch( + /locked \(claude agent\)/ + ) + expect(runWorktreeScript).not.toHaveBeenCalled() + expect(unlockWorktree).not.toHaveBeenCalled() + expect(moveWorktreeToTrash).not.toHaveBeenCalled() + })) + + it('unlocks before trashing when the lock is overridden', () => + withDir(async (dir) => { + const { fsm, events } = makeFsm() + + fsm.enqueue({ repoRoot: '/repo', path: dir, branch: 'x', locked: true, overrideLock: true }) + await new Promise((r) => setImmediate(r)) + + expect(unlockWorktree).toHaveBeenCalledWith('/repo', dir) + const unlockOrder = (unlockWorktree as unknown as ReturnType).mock.invocationCallOrder[0] + const moveOrder = (moveWorktreeToTrash as unknown as ReturnType).mock.invocationCallOrder[0] + expect(unlockOrder).toBeLessThan(moveOrder) + expect(events.some((e) => e.type === 'worktrees/pendingDeletionRemoved')).toBe(true) + })) + + it('removes a locked worktree whose directory is already gone without an override', async () => { + ;(isSameVolume as unknown as ReturnType).mockReturnValue(false) + ;(removeWorktree as unknown as ReturnType).mockResolvedValue(undefined) + const { fsm, events } = makeFsm() + + fsm.enqueue({ repoRoot: '/repo', path: '/gone/wt92', branch: 'sec-9.2-fix', locked: true }) + await new Promise((r) => setImmediate(r)) + + expect(unlockWorktree).not.toHaveBeenCalled() + expect(removeWorktree).toHaveBeenCalledWith('/repo', '/gone/wt92', undefined) + expect(events.some((e) => e.type === 'worktrees/pendingDeletionRemoved')).toBe(true) + }) +}) diff --git a/src/main/worktree-deletion-fsm.ts b/src/main/worktree-deletion-fsm.ts index 3863b2bdf..bc8c0ae46 100644 --- a/src/main/worktree-deletion-fsm.ts +++ b/src/main/worktree-deletion-fsm.ts @@ -1,4 +1,5 @@ -import { pruneWorktrees, removeWorktree, runWorktreeScript } from './worktree' +import { existsSync } from 'node:fs' +import { pruneWorktrees, removeWorktree, runWorktreeScript, unlockWorktree } from './worktree' import { deleteWorktreeDirectory, isSameVolume, @@ -49,6 +50,9 @@ export class WorktreeDeletionFSM { path: string branch: string force?: boolean + locked?: boolean + lockedReason?: string + overrideLock?: boolean }): void { void this.run(params) } @@ -62,8 +66,11 @@ export class WorktreeDeletionFSM { path: string branch: string force?: boolean + locked?: boolean + lockedReason?: string + overrideLock?: boolean }): Promise { - const { repoRoot, path, branch, force } = params + const { repoRoot, path, branch, force, locked, lockedReason, overrideLock } = params const repoCfg = loadRepoConfig(repoRoot) const teardownCmd = repoCfg.teardownCommand || this.opts.getGlobalTeardownCmd() || '' const hasTeardown = Boolean(teardownCmd.trim()) @@ -78,6 +85,21 @@ export class WorktreeDeletionFSM { this.store.dispatch({ type: 'worktrees/pendingDeletionStarted', payload: initial }) try { + // A locked worktree whose directory still exists was locked on + // purpose (usually by another Claude session), so it takes an + // explicit override. Checked before teardown so a refusal doesn't + // half-dismantle it. A missing directory has nothing left to + // protect — removeWorktree/pruneWorktrees clear those on their own. + if (locked && existsSync(path)) { + if (!overrideLock) { + throw new Error( + `The worktree is locked${lockedReason ? ` (${lockedReason})` : ''}. ` + + 'Unlock it with `git worktree unlock`, or delete it again from the sidebar and confirm the force remove.' + ) + } + await unlockWorktree(repoRoot, path) + } + if (hasTeardown) { let buffered = '' const result = await runWorktreeScript( @@ -109,15 +131,9 @@ export class WorktreeDeletionFSM { // bookkeeping, background-unlink. `isSameVolume` returns false // if either stat throws (e.g. path is already gone) — the fast // path is skipped in that case, which is what we want. - // - // Note on locked worktrees: `git worktree remove` refuses to - // remove a locked entry without --force. `mv` doesn't care about - // git's lock, so the fast path is silently *more* forgiving — - // once the working dir is renamed out, `git worktree prune` - // happily reaps the bookkeeping. If we ever want to preserve - // the old "locked" guardrail we'd need to inspect git's lock - // files here; today the deletion FSM's callers already gate on - // dirty/locked in the UI so this widening is fine. + // Locked worktrees were unlocked above, which matters here too: + // `git worktree prune` skips locked entries, so renaming a locked + // dir out would leave a ghost entry behind. if (isSameVolume(path, worktreeTrashDir())) { const trashPath = await moveWorktreeToTrash(path) await pruneWorktrees(repoRoot) diff --git a/src/main/worktree-locked.integration.test.ts b/src/main/worktree-locked.integration.test.ts new file mode 100644 index 000000000..1934c565a --- /dev/null +++ b/src/main/worktree-locked.integration.test.ts @@ -0,0 +1,87 @@ +import { describe, it, expect, beforeEach, afterEach } from 'vitest' +import fs from 'fs' +import os from 'os' +import { execFileSync } from 'child_process' +import { join } from 'path' + +import { listWorktrees, pruneWorktrees, removeWorktree } from './worktree' + +// REAL git, no mocks. Claude Code locks the worktrees it creates for its +// own isolation, then cleans up the scratchpad they live in, which leaves +// locked entries whose directory is gone. + +function git(cwd: string, ...args: string[]): string { + return execFileSync('git', args, { cwd, stdio: 'pipe' }).toString() +} + +let tmp: string +let repo: string + +beforeEach(() => { + tmp = fs.realpathSync(fs.mkdtempSync(join(os.tmpdir(), 'wt-locked-'))) + repo = join(tmp, 'repo') + fs.mkdirSync(repo) + git(repo, 'init', '-q', '-b', 'main') + git(repo, 'config', 'user.email', 't@t.t') + git(repo, 'config', 'user.name', 'T') + git(repo, 'commit', '-q', '--allow-empty', '-m', 'init') +}) + +afterEach(() => { + fs.rmSync(tmp, { recursive: true, force: true }) +}) + +function makeLockedWorktree(name: string, reason?: string): string { + const path = join(tmp, name) + git(repo, 'worktree', 'add', '-q', path, '-b', name) + git(repo, 'worktree', 'lock', ...(reason ? ['--reason', reason] : []), path) + return path +} + +describe('locked worktrees', () => { + it('lists a locked, missing worktree as prunable with its lock reason', async () => { + const path = makeLockedWorktree('gone', 'claude agent') + fs.rmSync(path, { recursive: true, force: true }) + + const wt = (await listWorktrees(repo)).find((w) => w.path === path) + + expect(wt).toMatchObject({ locked: true, lockedReason: 'claude agent', prunable: true }) + }) + + it('lists a locked, present worktree as locked but not prunable', async () => { + const path = makeLockedWorktree('here') + + const wt = (await listWorktrees(repo)).find((w) => w.path === path) + + expect(wt?.locked).toBe(true) + expect(wt?.prunable).toBeUndefined() + }) + + it('removeWorktree clears a locked entry whose directory is gone', async () => { + const path = makeLockedWorktree('gone') + fs.rmSync(path, { recursive: true, force: true }) + + await removeWorktree(repo, path) + + expect((await listWorktrees(repo)).map((w) => w.path)).toEqual([repo]) + }) + + it('removeWorktree still refuses a locked worktree whose directory exists', async () => { + const path = makeLockedWorktree('here') + + await expect(removeWorktree(repo, path, true)).rejects.toThrow(/locked/) + expect(fs.existsSync(path)).toBe(true) + }) + + it('pruneWorktrees unlocks and prunes missing locked entries but keeps present ones locked', async () => { + const gone = makeLockedWorktree('gone') + const here = makeLockedWorktree('here') + fs.rmSync(gone, { recursive: true, force: true }) + + await pruneWorktrees(repo) + + const trees = await listWorktrees(repo) + expect(trees.map((w) => w.path)).toEqual([repo, here]) + expect(trees[1].locked).toBe(true) + }) +}) diff --git a/src/main/worktree.test.ts b/src/main/worktree.test.ts index d1d95d087..81a4cfd5d 100644 --- a/src/main/worktree.test.ts +++ b/src/main/worktree.test.ts @@ -259,3 +259,43 @@ describe('unsymlinkClaudeSettings', () => { } }) }) + +describe('parseWorktreeListPorcelain — locked entries', () => { + it('records the lock and its reason', () => { + const dir = mkdtempSync(join(tmpdir(), 'wt-parse-locked-')) + try { + const stdout = [ + `worktree ${dir}`, + 'HEAD 1234567', + 'branch refs/heads/agent', + 'locked claude agent', + '' + ].join('\n') + + const [wt] = parseWorktreeListPorcelain(stdout, '/Users/x/repo') + + expect(wt.locked).toBe(true) + expect(wt.lockedReason).toBe('claude agent') + expect(wt.prunable).toBeUndefined() + } finally { + rmSync(dir, { recursive: true, force: true }) + } + }) + + it('marks a locked entry with a missing directory prunable, since git does not', () => { + const stdout = [ + 'worktree /Users/x/does-not-exist/wt92', + 'HEAD 1234567', + 'branch refs/heads/sec-9.2-fix', + 'locked', + '' + ].join('\n') + + const [wt] = parseWorktreeListPorcelain(stdout, '/Users/x/repo') + + expect(wt.locked).toBe(true) + expect(wt.lockedReason).toBeUndefined() + expect(wt.prunable).toBe(true) + expect(wt.prunableReason).toMatch(/locked/) + }) +}) diff --git a/src/main/worktree.ts b/src/main/worktree.ts index b0d852135..47aadcb27 100644 --- a/src/main/worktree.ts +++ b/src/main/worktree.ts @@ -106,6 +106,13 @@ export function parseWorktreeListPorcelain( let current: Partial = {} const flush = (): void => { if (!current.path) return + // `git worktree list` never marks a locked entry prunable, and `git + // worktree prune` skips it, so a locked worktree whose directory was + // deleted out from under it would otherwise look healthy forever. + if (current.locked && !current.prunable && !existsSync(current.path)) { + current.prunable = true + current.prunableReason = 'locked, and its directory no longer exists' + } worktrees.push({ path: current.path, branch: current.branch || '(detached)', @@ -115,7 +122,9 @@ export function parseWorktreeListPorcelain( createdAt: getCreatedAt(current.path), repoRoot, ...(current.prunable ? { prunable: true } : {}), - ...(current.prunableReason ? { prunableReason: current.prunableReason } : {}) + ...(current.prunableReason ? { prunableReason: current.prunableReason } : {}), + ...(current.locked ? { locked: true } : {}), + ...(current.lockedReason ? { lockedReason: current.lockedReason } : {}) }) current = {} } @@ -136,6 +145,10 @@ export function parseWorktreeListPorcelain( current.prunable = true const rest = line.slice('prunable'.length).trim() if (rest) current.prunableReason = rest + } else if (line === 'locked' || line.startsWith('locked ')) { + current.locked = true + const rest = line.slice('locked'.length).trim() + if (rest) current.lockedReason = rest } else if (line === '') { flush() } @@ -2008,14 +2021,31 @@ export function readClaudeAllowEntries(worktreePath: string): string[] { export async function removeWorktree(repoRoot: string, path: string, force?: boolean): Promise { log('worktree', `removing worktree: path=${path} force=${force}`) const args = ['worktree', 'remove', path] - if (force) args.push('--force') + // Git needs --force twice to remove a locked worktree. When the directory + // is already gone there's nothing left to lose, so override any lock + // rather than failing on an entry nobody can use anymore. + if (!existsSync(path)) args.push('--force', '--force') + else if (force) args.push('--force') await execFileAsync('git', args, { cwd: repoRoot }) } +export async function unlockWorktree(repoRoot: string, path: string): Promise { + log('worktree', `unlocking worktree: path=${path}`) + await execFileAsync('git', ['worktree', 'unlock', path], { cwd: repoRoot }) +} + /** Drop `.git/worktrees/` entries whose on-disk directory no * longer exists. Same as running `git worktree prune` by hand — this - * is the recovery action wired to the "Stale" badge in the sidebar. */ + * is the recovery action wired to the "Stale" badge in the sidebar. + * `git worktree prune` skips locked entries, so missing-directory ones + * are unlocked first. Locked entries whose directory still exists are + * left alone: someone locked those on purpose. */ export async function pruneWorktrees(repoRoot: string): Promise { log('worktree', `pruning stale worktrees at repoRoot=${repoRoot}`) + const trees = await listWorktrees(repoRoot) + for (const wt of trees) { + if (!wt.locked || wt.isMain || existsSync(wt.path)) continue + await unlockWorktree(repoRoot, wt.path) + } await execFileAsync('git', ['worktree', 'prune'], { cwd: repoRoot }) } diff --git a/src/renderer/build-backend.ts b/src/renderer/build-backend.ts index 065e8c6e0..bdd742af4 100644 --- a/src/renderer/build-backend.ts +++ b/src/renderer/build-backend.ts @@ -141,8 +141,9 @@ export function buildBackend( repoRoot: string, path: string, force?: boolean, - removeMeta?: { prNumber?: number; prState?: 'open' | 'draft' | 'merged' | 'closed' } - ) => req('worktree:remove', repoRoot, path, force, removeMeta), + removeMeta?: { prNumber?: number; prState?: 'open' | 'draft' | 'merged' | 'closed' }, + opts?: { overrideLock?: boolean } + ) => req('worktree:remove', repoRoot, path, force, removeMeta, opts), dismissPendingDeletion: (path: string) => req('worktree:dismissPendingDeletion', path), pruneWorktrees: (repoRoot: string) => req('worktrees:prune', repoRoot), getWorktreeDir: (repoRoot: string) => req('worktree:dir', repoRoot), diff --git a/src/renderer/hooks/useWorktreeHandlers.ts b/src/renderer/hooks/useWorktreeHandlers.ts index d3e53b1a6..8171a9f10 100644 --- a/src/renderer/hooks/useWorktreeHandlers.ts +++ b/src/renderer/hooks/useWorktreeHandlers.ts @@ -303,6 +303,16 @@ export function useWorktreeHandlers(args: UseWorktreeHandlersArgs) { const handleDeleteWorktree = useCallback( async (path: string) => { + const target = worktrees.find((w) => w.path === path) + const overrideLock = Boolean(target?.locked && !target.prunable) + if (overrideLock) { + const reason = target?.lockedReason ? `\n\nLock reason: ${target.lockedReason}` : '' + const confirmed = window.confirm( + `This worktree is locked (git worktree lock), usually because another Claude session is using it.${reason}\n\nForce remove it anyway?` + ) + if (!confirmed) return + } + // Check for dirty changes — git tracked + per-worktree scratchpad // notes are reported separately so the dialog can name exactly // what's about to be lost. @@ -344,7 +354,8 @@ export function useWorktreeHandlers(args: UseWorktreeHandlersArgs) { repoRoot, path, dirty.git, - pr ? { prNumber: pr.number, prState: pr.state } : undefined + pr ? { prNumber: pr.number, prState: pr.state } : undefined, + overrideLock ? { overrideLock } : undefined ) if (path === activeWorktreeId) { const next = worktrees.find((w) => w.path !== path) diff --git a/src/renderer/types.ts b/src/renderer/types.ts index 98cefe2f5..ac75f0761 100644 --- a/src/renderer/types.ts +++ b/src/renderer/types.ts @@ -264,7 +264,8 @@ export interface ElectronAPI { repoRoot: string, path: string, force?: boolean, - removeMeta?: { prNumber?: number; prState?: PRStatus['state'] } + removeMeta?: { prNumber?: number; prState?: PRStatus['state'] }, + opts?: { overrideLock?: boolean } ): Promise<{ queued: true }> dismissPendingDeletion(path: string): Promise pruneWorktrees(repoRoot: string): Promise diff --git a/src/shared/state/worktrees.ts b/src/shared/state/worktrees.ts index 04bbcc81b..4a0871500 100644 --- a/src/shared/state/worktrees.ts +++ b/src/shared/state/worktrees.ts @@ -21,6 +21,14 @@ export interface Worktree { * points to non-existent location"). Optional — shown as a tooltip * next to the stale badge. */ prunableReason?: string + /** True when git has the entry locked (`git worktree lock`). Claude + * Code's own worktree isolation locks the worktrees it creates in its + * scratchpad, so a locked entry usually means another session owns it. + * A locked entry whose directory is gone is also reported as + * `prunable`, since git itself won't flag it. */ + locked?: boolean + /** Reason passed to `git worktree lock --reason`, if any. */ + lockedReason?: string } /** Handle for a worktree in agent-facing text: `/`. Branch @@ -84,7 +92,9 @@ export function worktreeListsEqual(a: Worktree[], b: Worktree[]): boolean { x.createdAt !== y.createdAt || x.repoRoot !== y.repoRoot || x.prunable !== y.prunable || - x.prunableReason !== y.prunableReason + x.prunableReason !== y.prunableReason || + x.locked !== y.locked || + x.lockedReason !== y.lockedReason ) { return false }