fix(worktree): preflight delete before terminal teardown (#2103)

Implement delete preflight checks before PTY teardown across git/runtime/ipc flows. Design reference: docs/worktree-delete-preflight.md
2026-05-16 13:52:59 -07:00 · 2026-05-16 13:52:59 -07:00 · 087f1aa9bf
parent 675b1b0cb6
commit 087f1aa9bf
11 changed files with 450 additions and 66 deletions
--- a/docs/worktree-delete-preflight.md
+++ b/docs/worktree-delete-preflight.md
@ -0,0 +1,78 @@
+# Worktree Delete Preflight
+
+## Problem
+
+- Local delete paths kill PTYs before git deletion:
+- `worktrees:remove` IPC (`src/main/ipc/worktrees.ts`)
+- `removeManagedWorktree` runtime/RPC (`src/main/runtime/orca-runtime.ts`)
+- On non-force failures (dirty/untracked is common), the worktree stays on disk but terminals are already gone.
+- PTY teardown is intentionally destructive and best-effort (`src/main/runtime/worktree-teardown.ts`), so non-force deletability must be checked first.
+
+## Ground Truth From Code
+
+- There is no dry-run remove path currently used in git helpers (`src/main/git/worktree.ts`).
+- Errors shown to users are normalized through `formatWorktreeRemovalError` (`src/main/ipc/worktree-logic.ts`).
+- SSH-backed repos already delegate deletion to provider APIs and should remain provider-owned.
+- Current orphan cleanup (`is not a working tree` handling + prune + metadata cleanup) lives in IPC/runtime remove catch blocks and must not regress.
+
+## Non-goals
+
+- Do not change force-delete semantics; force may still kill PTYs before git remove.
+- Do not add new renderer confirmation states or copy.
+- Do not attempt to predict every possible git remove failure.
+- Do not change SSH provider teardown ownership.
+
+## Design
+
+1. Add local preflight helper in `src/main/git/worktree.ts`.
+- Export `assertWorktreeCleanForRemoval(worktreePath: string, force = false): Promise<void>`.
+- If `force`, return immediately.
+- Run `git status --porcelain --untracked-files=all` in `cwd = worktreePath`.
+- If output is non-empty, throw a dedicated error (dirty/untracked).
+- If command fails, rethrow original error.
+
+2. IPC local delete ordering (`worktrees:remove`).
+- Keep canonicalization and protected-path validation first (`getRegisteredDeletableWorktree`).
+- Keep SSH provider branch unchanged.
+- Keep archive hook and symlink cleanup before preflight, so preflight checks the exact post-hook/post-symlink state that `git worktree remove` will see.
+- Run preflight.
+- If preflight throws an orphan/missing-worktree style error, continue to existing remove path so current orphan cleanup behavior still executes.
+  Treat at least these as orphan-compatible for preflight: `is not a working tree`, `not a git repository`, and missing-path (`ENOENT`) failures from running status in a removed directory.
+- For other preflight failures, throw via `formatWorktreeRemovalError(...)`.
+- Only after successful preflight: run `killAllProcessesForWorktree(...)`, then `removeWorktree(...)`.
+
+3. Runtime/RPC local delete ordering (`removeManagedWorktree`).
+- Keep SSH branch unchanged.
+- Keep current hook behavior (archive optional via `--run-hooks`, warning when configured but skipped).
+- Run preflight after hook handling.
+- Preserve orphan compatibility exactly as in IPC: preflight must not short-circuit existing orphan cleanup semantics (including `not a git repository`/`ENOENT` preflight failures that should fall through to the existing remove/catch path).
+- On successful preflight: run PTY teardown, then `removeWorktree`.
+- Route failures through existing formatted error surface.
+
+4. Failure-class behavior contract.
+- Dirty/untracked (non-force): fail before PTY teardown.
+- Preflight subprocess/tooling failures: fail before PTY teardown, formatted.
+- Orphan/missing-worktree conditions (`is not a working tree`, `not a git repository`, `ENOENT`): retain current cleanup-and-metadata-removal behavior by running the existing remove/catch flow without PTY teardown.
+- Force deletes: no preflight; keep current teardown-before-remove order.
+
+5. Tests.
+- Update ordering assertions for non-force local deletes to `preflight -> kill -> git`.
+- Add IPC/runtime tests proving dirty non-force failures happen before any PTY kill.
+- Add IPC/runtime tests proving preflight error formatting uses `formatWorktreeRemovalError` path.
+- Add IPC/runtime regression tests proving orphan cleanup still runs when preflight encounters orphan-like failures.
+- Keep force ordering tests (`kill -> git`).
+- Keep SSH tests proving local PTY teardown is not used for SSH-backed repos.
+
+## Concurrency, Consistency, Limits
+
+- Preflight narrows, but does not close, the race window: external edits can occur after preflight and before `git worktree remove`.
+- Multi-window and out-of-band mutation races remain possible between canonicalization, hooks/symlink cleanup, preflight, kill, and remove.
+- IPC has symlink cleanup before preflight; runtime does not. That asymmetry remains unless runtime gains equivalent cleanup.
+- Cost is one additional git subprocess per non-force local delete; this is acceptable for a user-initiated destructive action.
+
+## Rollout
+
+1. Implement `assertWorktreeCleanForRemoval` + unit tests in `src/main/git/remove-worktree.test.ts`.
+2. Wire IPC ordering and failure mapping in `src/main/ipc/worktrees.ts`; update `src/main/ipc/worktrees.test.ts`.
+3. Wire runtime ordering and failure mapping in `src/main/runtime/orca-runtime.ts`; update `src/main/runtime/orca-runtime.test.ts`.
+4. Run focused tests, then `pnpm typecheck` and `pnpm lint`.
--- a/src/main/git/remove-worktree.test.ts
+++ b/src/main/git/remove-worktree.test.ts
@ -32,7 +32,12 @@ vi.mock('fs/promises', async () => {
  return { ...actual, stat: statMock }
 })

-import { addSparseWorktree, listWorktrees, removeWorktree } from './worktree'
+import {
+  addSparseWorktree,
+  assertWorktreeCleanForRemoval,
+  listWorktrees,
+  removeWorktree
+} from './worktree'

 type MockResult = {
  error?: Error
@ -292,6 +297,46 @@ branch refs/heads/main
  })
 })

