diff --git a/src/main/index.ts b/src/main/index.ts index 6550db88..19b53908 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 91e75ede..90da4452 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 3863b2bd..bc8c0ae4 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 00000000..1934c565 --- /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 d1d95d08..81a4cfd5 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 b0d85213..47aadcb2 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 065e8c6e..bdd742af 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 d3e53b1a..8171a9f1 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 98cefe2f..ac75f076 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 04bbcc81..4a087150 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 }