diff --git a/src/main/ipc/worktrees-removal-recovery.test.ts b/src/main/ipc/worktrees-removal-recovery.test.ts index cfe962cc09a..fc372ddc81f 100644 --- a/src/main/ipc/worktrees-removal-recovery.test.ts +++ b/src/main/ipc/worktrees-removal-recovery.test.ts @@ -279,6 +279,58 @@ describe('registerWorktreeHandlers', () => { }) }) + it.each(['unproven', 'removal-fails'] as const)( + 'keeps desktop orphan cleanup retryable when the directory is %s', + async (mode) => { + const parentDir = await mkdtemp(join(tmpdir(), 'orca-ipc-orphan-retention-')) + const repoPath = join(parentDir, 'repo') + const orphanPath = join(parentDir, 'orphan') + const worktreeId = `repo-1::${orphanPath}` + await mkdir(orphanPath, { recursive: true }) + if (mode === 'removal-fails') { + const adminPath = join(repoPath, '.git', 'worktrees', 'orphan') + await mkdir(adminPath, { recursive: true }) + await writeFile(join(orphanPath, '.git'), `gitdir: ${adminPath}\n`) + await writeFile(join(adminPath, 'gitdir'), `${join(orphanPath, '.git')}\n`) + } + const repo = { id: 'repo-1', path: repoPath, displayName: 'repo', badgeColor: '', addedAt: 0 } + store.getRepos.mockReturnValue([repo]) + store.getRepo.mockReturnValue(repo) + mockKnownFeatureWorktree(orphanPath, repoPath) + getEffectiveHooksMock.mockReturnValue(null) + removeWorktreeMock.mockRejectedValue( + Object.assign(new Error('Git remove failed'), { + stderr: `fatal: '${orphanPath}' is not a working tree` + }) + ) + const finish = vi.fn().mockResolvedValue(undefined) + runtimeStub.acquireFileWatcherRemoval.mockResolvedValue({ finish }) + const removePath = vi + .spyOn(localWorktreeFilesystem, 'removeLocalWorktreePath') + .mockRejectedValue(new Error('injected removal failure')) + try { + await expect(handlers['worktrees:remove'](null, { worktreeId })).rejects.toThrow( + 'Worktree is no longer registered with Git but its directory remains.' + ) + await expect(lstat(orphanPath)).resolves.toBeTruthy() + expect(store.removeWorktreeMeta).not.toHaveBeenCalled() + expect(gitExecFileAsyncMock).not.toHaveBeenCalledWith( + ['worktree', 'prune'], + expect.anything() + ) + expect(finish).toHaveBeenCalledWith(false) + expect(removePath).toHaveBeenCalledTimes(mode === 'removal-fails' ? 1 : 0) + await rm(orphanPath, { recursive: true, force: true }) + await expect(handlers['worktrees:remove'](null, { worktreeId })).resolves.toEqual({}) + expect(store.removeWorktreeMeta).toHaveBeenCalledWith(worktreeId, 'local') + expect(finish).toHaveBeenLastCalledWith(true) + } finally { + removePath.mockRestore() + await rm(parentDir, { recursive: true, force: true }) + } + } + ) + it('recovers forced Windows long-path worktree removal through local deletion and prune', async () => { setPlatform('win32') const parentDir = await mkdtemp(join(tmpdir(), 'orca-ipc-long-path-')) diff --git a/src/main/ipc/worktrees/removal/remove-registered-local-worktree.ts b/src/main/ipc/worktrees/removal/remove-registered-local-worktree.ts index c22e6b0ad96..5a36199c8e3 100644 --- a/src/main/ipc/worktrees/removal/remove-registered-local-worktree.ts +++ b/src/main/ipc/worktrees/removal/remove-registered-local-worktree.ts @@ -11,17 +11,10 @@ import { } from '../../../git/worktree' import { gitExecFileAsync } from '../../../git/runner' import { getWorktreeSharedLinkPaths } from '../../../git/worktree-shared-directories' -import { - getLocalWorktreePathAccess, - removeLocalWorktreePath, - toLocalWorktreeRuntimePath -} from '../../../local-worktree-filesystem' +import { cleanupLocalOrphanedWorktreeDirectory } from '../../../local-orphaned-worktree-cleanup' import { recoverLocalWindowsWorktreeRemoval } from '../../../local-worktree-removal-recovery' import { withWorktreeRemoveStageSpan } from '../../../observability/instrumentation' -import { - canSafelyRemoveOrphanedWorktreeDirectory, - findRegisteredDeletableWorktree -} from '../../../worktree-removal-safety' +import { findRegisteredDeletableWorktree } from '../../../worktree-removal-safety' import { CLIENT_REMOVAL_HOME } from '../../../worktree-removal-home-guard' import { cleanupUnusedWorktreePushTargetRemote, @@ -162,25 +155,12 @@ export async function removeRegisteredLocalWorktree( console.warn( `[worktrees] Orphaned worktree detected at ${canonicalWorktreePath}, cleaning up` ) - const access = getLocalWorktreePathAccess(localWorktreeGitOptions) - if ( - await canSafelyRemoveOrphanedWorktreeDirectory( - toLocalWorktreeRuntimePath(canonicalWorktreePath, localWorktreeGitOptions), - toLocalWorktreeRuntimePath(repo.path, localWorktreeGitOptions), - CLIENT_REMOVAL_HOME, - access.statPath, - access.readPath - ) - ) { - await runtime.closeFileWatchersForRemoval(canonicalWorktreePath) - await removeLocalWorktreePath(canonicalWorktreePath, localWorktreeGitOptions).catch( - () => {} - ) - } else { - console.warn( - `[worktrees] Refusing recursive cleanup for unproven worktree directory: ${canonicalWorktreePath}` - ) - } + await cleanupLocalOrphanedWorktreeDirectory( + repo.path, + canonicalWorktreePath, + localWorktreeGitOptions, + (path) => runtime.closeFileWatchersForRemoval(path) + ) // Why: remove failed so git still tracks it (.git/worktrees/); prune or the stale entry keeps its branch locked. await gitExecFileAsync(['worktree', 'prune'], { cwd: repo.path, diff --git a/src/main/local-orphaned-worktree-cleanup.ts b/src/main/local-orphaned-worktree-cleanup.ts new file mode 100644 index 00000000000..9ed666e5021 --- /dev/null +++ b/src/main/local-orphaned-worktree-cleanup.ts @@ -0,0 +1,39 @@ +import type { LocalWorktreeFilesystemOptions } from './local-worktree-filesystem' +import { + getLocalWorktreePathAccess, + removeLocalWorktreePath, + toLocalWorktreeRuntimePath +} from './local-worktree-filesystem' +import { + canSafelyRemoveOrphanedWorktreeDirectory, + isWorktreePathMissing, + ORPHANED_WORKTREE_DIRECTORY_MESSAGE +} from './worktree-removal-safety' +import { CLIENT_REMOVAL_HOME } from './worktree-removal-home-guard' + +export async function cleanupLocalOrphanedWorktreeDirectory( + repoPath: string, + path: string, + options: LocalWorktreeFilesystemOptions, + closeWatchers: (path: string) => Promise +): Promise { + const access = getLocalWorktreePathAccess(options) + const runtimePath = toLocalWorktreeRuntimePath(path, options) + if ( + await canSafelyRemoveOrphanedWorktreeDirectory( + runtimePath, + toLocalWorktreeRuntimePath(repoPath, options), + CLIENT_REMOVAL_HOME, + access.statPath, + access.readPath + ) + ) { + await closeWatchers(path) + await removeLocalWorktreePath(path, options).catch(() => {}) + } else { + console.warn(`[worktrees] Refusing recursive cleanup for unproven worktree directory: ${path}`) + } + if (!(await isWorktreePathMissing(runtimePath, access.statPath))) { + throw new Error(ORPHANED_WORKTREE_DIRECTORY_MESSAGE) + } +} diff --git a/src/main/local-worktree-filesystem.test.ts b/src/main/local-worktree-filesystem.test.ts index c9e97f72e90..4bb4f182bdd 100644 --- a/src/main/local-worktree-filesystem.test.ts +++ b/src/main/local-worktree-filesystem.test.ts @@ -30,12 +30,12 @@ function completeExecFile(stdout = ''): void { runProcessMock.mockResolvedValue({ code: 0, signal: null, stdout, stderr: '', timedOut: false }) } -function failExecFile(exitCode: number): void { +function failExecFile(exitCode: number, stderr = 'missing'): void { runProcessMock.mockResolvedValue({ code: exitCode, signal: null, stdout: '', - stderr: 'missing', + stderr, timedOut: false }) } @@ -171,7 +171,7 @@ describe('local worktree filesystem runtime access', () => { it('uses the selected WSL distro for stat, read, and removal on Windows', async () => { await withPlatform('win32', async () => { - completeExecFile('file') + completeExecFile('regular file') const access = getLocalWorktreePathAccess({ wslDistro: 'Ubuntu' }) await expect(access.statPath('/home/me/repo/.git')).resolves.toEqual({ type: 'file' }) @@ -220,7 +220,7 @@ describe('local worktree filesystem runtime access', () => { it('reports missing WSL stat targets with an ENOENT-shaped error', async () => { await withPlatform('win32', async () => { - failExecFile(2) + failExecFile(1, "stat: cannot statx '/mnt/c/repo/missing/.git': No such file or directory\n") const access = getLocalWorktreePathAccess({ wslDistro: 'Ubuntu' }) await expect(access.statPath('/mnt/c/repo/missing/.git')).rejects.toMatchObject({ @@ -228,4 +228,13 @@ describe('local worktree filesystem runtime access', () => { }) }) }) + + it('does not treat an unreadable WSL stat target as missing', async () => { + await withPlatform('win32', async () => { + failExecFile(1, "stat: cannot statx '/mnt/c/repo/locked': Permission denied\n") + const access = getLocalWorktreePathAccess({ wslDistro: 'Ubuntu' }) + + await expect(access.statPath('/mnt/c/repo/locked')).rejects.toThrow('Permission denied') + }) + }) }) diff --git a/src/main/local-worktree-filesystem.ts b/src/main/local-worktree-filesystem.ts index 1078b1f50bc..23861c71a87 100644 --- a/src/main/local-worktree-filesystem.ts +++ b/src/main/local-worktree-filesystem.ts @@ -18,8 +18,6 @@ type LocalWorktreePathAccess = { } const WSL_FILE_OPERATION_TIMEOUT_MS = 30_000 -/** The stat probe's explicit "missing path" branch. */ -const WSL_MISSING_PATH_EXIT_CODE = 2 function shouldUseWslFilesystem(options: LocalWorktreeFilesystemOptions): boolean { return process.platform === 'win32' && !!options.wslDistro?.trim() @@ -48,17 +46,23 @@ async function runWslCommand(distro: string, command: string): Promise { } if (result.code !== 0) { throw Object.assign(new Error(result.stderr.trim() || `wsl.exe exited ${result.code}`), { - exitCode: result.code + exitCode: result.code, + stderr: result.stderr }) } return result.stdout } function isWslMissingPathError(error: unknown): boolean { + if (typeof error !== 'object' || error === null || !('exitCode' in error)) { + return false + } + const stderr = 'stderr' in error && typeof error.stderr === 'string' ? error.stderr : '' return ( - typeof error === 'object' && - error !== null && - (error as { exitCode?: unknown }).exitCode === WSL_MISSING_PATH_EXIT_CODE + error.exitCode === 1 && + /^stat: cannot stat(?:x)? [\s\S]+: (?:No such file or directory|Not a directory)\r?\n?$/.test( + stderr + ) ) } @@ -83,19 +87,26 @@ export function getLocalWorktreePathAccess( return { statPath: async (path) => { const target = quotePosixShell(toLinuxPath(path)) - const stdout = await runWslCommand( - distro, - [ - `target=${target}`, - 'if [ -L "$target" ]; then printf symlink; elif [ -f "$target" ]; then printf file; elif [ -d "$target" ]; then printf directory; else exit 2; fi' - ].join('\n') - ).catch((error) => { - if (isWslMissingPathError(error)) { - throw Object.assign(new Error(`missing ${path}`), { code: 'ENOENT' }) + // Shell file tests conflate permission failures with absence; stat preserves the reason. + const stdout = await runWslCommand(distro, `LC_ALL=C stat -c %F -- ${target}`).catch( + (error: unknown) => { + if (isWslMissingPathError(error)) { + throw Object.assign(new Error(`missing ${path}`), { code: 'ENOENT' }) + } + throw error } - throw error - }) - return { type: stdout.trim() } + ) + const kind = stdout.trim() + return { + type: + kind === 'symbolic link' + ? 'symlink' + : kind === 'regular file' || kind === 'regular empty file' + ? 'file' + : kind === 'directory' + ? 'directory' + : 'other' + } }, readPath: async (path) => { const target = quotePosixShell(toLinuxPath(path)) diff --git a/src/main/runtime/orca-runtime-tests/worktree-removal-and-reconciliation-part-04.spec.ts b/src/main/runtime/orca-runtime-tests/worktree-removal-and-reconciliation-part-04.spec.ts index 78c4f072bb3..10cf9b4fbb9 100644 --- a/src/main/runtime/orca-runtime-tests/worktree-removal-and-reconciliation-part-04.spec.ts +++ b/src/main/runtime/orca-runtime-tests/worktree-removal-and-reconciliation-part-04.spec.ts @@ -14,14 +14,18 @@ import { listWorktrees, listWorktreesStrict, lstat, + localWorktreeFilesystem, + mkdir, mkdtemp, + restoreLocalWatcherAfterFailedRemovalMock, registerSshGitProvider, removeWorktree, removeWorktreeLinkedPathsMock, rm, runHook, tmpdir, - unregisterSshGitProvider + unregisterSshGitProvider, + writeFile } from '../orca-runtime-test-mocks.spec' import type { WorktreeMeta } from '../orca-runtime-test-mocks.spec' import { @@ -344,6 +348,158 @@ describe('OrcaRuntimeService', () => { expect(deleteWorktreeHistoryDirMock).toHaveBeenCalledWith(TEST_WORKTREE_ID) }) + it('retains metadata when an unproven orphan directory survives cleanup', async () => { + const parentDir = await mkdtemp(join(tmpdir(), 'orca-runtime-orphan-retention-')) + const repoPath = join(parentDir, 'repo') + const orphanPath = join(parentDir, 'orphan') + const worktreeId = `${TEST_REPO_ID}::${orphanPath}` + await mkdir(orphanPath, { recursive: true }) + const { runtimeStore, removeWorktreeMeta } = createStaleRuntimeWorktreeStore(worktreeId) + const runtimeStoreWithRepoPath = { + ...runtimeStore, + getRepos: () => [ + { + id: TEST_REPO_ID, + path: repoPath, + displayName: 'repo', + badgeColor: 'blue', + addedAt: 1 + } + ], + getRepo: (id: string) => + id === TEST_REPO_ID + ? { + id: TEST_REPO_ID, + path: repoPath, + displayName: 'repo', + badgeColor: 'blue', + addedAt: 1 + } + : undefined + } + const runtime = createWorktreeRemovalRuntime(runtimeStoreWithRepoPath) + const registeredWorktree = { + path: orphanPath, + head: 'abc', + branch: 'feature/orphan', + isBare: false, + isMainWorktree: false + } + vi.mocked(listWorktrees).mockResolvedValue([registeredWorktree]) + vi.mocked(listWorktreesStrict).mockResolvedValue([registeredWorktree]) + vi.mocked(removeWorktree).mockRejectedValue( + Object.assign(new Error('git worktree remove failed'), { + stderr: `fatal: '${orphanPath}' is not a working tree` + }) + ) + vi.mocked(assertWorktreeCleanForRemoval).mockRejectedValue( + Object.assign(new Error('status failed'), { + stderr: 'fatal: not a git repository (or any of the parent directories): .git\n' + }) + ) + const gitSpy = vi.spyOn(gitRunner, 'gitExecFileAsync').mockResolvedValue({ + stdout: '', + stderr: '' + }) + const pruneCallsBefore = gitSpy.mock.calls.filter(([args]) => args[0] === 'worktree').length + + try { + await expect(runtime.removeManagedWorktree(worktreeId)).rejects.toThrow( + 'Worktree is no longer registered with Git but its directory remains.' + ) + await expect(lstat(orphanPath)).resolves.toBeTruthy() + expect(removeWorktreeMeta).not.toHaveBeenCalled() + expect(restoreLocalWatcherAfterFailedRemovalMock).toHaveBeenCalledWith(orphanPath) + expect(gitSpy.mock.calls.filter(([args]) => args[0] === 'worktree')).toHaveLength( + pruneCallsBefore + ) + + await rm(orphanPath, { recursive: true, force: true }) + vi.mocked(listWorktrees).mockResolvedValue([]) + await expect(runtime.removeManagedWorktree(worktreeId)).resolves.toEqual({}) + expect(removeWorktreeMeta).toHaveBeenCalledWith(worktreeId, 'local') + } finally { + gitSpy.mockRestore() + await rm(parentDir, { recursive: true, force: true }) + } + }) + + it('retains metadata when proven orphan cleanup cannot remove the directory', async () => { + const parentDir = await mkdtemp(join(tmpdir(), 'orca-runtime-orphan-removal-failure-')) + const repoPath = join(parentDir, 'repo') + const orphanPath = join(parentDir, 'orphan') + const adminWorktreePath = join(repoPath, '.git', 'worktrees', 'orphan') + const worktreeId = `${TEST_REPO_ID}::${orphanPath}` + await mkdir(orphanPath, { recursive: true }) + await mkdir(adminWorktreePath, { recursive: true }) + await writeFile(join(orphanPath, '.git'), `gitdir: ${adminWorktreePath}\n`) + await writeFile(join(adminWorktreePath, 'gitdir'), `${join(orphanPath, '.git')}\n`) + const { runtimeStore, removeWorktreeMeta } = createStaleRuntimeWorktreeStore(worktreeId) + const runtimeStoreWithRepoPath = { + ...runtimeStore, + getRepos: () => [ + { + id: TEST_REPO_ID, + path: repoPath, + displayName: 'repo', + badgeColor: 'blue', + addedAt: 1 + } + ], + getRepo: (id: string) => + id === TEST_REPO_ID + ? { + id: TEST_REPO_ID, + path: repoPath, + displayName: 'repo', + badgeColor: 'blue', + addedAt: 1 + } + : undefined + } + const runtime = createWorktreeRemovalRuntime(runtimeStoreWithRepoPath) + const registeredWorktree = { + path: orphanPath, + head: 'abc', + branch: 'feature/orphan', + isBare: false, + isMainWorktree: false + } + vi.mocked(listWorktrees).mockResolvedValue([registeredWorktree]) + vi.mocked(listWorktreesStrict).mockResolvedValue([registeredWorktree]) + vi.mocked(removeWorktree).mockRejectedValue( + Object.assign(new Error('git worktree remove failed'), { + stderr: `fatal: '${orphanPath}' is not a working tree` + }) + ) + vi.mocked(assertWorktreeCleanForRemoval).mockRejectedValue( + Object.assign(new Error('status failed'), { + stderr: 'fatal: not a git repository (or any of the parent directories): .git\n' + }) + ) + const removePathSpy = vi + .spyOn(localWorktreeFilesystem, 'removeLocalWorktreePath') + .mockRejectedValue(new Error('injected removal failure')) + const gitSpy = vi.spyOn(gitRunner, 'gitExecFileAsync').mockResolvedValue({ + stdout: '', + stderr: '' + }) + + try { + await expect(runtime.removeManagedWorktree(worktreeId)).rejects.toThrow( + 'Worktree is no longer registered with Git but its directory remains.' + ) + await expect(lstat(orphanPath)).resolves.toBeTruthy() + expect(removePathSpy).toHaveBeenCalledWith(orphanPath, {}) + expect(removeWorktreeMeta).not.toHaveBeenCalled() + expect(gitSpy).not.toHaveBeenCalledWith(['worktree', 'prune'], expect.anything()) + } finally { + removePathSpy.mockRestore() + gitSpy.mockRestore() + await rm(parentDir, { recursive: true, force: true }) + } + }) + it('drops the bounded scan cache when orphan-cleanup removal completes', async () => { const runtime = createWorktreeRemovalRuntime() vi.mocked(getEffectiveHooks).mockReturnValue(null) diff --git a/src/main/runtime/runtime-registered-local-worktree-removal.ts b/src/main/runtime/runtime-registered-local-worktree-removal.ts index e6658b73ef2..011d30207fe 100644 --- a/src/main/runtime/runtime-registered-local-worktree-removal.ts +++ b/src/main/runtime/runtime-registered-local-worktree-removal.ts @@ -20,16 +20,9 @@ import { isOrphanCompatiblePreflightError, isOrphanedWorktreeError } from '../ipc/worktree-logic' -import { - getLocalWorktreePathAccess, - removeLocalWorktreePath, - toLocalWorktreeRuntimePath -} from '../local-worktree-filesystem' +import { cleanupLocalOrphanedWorktreeDirectory } from '../local-orphaned-worktree-cleanup' import { recoverLocalWindowsWorktreeRemoval } from '../local-worktree-removal-recovery' -import { - canSafelyRemoveOrphanedWorktreeDirectory, - findRegisteredDeletableWorktree -} from '../worktree-removal-safety' +import { findRegisteredDeletableWorktree } from '../worktree-removal-safety' import { CLIENT_REMOVAL_HOME } from '../worktree-removal-home-guard' import type { RuntimeStore } from './runtime-store-contract' import type { RuntimeWorktreeRemovalTarget } from './runtime-worktree-selection' @@ -160,7 +153,12 @@ export async function removeRuntimeRegisteredLocalWorktree(args: { removalResult = recovered completed = true } else if (isOrphanedWorktreeError(error)) { - await cleanupOrphanedDirectory(repo, canonicalPath, localOptions, args.closeWatchers) + await cleanupLocalOrphanedWorktreeDirectory( + repo.path, + canonicalPath, + localOptions, + args.closeWatchers + ) await gitExecFileAsync(['worktree', 'prune'], { cwd: repo.path, ...localOptions }).catch( () => {} ) @@ -190,29 +188,6 @@ export async function removeRuntimeRegisteredLocalWorktree(args: { } } -async function cleanupOrphanedDirectory( - repo: Repo, - path: string, - options: LocalProjectWorktreeGitOptions, - closeWatchers: (path: string) => Promise -): Promise { - const access = getLocalWorktreePathAccess(options) - if ( - await canSafelyRemoveOrphanedWorktreeDirectory( - toLocalWorktreeRuntimePath(path, options), - toLocalWorktreeRuntimePath(repo.path, options), - CLIENT_REMOVAL_HOME, - access.statPath, - access.readPath - ) - ) { - await closeWatchers(path) - await removeLocalWorktreePath(path, options).catch(() => {}) - } else { - console.warn(`[worktrees] Refusing recursive cleanup for unproven worktree directory: ${path}`) - } -} - async function cleanupPushTarget( args: Parameters[0] ): Promise { diff --git a/src/main/worktree-removal-safety.test.ts b/src/main/worktree-removal-safety.test.ts index 9aa9775de44..d32289a5fc7 100644 --- a/src/main/worktree-removal-safety.test.ts +++ b/src/main/worktree-removal-safety.test.ts @@ -6,7 +6,8 @@ import { canSafelyRemoveOrphanedWorktreeDirectory, findRegisteredDeletableWorktree, getRegisteredDeletableWorktree, - isDangerousWorktreeRemovalPath + isDangerousWorktreeRemovalPath, + isWorktreePathMissing } from './worktree-removal-safety' import { CLIENT_REMOVAL_HOME, executionHostRemovalHome } from './worktree-removal-home-guard' @@ -349,6 +350,16 @@ describe('canSafelyRemoveOrphanedWorktreeDirectory', () => { }) }) +describe('isWorktreePathMissing', () => { + it('does not treat an unreadable path as absent', async () => { + await expect( + isWorktreePathMissing('/workspaces/locked', async () => { + throw Object.assign(new Error('permission denied'), { code: 'EACCES' }) + }) + ).resolves.toBe(false) + }) +}) + describe('canCleanupUnregisteredOrcaLeftoverDirectory', () => { const repo = { path: '/repos/main' } const ownedMeta = { orcaCreatedAt: 1, orcaCreationSource: 'runtime' as const }