fix(worktrees): keep failed orphan cleanup retryable (#22409)

Co-authored-by: SiinXu <SiinXu@users.noreply.github.com>

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-'))

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/<name>); prune or the stale entry keeps its branch locked.
         await gitExecFileAsync(['worktree', 'prune'], {
           cwd: repo.path,

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<void>
+): Promise<void> {
+  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)
+  }
+}

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')
+    })
+  })
 })

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<string> {
   }
   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))

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)

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<void>
-): Promise<void> {
-  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<typeof removeRuntimeRegisteredLocalWorktree>[0]
 ): Promise<void> {

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 }