fix(worktrees): a worktree delete git fails partway stays listed and can be retried

src/main/git/worktree-add.ts

@@ -21,7 +21,7 @@ import type {
 } from './worktree-operation-options'
 import { gitExecOptions, resolveWorktreeAddTimeoutMs } from './worktree-operation-options'
 import { bumpWorktreeScanGeneration } from './worktree-scan-cache'
-import { assertNoPendingWorktreeRemovalConflict } from '../worktree-background-removal'
+import { assertNoPendingWorktreeRemovalConflict } from '../worktree-removal-table'
 
 export type WorktreeAddBaseContext = Pick<AddWorktreeResult, 'localBaseRefUpdateSuggestion'> & {
   effectiveBase: string

src/main/git/worktree-removal.ts

@@ -128,15 +128,21 @@ function deleteBranchOfRemovedWorktree(
 /**
  * Finishes a removal whose checkout Git no longer registers (it finished deleting, or an earlier
  * run did): leftover files, stale admin records, then the branch. Already-gone parts are done.
+ * `assertLeftover` refuses unless the path still holds the removed checkout's own leftover.
  */
 export async function finishUnregisteredWorktreeRemoval(
   repoPath: string,
   worktreePath: string,
   branch: { name: string; head: string } | null,
+  assertLeftover: () => Promise<void>,
   options: RemoveWorktreeOptions = {}
 ): Promise<RemoveWorktreeResult> {
   try {
-    await runUnderWorktreeDeleteLimit(() => removeCheckoutLeftByGit(worktreePath, options))
+    await runUnderWorktreeDeleteLimit(async () => {
+      // Why in the slot: the wait can outlast two large deletes, and the path may change meanwhile.
+      await assertLeftover()
+      await removeCheckoutLeftByGit(worktreePath, options)
+    })
     await gitExecFileAsync(['worktree', 'prune'], gitExecOptions(repoPath, options)).catch(
       (error: unknown) => console.warn(`[git] worktree prune failed in ${repoPath}`, error)
     )

src/main/ipc/worktree-metadata-merge.ts

@@ -50,6 +50,7 @@ export function mergeWorktree(
     isBare: git.isBare,
     ...(git.isSparse === true ? { isSparse: true } : {}),
     isMainWorktree: git.isMainWorktree,
+    ...(git.removalError ? { removalError: git.removalError } : {}),
     // Automatic labels follow the live branch; persisted values are only authoritative when pinned.
     displayName:
       meta?.displayNameIsPinned === false

src/main/ipc/worktree-remote.ts

@@ -169,7 +169,7 @@ import {
 import { createRetiredNameLookup } from '../../shared/worktree/retired-name-registry'
 import { toLocalBaseRefRefreshResult } from '../../shared/worktree/local-base-branch-fast-forward'
 import { isSshRequestOutcomeUnverifiable } from '../ssh/ssh-channel-multiplexer'
-import { findPendingWorktreeRemovalConflict } from '../worktree-background-removal'
+import { findPendingWorktreeRemovalConflict } from '../worktree-removal-table'
 
 const SSH_WORKTREE_CREATE_FETCH_FRESHNESS_MS = 30_000
 const SSH_WORKTREE_CREATE_FETCH_CACHE_MAX = 512

src/main/ipc/worktrees-failed-removal.test.ts

@@ -0,0 +1,252 @@
+// Desktop IPC for a delete that failed after Git dropped the registration: the leftover stays in
+// `worktrees:list` with the error, Delete retries it. Git and the disk are mocked;
+// runtime-failed-local-worktree-removal.test.ts runs the real thing.
+import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
+import {
+  killAllProcessesForWorktreeMock,
+  listWorktreesMock,
+  removeWorktreeMock
+} from './worktrees-test-module-mocks'
+import { handlers, setupWorktreeHandlers, store } from './worktrees-test-harness'
+import { mockKnownFeatureWorktree } from './worktrees-test-fixtures'
+import type { RemoveWorktreeResult } from '../../shared/worktree/create-types'
+import type { Worktree } from '../../shared/worktree/types'
+import { finishUnregisteredWorktreeRemoval } from '../git/worktree-removal'
+import type * as WorktreeRemovalModule from '../git/worktree-removal'
+import type * as WorktreeRemovalTable from '../worktree-removal-table'
+import type * as WorktreeRemovalLeftover from '../worktree-removal-leftover'
+import {
+  _resetPendingWorktreeRemovalsForTests,
+  _settlePendingWorktreeRemovalsForTests,
+  retryFailedWorktreeRemoval,
+  startBackgroundWorktreeRemoval
+} from '../worktree-background-removal'
+
+vi.mock('electron', async () =>
+  (await import('./worktrees-test-module-mocks')).electronModuleMock()
+)
+vi.mock('../git/worktree', async () =>
+  (await import('./worktrees-test-module-mocks')).gitWorktreeModuleMock()
+)
+vi.mock('../git/runner', async () =>
+  (await import('./worktrees-test-module-mocks')).gitRunnerModuleMock()
+)
+vi.mock('../git/repo', async () =>
+  (await import('./worktrees-test-module-mocks')).gitRepoModuleMock()
+)
+vi.mock('../git/git-username', async (importOriginal) => ({
+  ...(await importOriginal<Record<string, unknown>>()),
+  resolveLocalGitUsername: (await import('./worktrees-test-module-mocks'))
+    .resolveLocalGitUsernameMock
+}))
+vi.mock('../github/client', async () =>
+  (await import('./worktrees-test-module-mocks')).githubClientModuleMock()
+)
+vi.mock('../source-control/hosted-review', async () =>
+  (await import('./worktrees-test-module-mocks')).hostedReviewModuleMock()
+)
+vi.mock('../providers/ssh-git-dispatch', async () =>
+  (await import('./worktrees-test-module-mocks')).sshGitDispatchModuleMock()
+)
+vi.mock('../providers/ssh-filesystem-dispatch', async () =>
+  (await import('./worktrees-test-module-mocks')).sshFilesystemDispatchModuleMock()
+)
+vi.mock('./worktree-symlinks', async () =>
+  (await import('./worktrees-test-module-mocks')).worktreeSymlinksModuleMock()
+)
+vi.mock('./ssh', async () => (await import('./worktrees-test-module-mocks')).sshModuleMock())
+vi.mock('../ssh/ssh-target-registry', async () =>
+  (await import('./worktrees-test-module-mocks')).sshTargetRegistryModuleMock()
+)
+vi.mock('../hooks', async () => (await import('./worktrees-test-module-mocks')).hooksModuleMock())
+vi.mock('../setup-runner-script-text', async (importOriginal) =>
+  (await import('./worktrees-test-module-mocks')).setupRunnerScriptTextModuleMock(
+    await importOriginal<Record<string, unknown>>()
+  )
+)
+vi.mock('../worktree-runner-script', async (importOriginal) =>
+  (await import('./worktrees-test-module-mocks')).worktreeRunnerScriptModuleMock(
+    await importOriginal<Record<string, unknown>>()
+  )
+)
+vi.mock('../effective-hook-config', async (importOriginal) =>
+  (await import('./worktrees-test-module-mocks')).effectiveHookConfigModuleMock(
+    await importOriginal<Record<string, unknown>>()
+  )
+)
+vi.mock('../setup-hook-env-vars', async (importOriginal) =>
+  (await import('./worktrees-test-module-mocks')).setupHookEnvVarsModuleMock(
+    await importOriginal<Record<string, unknown>>()
+  )
+)
+vi.mock('./worktree-logic', async (importOriginal) =>
+  (await import('./worktrees-test-module-mocks')).worktreeLogicModuleMock(
+    await importOriginal<Record<string, unknown>>()
+  )
+)
+vi.mock('../terminal-history-deletion', async () =>
+  (await import('./worktrees-test-module-mocks')).terminalHistoryDeletionModuleMock()
+)
+vi.mock('../ports/advertised-url-watcher', async () =>
+  (await import('./worktrees-test-module-mocks')).advertisedUrlWatcherModuleMock()
+)
+vi.mock('../workspace-cleanup-scan-snapshot', async () =>
+  (await import('./worktrees-test-module-mocks')).workspaceCleanupScanSnapshotModuleMock()
+)
+vi.mock('../workspace-space-analysis-snapshot', async () =>
+  (await import('./worktrees-test-module-mocks')).workspaceSpaceAnalysisSnapshotModuleMock()
+)
+vi.mock('../workspace-cleanup-removal-snapshot-prune', async () =>
+  (await import('./worktrees-test-module-mocks')).workspaceCleanupRemovalSnapshotPruneModuleMock()
+)
+vi.mock('../runtime/worktree-teardown', async () =>
+  (await import('./worktrees-test-module-mocks')).worktreeTeardownModuleMock()
+)
+vi.mock('./pty', async () => (await import('./worktrees-test-module-mocks')).ptyModuleMock())
+
+vi.mock('../git/worktree-removal', async (importOriginal) => ({
+  ...(await importOriginal<typeof WorktreeRemovalModule>()),
+  finishUnregisteredWorktreeRemoval: vi.fn(async () => ({}))
+}))
+// The leftover is on disk and is the removed checkout's own (no `.git` left).
+vi.mock('../worktree-removal-table', async (importOriginal) => ({
+  ...(await importOriginal<typeof WorktreeRemovalTable>()),
+  worktreeCheckoutExists: vi.fn(async () => true)
+}))
+vi.mock('../worktree-removal-leftover', async (importOriginal) => ({
+  ...(await importOriginal<typeof WorktreeRemovalLeftover>()),
+  isUnregisteredRemovalLeftover: vi.fn(async () => true)
+}))
+
+const featureId = 'repo-1::/workspace/feature-wt'
+const GIT_ERROR = "error: failed to delete '/workspace/feature-wt': Operation not permitted"
+
+function remove(args: Record<string, unknown>): Promise<RemoveWorktreeResult> {
+  // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: worktrees:remove resolves a RemoveWorktreeResult.
+  return handlers['worktrees:remove'](null, args) as Promise<RemoveWorktreeResult>
+}
+
+async function listFeature(): Promise<Worktree | undefined> {
+  // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: worktrees:list resolves the repo's Worktree rows.
+  const rows = (await handlers['worktrees:list'](null, { repoId: 'repo-1' })) as Worktree[]
+  return rows.find((row) => row.id === featureId)
+}
+
+/** Git fails partway and drops the registration, as `git worktree remove --force` does. */
+async function failAfterGitDroppedIt(): Promise<void> {
+  const [main, feature] = mockKnownFeatureWorktree()
+  const result = startBackgroundWorktreeRemoval({
+    removal: {
+      worktreeId: featureId,
+      repoId: 'repo-1',
+      repoPath: '/workspace/repo',
+      worktree: feature,
+      deleteBranch: true,
+      force: false
+    },
+    run: async () => {
+      listWorktreesMock.mockResolvedValue([main])
+      throw new Error(GIT_ERROR)
+    },
+    publish: () => {}
+  })
+  await expect(result).rejects.toThrow(GIT_ERROR)
+  await _settlePendingWorktreeRemovalsForTests()
+}
+
+describe('a failed delete Git no longer registers, over desktop IPC', () => {
+  beforeEach(() => {
+    vi.spyOn(console, 'warn').mockImplementation(() => {})
+    setupWorktreeHandlers()
+  })
+
+  afterEach(() => {
+    _resetPendingWorktreeRemovalsForTests()
+    vi.mocked(finishUnregisteredWorktreeRemoval).mockClear()
+  })
+
+  it('stays in the listing with the error instead of vanishing', async () => {
+    await failAfterGitDroppedIt()
+
+    const row = await listFeature()
+    expect(row).toMatchObject({ path: '/workspace/feature-wt', removalError: GIT_ERROR })
+    expect(row?.removing).toBeUndefined()
+  })
+
+  it('Delete runs the recorded removal again: teardown, leftover, branch and metadata', async () => {
+    await failAfterGitDroppedIt()
+    killAllProcessesForWorktreeMock.mockClear()
+
+    await expect(remove({ worktreeId: featureId })).resolves.not.toHaveProperty('removing')
+
+    // Git has no registration to delete by; the leftover goes through Orca's own delete.
+    expect(removeWorktreeMock).not.toHaveBeenCalled()
+    expect(finishUnregisteredWorktreeRemoval).toHaveBeenCalledWith(
+      '/workspace/repo',
+      '/workspace/feature-wt',
+      { name: 'feature', head: 'feature' },
+      expect.any(Function),
+      {}
+    )
+    expect(killAllProcessesForWorktreeMock).toHaveBeenCalledWith(
+      featureId,
+      expect.objectContaining({ requirePhysicalStop: true })
+    )
+    expect(store.removeWorktreeMeta).toHaveBeenCalledWith(featureId, 'local')
+    expect(await listFeature()).toBeUndefined()
+  })
+
+  it('keeps the row with the new error when the retry fails again', async () => {
+    await failAfterGitDroppedIt()
+    vi.mocked(finishUnregisteredWorktreeRemoval).mockRejectedValueOnce(new Error('EPERM again'))
+
+    await expect(remove({ worktreeId: featureId })).rejects.toThrow('EPERM again')
+    await _settlePendingWorktreeRemovalsForTests()
+
+    expect(await listFeature()).toMatchObject({ removalError: 'EPERM again' })
+    expect(store.removeWorktreeMeta).not.toHaveBeenCalled()
+  })
+
+  it('joins a retry another client started while this Delete listed Git', async () => {
+    await failAfterGitDroppedIt()
+    const [main] = mockKnownFeatureWorktree()
+    listWorktreesMock.mockResolvedValue([main])
+    const otherClientsRetry = vi.fn(async () => ({}))
+    listWorktreesMock.mockImplementationOnce(async () => {
+      // Another client's Delete takes the failed record during this Delete's `git worktree list`.
+      void retryFailedWorktreeRemoval(featureId, 'local', () => ({
+        run: otherClientsRetry,
+        publish: () => {}
+      }))
+      return [main]
+    })
+
+    await expect(remove({ worktreeId: featureId })).resolves.not.toHaveProperty('removing')
+    await _settlePendingWorktreeRemovalsForTests()
+
+    expect(otherClientsRetry).toHaveBeenCalledTimes(1)
+    // Neither a second retry nor the delete for leftovers without a record ran.
+    expect(finishUnregisteredWorktreeRemoval).not.toHaveBeenCalled()
+    expect(removeWorktreeMock).not.toHaveBeenCalled()
+  })
+
+  it('Delete takes the normal delete once Git registers a checkout at the path again', async () => {
+    await failAfterGitDroppedIt()
+    // A new checkout at the same path: the recorded choices were for the leftover, not for it.
+    mockKnownFeatureWorktree()
+    removeWorktreeMock.mockResolvedValue({})
+
+    await remove({ worktreeId: featureId, force: false })
+    await _settlePendingWorktreeRemovalsForTests()
+
+    expect(finishUnregisteredWorktreeRemoval).not.toHaveBeenCalled()
+    expect(removeWorktreeMock).toHaveBeenCalledWith(
+      '/workspace/repo',
+      '/workspace/feature-wt',
+      false,
+      expect.anything()
+    )
+    listWorktreesMock.mockResolvedValue([mockKnownFeatureWorktree()[0]])
+    expect(await listFeature()).toBeUndefined()
+  })
+})

src/main/ipc/worktrees/listing/detected-provider-listing.ts

@@ -1,7 +1,8 @@
 import {
   projectPendingWorktreeRemovals,
-  snapshotPendingWorktreeRemovals
-} from '../../../worktree-background-removal'
+  snapshotPendingWorktreeRemovals,
+  withUnregisteredRemovalCheckouts
+} from '../../../worktree-removal-listing'
 import {
   getRepoExecutionHostId,
   getSshTargetIdForExecutionHost
@@ -149,6 +150,9 @@ export async function listDetectedWorktreesForCapturedRepo(
       return abortedResult() ?? null
     }
     const { gitWorktrees, fresh: freshScan, sideEffectToken, metadataPrune, hygieneDue } = scan
+    const localRows = connectionId
+      ? gitWorktrees
+      : await withUnregisteredRemovalCheckouts(repo.id, gitWorktrees)
     const aborted = abortedResult()
     if (aborted) {
       return aborted
@@ -193,7 +197,7 @@ export async function listDetectedWorktreesForCapturedRepo(
         ? buildDetectedGitWorktrees(store, repo, gitWorktrees, allMeta)
         : // Why always marked: the desktop renderer ships with this main process.
           projectPendingWorktreeRemovals(
-            buildDetectedGitWorktrees(store, repo, gitWorktrees, allMeta),
+            buildDetectedGitWorktrees(store, repo, localRows, allMeta),
             (worktree) => worktree.id,
             true,
             pendingAtScan

src/main/ipc/worktrees/listing/register-worktree-catalog-handlers.ts

@@ -36,8 +36,9 @@ import type { Worktree } from '../../../../shared/worktree/types'
 import {
   projectPendingWorktreeRemovals,
   snapshotPendingWorktreeRemovals,
+  withUnregisteredRemovalCheckouts,
   type PendingWorktreeRemovals
-} from '../../../worktree-background-removal'
+} from '../../../worktree-removal-listing'
 import { getLocalWorktreeScanGeneration } from '../../../local-worktree-scan-generation'
 import { getRegisteredWorktreeRootsRevision } from '../../registered-worktree-roots-cache'
 
@@ -167,7 +168,10 @@ export function registerWorktreeCatalogHandlers(context: WorktreeIpcContext): vo
         }
         loggedWorktreeListFailures.delete(`${repo.id}:${repo.path}`)
         const metadata = metadataForRepo(repo)
-        const worktrees = buildDetectedGitWorktrees(store, repo, gitWorktrees, metadata)
+        const rows = connectionId
+          ? gitWorktrees
+          : await withUnregisteredRemovalCheckouts(repo.id, gitWorktrees)
+        const worktrees = buildDetectedGitWorktrees(store, repo, rows, metadata)
           .filter((worktree) => worktree.visible)
           .map((worktree) => stampAndMergeVisibleDetectedWorktree(store, repo, worktree, metadata))
         return connectionId ? worktrees : markLocalWorktreesUnderRemoval(worktrees, pendingAtScan)
@@ -261,7 +265,10 @@ export function registerWorktreeCatalogHandlers(context: WorktreeIpcContext): vo
       }
       loggedWorktreeListFailures.delete(`${repo.id}:${repo.path}`)
       const metadata = allMeta ?? readAllWorktreeMetaForRepo(store, repo)
-      const worktrees = buildDetectedGitWorktrees(store, repo, gitWorktrees, metadata)
+      const rows = connectionId
+        ? gitWorktrees
+        : await withUnregisteredRemovalCheckouts(repo.id, gitWorktrees)
+      const worktrees = buildDetectedGitWorktrees(store, repo, rows, metadata)
         .filter((worktree) => worktree.visible)
         .map((worktree) => stampAndMergeVisibleDetectedWorktree(store, repo, worktree, metadata))
       return connectionId ? worktrees : markLocalWorktreesUnderRemoval(worktrees, pendingAtScan)

src/main/ipc/worktrees/removal/execute-worktree-removal.ts

@@ -36,6 +36,8 @@ import { removeFolderWorkspace } from './remove-folder-workspace'
 import { removeUnregisteredWorktree } from './remove-unregistered-worktree'
 import { removeRegisteredRemoteWorktree } from './remove-registered-remote-worktree'
 import { removeRegisteredLocalWorktree } from './remove-registered-local-worktree'
+import { retryFailedLocalWorktreeRemoval } from './retry-failed-local-worktree-removal'
+import { retryFailedRemovalUnlessRegistered } from '../../../worktree-removal-table'
 
 /**
  * Refuses a repo row whose two host spellings disagree.
@@ -114,6 +116,14 @@ export async function executeWorktreeRemoval(
     registeredWorktrees,
     resolveWorktreeRemovalHomeForHost(removalHostId)
   )
+  if (
+    !repo.connectionId &&
+    retryFailedRemovalUnlessRegistered(args.worktreeId, worktreePath, registeredWorktrees, () =>
+      retryFailedLocalWorktreeRemoval(context, args, removalHostId)
+    )
+  ) {
+    return { removing: true }
+  }
   if (!registeredWorktree) {
     return removeUnregisteredWorktree(
       context,