+describe('assertWorktreeCleanForRemoval', () => {
+  beforeEach(() => {
+    gitExecFileAsyncMock.mockReset()
+  })
+
+  it('returns without checking git status for force removals', async () => {
+    await expect(assertWorktreeCleanForRemoval('/repo-feature', true)).resolves.toBeUndefined()
+    expect(gitExecFileAsyncMock).not.toHaveBeenCalled()
+  })
+
+  it('passes when git status output is empty', async () => {
+    gitExecFileAsyncMock.mockResolvedValueOnce({ stdout: '', stderr: '' })
+
+    await expect(assertWorktreeCleanForRemoval('/repo-feature')).resolves.toBeUndefined()
+
+    expect(gitExecFileAsyncMock).toHaveBeenCalledWith(
+      ['status', '--porcelain', '--untracked-files=all'],
+      { cwd: '/repo-feature' }
+    )
+  })
+
+  it('throws a dedicated dirty/untracked error when status output is non-empty', async () => {
+    gitExecFileAsyncMock.mockResolvedValueOnce({ stdout: '?? scratch.txt\n', stderr: '' })
+
+    await expect(assertWorktreeCleanForRemoval('/repo-feature')).rejects.toMatchObject({
+      message: 'Worktree has uncommitted or untracked changes.',
+      stdout: '?? scratch.txt\n'
+    })
+  })
+
+  it('rethrows preflight subprocess failures as-is', async () => {
+    const error = Object.assign(new Error('fatal: not a git repository'), {
+      stderr: 'fatal: not a git repository (or any of the parent directories): .git\n'
+    })
+    gitExecFileAsyncMock.mockRejectedValueOnce(error)
+
+    await expect(assertWorktreeCleanForRemoval('/repo-feature')).rejects.toBe(error)
+  })
+})
+
 describe('listWorktrees', () => {
  beforeEach(() => {
    gitExecFileAsyncMock.mockReset()
--- a/src/main/git/worktree.ts
+++ b/src/main/git/worktree.ts
@ -378,6 +378,29 @@ export async function removeWorktree(
  }
 }

+/**
+ * Assert a worktree is clean enough for non-force removal.
+ */
+export async function assertWorktreeCleanForRemoval(
+  worktreePath: string,
+  force = false
+): Promise<void> {
+  if (force) {
+    return
+  }
+
+  const { stdout } = await gitExecFileAsync(['status', '--porcelain', '--untracked-files=all'], {
+    cwd: worktreePath
+  })
+  if (!stdout.trim()) {
+    return
+  }
+
+  const error = new Error('Worktree has uncommitted or untracked changes.')
+  ;(error as Error & { stdout?: string }).stdout = stdout
+  throw error
+}
+
 function translateWorktreePath(worktreePath: string, repoPath: string): string {
  const prefix = 'worktree '
  const translated = translateWslOutputPaths(`${prefix}${worktreePath}`, repoPath)
--- a/src/main/ipc/worktree-logic.test.ts
+++ b/src/main/ipc/worktree-logic.test.ts
@ -13,6 +13,7 @@ import {
  mergeWorktree,
  parseWorktreeId,
  formatWorktreeRemovalError,
+  isOrphanCompatiblePreflightError,
  isOrphanedWorktreeError,
  areWorktreePathsEqual
 } from './worktree-logic'
@ -366,3 +367,35 @@ describe('isOrphanedWorktreeError', () => {
    expect(isOrphanedWorktreeError(null)).toBe(false)
  })
 })
+
+describe('isOrphanCompatiblePreflightError', () => {
+  it('matches not-a-working-tree errors', () => {
+    const error = Object.assign(new Error('git failed'), {
+      stderr: "fatal: '/some/path' is not a working tree"
+    })
+
+    expect(isOrphanCompatiblePreflightError(error)).toBe(true)
+  })
+
+  it('matches status failures from non-repo directories', () => {
+    const error = Object.assign(new Error('status failed'), {
+      stderr: 'fatal: not a git repository (or any of the parent directories): .git'
+    })
+
+    expect(isOrphanCompatiblePreflightError(error)).toBe(true)
+  })
+
+  it('matches missing directories by error code', () => {
+    const error = Object.assign(new Error('spawn git'), { code: 'ENOENT' })
+
+    expect(isOrphanCompatiblePreflightError(error)).toBe(true)
+  })
+
+  it('does not match unrelated subprocess failures', () => {
+    const error = Object.assign(new Error('status failed'), {
+      stderr: 'fatal: unable to read current working directory'
+    })
+
+    expect(isOrphanCompatiblePreflightError(error)).toBe(false)
+  })
+})
--- a/src/main/ipc/worktree-logic.ts
+++ b/src/main/ipc/worktree-logic.ts
@ -239,6 +239,25 @@ export function isOrphanedWorktreeError(error: unknown): boolean {
  return /is not a working tree/.test(msg)
 }

+export function isOrphanCompatiblePreflightError(error: unknown): boolean {
+  if (isOrphanedWorktreeError(error)) {
+    return true
+  }
+  if (!(error instanceof Error)) {
+    return false
+  }
+  const errorWithDetails = error as Error & { code?: unknown; stderr?: string; stdout?: string }
+  const details = [
+    errorWithDetails.stderr,
+    errorWithDetails.stdout,
+    errorWithDetails.message,
+    typeof errorWithDetails.code === 'string' ? errorWithDetails.code : undefined
+  ]
+    .filter((value): value is string => Boolean(value))
+    .join('\n')
+  return /not a git repository/i.test(details) || /\bENOENT\b/i.test(details)
+}
+
 /**
 * Format a human-readable error message for worktree removal failures.
 */
--- a/src/main/ipc/worktrees.test.ts
+++ b/src/main/ipc/worktrees.test.ts
@ -5,6 +5,7 @@ const {
  handleMock,
  removeHandlerMock,
  listWorktreesMock,
+  assertWorktreeCleanForRemovalMock,
  addWorktreeMock,
  addSparseWorktreeMock,
  removeWorktreeMock,
@ -31,6 +32,7 @@ const {
  handleMock: vi.fn(),
  removeHandlerMock: vi.fn(),
  listWorktreesMock: vi.fn(),
+  assertWorktreeCleanForRemovalMock: vi.fn(),
  addWorktreeMock: vi.fn(),
  addSparseWorktreeMock: vi.fn(),
  removeWorktreeMock: vi.fn(),
@ -64,6 +66,7 @@ vi.mock('electron', () => ({

 vi.mock('../git/worktree', () => ({
  listWorktrees: listWorktreesMock,
+  assertWorktreeCleanForRemoval: assertWorktreeCleanForRemovalMock,
  addWorktree: addWorktreeMock,
  addSparseWorktree: addSparseWorktreeMock,
  removeWorktree: removeWorktreeMock
@ -176,6 +179,7 @@ describe('registerWorktreeHandlers', () => {
      handleMock,
      removeHandlerMock,
      listWorktreesMock,
+      assertWorktreeCleanForRemovalMock,
      addWorktreeMock,
      addSparseWorktreeMock,
      removeWorktreeMock,
@ -219,6 +223,7 @@ describe('registerWorktreeHandlers', () => {
      providerStopped: 0,
      registryStopped: 0
    })
+    assertWorktreeCleanForRemovalMock.mockResolvedValue(undefined)
    getLocalPtyProviderMock.mockReturnValue({} as never)

    for (const key of Object.keys(handlers)) {
@ -1792,6 +1797,9 @@ describe('registerWorktreeHandlers', () => {
    mockKnownFeatureWorktree()
    getEffectiveHooksMock.mockReturnValue(null)
    const callOrder: string[] = []
+    assertWorktreeCleanForRemovalMock.mockImplementation(async () => {
+      callOrder.push('preflight')
+    })
    killAllProcessesForWorktreeMock.mockImplementation(async () => {
      callOrder.push('kill')
      return { runtimeStopped: 1, providerStopped: 0, registryStopped: 0 }
@ -1811,7 +1819,73 @@ describe('registerWorktreeHandlers', () => {
      })
    )
    expect(removeWorktreeMock).toHaveBeenCalled()
-    expect(callOrder).toEqual(['kill', 'git'])
+    expect(callOrder).toEqual(['preflight', 'kill', 'git'])
+  })
+
+  it('fails dirty non-force deletes before PTY teardown', async () => {
+    mockKnownFeatureWorktree()
+    getEffectiveHooksMock.mockReturnValue(null)
+    assertWorktreeCleanForRemovalMock.mockRejectedValue(
+      Object.assign(new Error('Worktree has uncommitted or untracked changes.'), {
+        stdout: '?? scratch.txt\n'
+      })
+    )
+
+    await expect(
+      handlers['worktrees:remove'](null, {
+        worktreeId: 'repo-1::/workspace/feature-wt'
+      })
+    ).rejects.toThrow('Failed to delete worktree at /workspace/feature-wt. ?? scratch.txt')
+
+    expect(killAllProcessesForWorktreeMock).not.toHaveBeenCalled()
+    expect(removeWorktreeMock).not.toHaveBeenCalled()
+  })
+
+  it('formats preflight subprocess failures and does not tear down PTYs', async () => {
+    mockKnownFeatureWorktree()
+    getEffectiveHooksMock.mockReturnValue(null)
+    assertWorktreeCleanForRemovalMock.mockRejectedValue(
+      Object.assign(new Error('status failed'), {
+        stderr: 'fatal: unable to read current working directory\n'
+      })
+    )
+
+    await expect(
+      handlers['worktrees:remove'](null, {
+        worktreeId: 'repo-1::/workspace/feature-wt'
+      })
+    ).rejects.toThrow(
+      'Failed to delete worktree at /workspace/feature-wt. fatal: unable to read current working directory'
+    )
+
+    expect(killAllProcessesForWorktreeMock).not.toHaveBeenCalled()
+    expect(removeWorktreeMock).not.toHaveBeenCalled()
+  })
+
+  it('falls through to orphan cleanup when preflight reports missing/non-repo worktree', async () => {
+    mockKnownFeatureWorktree()
+    getEffectiveHooksMock.mockReturnValue(null)
+    assertWorktreeCleanForRemovalMock.mockRejectedValue(
+      Object.assign(new Error('status failed'), {
+        stderr: 'fatal: not a git repository (or any of the parent directories): .git\n'
+      })
+    )
+    removeWorktreeMock.mockRejectedValue(
+      Object.assign(new Error('git worktree remove failed'), {
+        stderr: "fatal: '/workspace/feature-wt' is not a working tree"
+      })
+    )
+    gitExecFileAsyncMock.mockResolvedValue({ stdout: '', stderr: '' })
+
+    await handlers['worktrees:remove'](null, {
+      worktreeId: 'repo-1::/workspace/feature-wt'
+    })
+
+    expect(killAllProcessesForWorktreeMock).not.toHaveBeenCalled()
+    expect(removeWorktreeMock).toHaveBeenCalled()
+    expect(gitExecFileAsyncMock).toHaveBeenCalledWith(['worktree', 'prune'], {
+      cwd: '/workspace/repo'
+    })
  })

  it('skips the PTY teardown for SSH-backed repos (design §6 out-of-scope)', async () => {
--- a/src/main/ipc/worktrees.ts
+++ b/src/main/ipc/worktrees.ts
@ -14,7 +14,11 @@ import type {
  Repo,
  WorktreeMeta
 } from '../../shared/types'
-import { listWorktrees as listGitWorktrees, removeWorktree } from '../git/worktree'
+import {
+  assertWorktreeCleanForRemoval,
+  listWorktrees as listGitWorktrees,
+  removeWorktree
+} from '../git/worktree'
 import { gitExecFileAsync } from '../git/runner'
 import { getDefaultRemote } from '../git/repo'
 import { getPullRequestPushTarget, getWorkItem } from '../github/client'
@ -37,6 +41,7 @@ import {
  parseWorktreeId,
  areWorktreePathsEqual,
  formatWorktreeRemovalError,
+  isOrphanCompatiblePreflightError,
  isOrphanedWorktreeError
 } from './worktree-logic'
 import {
@ -682,31 +687,6 @@ export function registerWorktreeHandlers(
        registeredWorktrees
      ).path

-      // Why: kill every PTY belonging to this worktree BEFORE git-level
-      // removal. The renderer pre-kills via shutdownWorktreeTerminals, but
-      // defensive teardown here protects against: (a) a future renderer bug,
-      // (b) a disconnected window, (c) an out-of-band window.api.worktrees.remove
-      // caller. Placement is before the SSH early-return so local-host PTYs
-      // are still reaped for local repos; SSH-backed PTYs are handled by the
-      // remote provider's own teardown (design §4.3, §6).
-      if (!repo.connectionId) {
-        await killAllProcessesForWorktree(args.worktreeId, {
-          runtime,
-          localProvider: getLocalPtyProvider()
-        })
-          .then((r) => {
-            const total = r.runtimeStopped + r.providerStopped + r.registryStopped
-            if (total > 0) {
-              console.info(
-                `[worktree-teardown] ${args.worktreeId} killed runtime=${r.runtimeStopped} provider=${r.providerStopped} registry=${r.registryStopped}`
-              )
-            }
-          })
-          .catch((err) => {
-            console.warn(`[worktree-teardown] failed for ${args.worktreeId}:`, err)
-          })
-      }
-
      if (repo.connectionId) {
        await provider!.removeWorktree(canonicalWorktreePath, args.force)
        runtime.clearOptimisticReconcileToken(args.worktreeId)
@ -734,6 +714,40 @@ export function registerWorktreeHandlers(
        await removeWorktreeSymlinks(canonicalWorktreePath, repo.symlinkPaths)
      }

+      let shouldTearDownPtys = true
+      try {
+        await assertWorktreeCleanForRemoval(canonicalWorktreePath, args.force ?? false)
+      } catch (error) {
+        if (!isOrphanCompatiblePreflightError(error)) {
+          throw new Error(
+            formatWorktreeRemovalError(error, canonicalWorktreePath, args.force ?? false)
+          )
+        }
+        // Why: orphan cleanup does not need live shells to be killed first,
+        // and preflight did not prove the worktree is cleanly removable.
+        shouldTearDownPtys = false
+      }
+
+      if (shouldTearDownPtys) {
+        // Why: once preflight proves normal deletion is clean, kill PTYs before
+        // git-level removal so shells cannot keep the directory busy.
+        await killAllProcessesForWorktree(args.worktreeId, {
+          runtime,
+          localProvider: getLocalPtyProvider()
+        })
+          .then((r) => {
+            const total = r.runtimeStopped + r.providerStopped + r.registryStopped
+            if (total > 0) {
+              console.info(
+                `[worktree-teardown] ${args.worktreeId} killed runtime=${r.runtimeStopped} provider=${r.providerStopped} registry=${r.registryStopped}`
+              )
+            }
+          })
+          .catch((err) => {
+            console.warn(`[worktree-teardown] failed for ${args.worktreeId}:`, err)
+          })
+      }
+
      try {
        await removeWorktree(repo.path, canonicalWorktreePath, args.force ?? false)
      } catch (error) {
--- a/src/main/runtime/orca-runtime.test.ts
+++ b/src/main/runtime/orca-runtime.test.ts
@ -3,7 +3,12 @@ import { afterEach, describe, expect, it, vi } from 'vitest'
 import { EventEmitter } from 'events'
 import { mkdtemp, rm } from 'fs/promises'
 import type { WorktreeLineage, WorktreeMeta } from '../../shared/types'
-import { addWorktree, listWorktrees, removeWorktree } from '../git/worktree'
+import {
+  addWorktree,
+  assertWorktreeCleanForRemoval,
+  listWorktrees,
+  removeWorktree
+} from '../git/worktree'
 import * as gitRunner from '../git/runner'
 import {
  createSetupRunnerScript,
@ -67,6 +72,7 @@ const {

 vi.mock('../git/worktree', () => ({
  listWorktrees: vi.fn().mockResolvedValue(MOCK_GIT_WORKTREES),
+  assertWorktreeCleanForRemoval: vi.fn().mockResolvedValue(undefined),
  addWorktree: addWorktreeMock,
  removeWorktree: removeWorktreeMock
 }))
@ -122,6 +128,8 @@ vi.mock('../git/repo', async (importOriginal) => {
 afterEach(() => {
  vi.mocked(listWorktrees).mockResolvedValue(MOCK_GIT_WORKTREES)
  vi.mocked(addWorktree).mockReset()
+  vi.mocked(assertWorktreeCleanForRemoval).mockReset()
+  vi.mocked(assertWorktreeCleanForRemoval).mockResolvedValue(undefined)
  vi.mocked(removeWorktree).mockReset()
  sshGitProviders.clear()
  getSshGitProviderMock.mockReset()
@ -3820,6 +3828,73 @@ describe('OrcaRuntimeService', () => {
    )
  })

+  it('fails dirty non-force deletes before PTY teardown', async () => {
+    const runtime = new OrcaRuntimeService(store)
+    const killSpy = vi.fn().mockReturnValue(true)
+    runtime.setPtyController({
+      write: () => true,
+      kill: (id) => killSpy(id),
+      getForegroundProcess: async () => null
+    })
+    syncSinglePty(runtime, 'pty-1')
+    vi.mocked(getEffectiveHooks).mockReturnValue(null)
+    vi.mocked(assertWorktreeCleanForRemoval).mockRejectedValue(
+      Object.assign(new Error('Worktree has uncommitted or untracked changes.'), {
+        stdout: '?? scratch.txt\n'
+      })
+    )
+
+    await expect(runtime.removeManagedWorktree(TEST_WORKTREE_ID)).rejects.toThrow(
+      `Failed to delete worktree at ${TEST_WORKTREE_PATH}. ?? scratch.txt`
+    )
+
+    expect(killSpy).not.toHaveBeenCalled()
+    expect(removeWorktree).not.toHaveBeenCalled()
+  })
+
+  it('formats preflight subprocess failures and skips PTY teardown', async () => {
+    const runtime = new OrcaRuntimeService(store)
+    const killSpy = vi.fn().mockReturnValue(true)
+    runtime.setPtyController({
+      write: () => true,
+      kill: (id) => killSpy(id),
+      getForegroundProcess: async () => null
+    })
+    syncSinglePty(runtime, 'pty-1')
+    vi.mocked(getEffectiveHooks).mockReturnValue(null)
+    vi.mocked(assertWorktreeCleanForRemoval).mockRejectedValue(
+      Object.assign(new Error('status failed'), {
+        stderr: 'fatal: unable to read current working directory\n'
+      })
+    )
+
+    await expect(runtime.removeManagedWorktree(TEST_WORKTREE_ID)).rejects.toThrow(
+      `Failed to delete worktree at ${TEST_WORKTREE_PATH}. fatal: unable to read current working directory`
+    )
+
+    expect(killSpy).not.toHaveBeenCalled()
+    expect(removeWorktree).not.toHaveBeenCalled()
+  })
+
+  it('falls through to orphan cleanup when preflight reports missing/non-repo worktree', async () => {
+    const runtime = new OrcaRuntimeService(store)
+    vi.mocked(getEffectiveHooks).mockReturnValue(null)
+    vi.mocked(assertWorktreeCleanForRemoval).mockRejectedValue(
+      Object.assign(new Error('status failed'), {
+        stderr: 'fatal: not a git repository (or any of the parent directories): .git\n'
+      })
+    )
+    vi.mocked(removeWorktree).mockRejectedValue(
+      Object.assign(new Error('git worktree remove failed'), {
+        stderr: `fatal: '${TEST_WORKTREE_PATH}' is not a working tree`
+      })
+    )
+    vi.spyOn(gitRunner, 'gitExecFileAsync').mockResolvedValue({ stdout: '', stderr: '' })
+
+    await expect(runtime.removeManagedWorktree(TEST_WORKTREE_ID)).resolves.toEqual({})
+    expect(removeWorktree).toHaveBeenCalledWith(TEST_REPO_PATH, TEST_WORKTREE_PATH, false)
+  })
+
  it('runs archive hooks for CLI worktree removal when hooks are explicitly enabled', async () => {
    const runtime = new OrcaRuntimeService(store)
    vi.mocked(getEffectiveHooks).mockReturnValue({
@ -4228,6 +4303,9 @@ describe('OrcaRuntimeService', () => {
      const killSpy = vi.fn().mockReturnValue(true)
      const localProvider = createProviderStub(async () => [])
      const callOrder: string[] = []
+      vi.mocked(assertWorktreeCleanForRemoval).mockImplementation(async () => {
+        callOrder.push('preflight')
+      })
      vi.mocked(removeWorktree).mockImplementation(async () => {
        callOrder.push('git-removeWorktree')
      })
@ -4253,8 +4331,11 @@ describe('OrcaRuntimeService', () => {
      expect(killSpy).toHaveBeenCalledWith('pty-1')
      // The provider-prefix sweep and the git removal must happen AFTER the
      // runtime-graph kill. Git removal must NOT happen before any kill.
+      const preflightIdx = callOrder.indexOf('preflight')
      const killIdx = callOrder.indexOf('kill:pty-1')
      const gitIdx = callOrder.indexOf('git-removeWorktree')
+      expect(preflightIdx).toBeGreaterThanOrEqual(0)
+      expect(killIdx).toBeGreaterThan(preflightIdx)
      expect(killIdx).toBeGreaterThanOrEqual(0)
      expect(gitIdx).toBeGreaterThan(killIdx)
    })
--- a/src/main/runtime/orca-runtime.ts
+++ b/src/main/runtime/orca-runtime.ts
@ -204,7 +204,13 @@ import {
  getRemoteDrift,
  getRecentDriftSubjects
 } from '../git/repo'
-import { listWorktrees, addWorktree, addSparseWorktree, removeWorktree } from '../git/worktree'
+import {
+  listWorktrees,
+  addWorktree,
+  addSparseWorktree,
+  assertWorktreeCleanForRemoval,
+  removeWorktree
+} from '../git/worktree'
 import { isENOENT } from '../ipc/filesystem-auth'
 import {
  createSetupRunnerScript,
@ -235,6 +241,7 @@ import {
  computeWorktreePath,
  ensurePathWithinWorkspace,
  formatWorktreeRemovalError,
+  isOrphanCompatiblePreflightError,
  isOrphanedWorktreeError,
  mergeWorktree,
  sanitizeWorktreeName,
@ -6016,15 +6023,36 @@ export class OrcaRuntimeService {
      return {}
    }

-    // Why: kill every PTY belonging to this worktree BEFORE the git-level
-    // removal. Some shells keep the worktree directory busy, and `git worktree
-    // remove` throws a confusing error if PTYs still hold it open. This also
-    // closes the headless-CLI leak (design §2a/§2b): without this call, the
-    // CLI path runs git removal and never touches PTYs, leaving zombies
-    // behind. Best-effort: any failure here must not prevent git removal —
-    // the worst case without the call is the status quo.
+    const hooks = getEffectiveHooks(repo)
+    let warning: string | undefined
+    if (hooks?.scripts.archive && runHooks) {
+      const result = await runHook('archive', worktree.path, repo)
+      if (!result.success) {
+        console.error(`[hooks] archive hook failed for ${worktree.path}:`, result.output)
+      }
+    } else if (hooks?.scripts.archive) {
+      // Runtime RPC calls have no renderer trust prompt, so hooks require explicit CLI opt-in.
+      warning = `orca.yaml archive hook skipped for ${worktree.path}; pass --run-hooks to run it.`
+      console.warn(`[hooks] ${warning}`)
+    }
+
+    let shouldTearDownPtys = true
+    try {
+      await assertWorktreeCleanForRemoval(worktree.path, force)
+    } catch (error) {
+      if (!isOrphanCompatiblePreflightError(error)) {
+        throw new Error(formatWorktreeRemovalError(error, worktree.path, force))
+      }
+      // Why: orphan cleanup does not need live shells to be killed first,
+      // and preflight did not prove the worktree is cleanly removable.
+      shouldTearDownPtys = false
+    }
+
    const localProvider = this.getLocalProvider()
-    if (localProvider) {
+    if (localProvider && shouldTearDownPtys) {
+      // Why: once preflight proves normal deletion is clean, kill PTYs before
+      // git-level removal so shells cannot keep the directory busy. This also
+      // closes the headless-CLI leak for confirmed-removable worktrees.
      await killAllProcessesForWorktree(worktree.id, {
        runtime: this,
        localProvider
@ -6047,19 +6075,6 @@ export class OrcaRuntimeService {
        })
    }

-    const hooks = getEffectiveHooks(repo)
-    let warning: string | undefined
-    if (hooks?.scripts.archive && runHooks) {
-      const result = await runHook('archive', worktree.path, repo)
-      if (!result.success) {
-        console.error(`[hooks] archive hook failed for ${worktree.path}:`, result.output)
-      }
-    } else if (hooks?.scripts.archive) {
-      // Runtime RPC calls have no renderer trust prompt, so hooks require explicit CLI opt-in.
-      warning = `orca.yaml archive hook skipped for ${worktree.path}; pass --run-hooks to run it.`
-      console.warn(`[hooks] ${warning}`)
-    }
-
    try {
      await removeWorktree(repo.path, worktree.path, force)
    } catch (error) {
--- a/src/renderer/src/store/slices/store-cascades.test.ts
+++ b/src/renderer/src/store/slices/store-cascades.test.ts
@ -167,6 +167,8 @@ describe('removeWorktree cascade', () => {
    // State NOT cleaned up
    expect(s.worktreesByRepo['repo1']).toHaveLength(1)
    expect(s.tabsByWorktree[worktreeId]).toHaveLength(1)
+    expect(s.ptyIdsByTabId['tab1']).toEqual(['pty1'])
+    expect(mockApi.pty.kill).not.toHaveBeenCalled()
    expect(s.activeWorktreeId).toBe(worktreeId)
  })

@ -268,7 +270,7 @@ describe('removeWorktree cascade', () => {
    expect(s.fileSearchStateByWorktree[wt1]).toBeUndefined()
  })

-  it('shuts down terminals before asking the backend to remove the worktree', async () => {
+  it('shuts down terminals after the backend confirms worktree removal', async () => {
    const store = createTestStore()
    const worktreeId = 'repo1::/path/wt1'
    const callOrder: string[] = []
@ -298,7 +300,7 @@ describe('removeWorktree cascade', () => {
    const result = await store.getState().removeWorktree(worktreeId)

    expect(result).toEqual({ ok: true })
-    expect(callOrder).toEqual(['kill', 'remove'])
+    expect(callOrder).toEqual(['remove', 'kill'])
  })
 })

--- a/src/renderer/src/store/slices/worktrees.ts
+++ b/src/renderer/src/store/slices/worktrees.ts
@ -531,10 +531,19 @@ export const createWorktreeSlice: StateCreator<AppState, [], [], WorktreeSlice>
      const trustDecision = await ensureHooksConfirmed(get(), repoIdForTrust, 'archive')
      const skipArchive = trustDecision === 'skip'

-      // Why: setup-enabled worktrees now commonly have a live shell open as soon as
-      // they are created. We must tear those PTYs down before asking Git to remove
-      // the working tree or Windows and some shells can keep the directory in use
-      // and make delete look broken even though the git state itself is fine.
+      const target = getActiveRuntimeTarget(get().settings)
+      await (target.kind === 'local'
+        ? window.api.worktrees.remove({ worktreeId, force, skipArchive })
+        : callRuntimeRpc(
+            target,
+            'worktree.rm',
+            { worktree: worktreeId, force, runHooks: !skipArchive },
+            { timeoutMs: 60_000 }
+          ))
+
+      // Why: backend delete paths now preflight and kill PTYs only after the
+      // worktree is cleanly removable. Renderer state follows the successful
+      // backend result so blocked dirty deletes keep their terminals intact.
      //
      // Why browsers first: `shutdownWorktreeTerminals` used to own the
      // `browserTabsByWorktree[worktreeId]` delete as a side effect, which would
@ -545,15 +554,6 @@ export const createWorktreeSlice: StateCreator<AppState, [], [], WorktreeSlice>
      // can intercept them.
      await get().shutdownWorktreeBrowsers(worktreeId)
      await get().shutdownWorktreeTerminals(worktreeId)
-      const target = getActiveRuntimeTarget(get().settings)
-      await (target.kind === 'local'
-        ? window.api.worktrees.remove({ worktreeId, force, skipArchive })
-        : callRuntimeRpc(
-            target,
-            'worktree.rm',
-            { worktree: worktreeId, force, runHooks: !skipArchive },
-            { timeoutMs: 60_000 }
-          ))
      const tabs = get().tabsByWorktree[worktreeId] ?? []
      const tabIds = new Set(tabs.map((t) => t.id))