fix(worktree): stop terminal removal fence error flashing on delete (#10240)

* fix(worktree): stop terminal removal fence error flashing on delete

Deleting a worktree kills its PTYs for the filesystem teardown, then runs
git worktree remove (~1s). During that window a doomed pane races a fresh
respawn that main correctly fences with TerminalRemovalInProgressError, but
the renderer surfaced that internal fence verbatim as a pane error banner
until the worktree unmounted.

- startFreshSpawn: skip the respawn when the pane's own worktree is being
  deleted (isDeleting) — no shell should spawn into a directory being removed
  and the pane is about to unmount.
- reportError: swallow the removal fence at the single pane-error sink so it
  never reaches the banner, covering the parent-removal-fences-child case the
  own-worktree skip cannot see.
- Share the fence messages between main (thrown) and renderer (recognized) via
  worktree-removal-fence-error.ts so the thrown text and predicate can't drift.

* test(worktree): assert onError callback captured before invoking

Optional invocation let the fence-suppression test false-pass if the
transport onError wiring broke; require the callback so a broken wire
fails loudly. Addresses CodeRabbit review on #10240.

src/main/ipc/watcher-removal-gate.test.ts

@@ -6,6 +6,7 @@ import {
   TerminalRemovalInProgressError,
   WatcherRemovalInProgressError
 } from './watcher-removal-gate'
+import { isWorktreeRemovalFenceError } from '../../shared/worktree-removal-fence-error'
 
 describe('watcher removal gate', () => {
   it('waits for an existing install and rejects later equivalent-path installs', async () => {
@@ -106,4 +107,34 @@ describe('watcher removal gate', () => {
     finishInstall()
     removal.release()
   })
+
+  // Why: the renderer swallows this fence via isWorktreeRemovalFenceError so a
+  // doomed pane never shows the raw error. That only holds if the thrown message
+  // still matches the shared predicate — pin the cross-module contract here.
+  it('throws fence errors the renderer recognizes as benign removal fences', async () => {
+    const removal = acquireWatcherRemovalGate('/repo')
+    await removal.ready
+
+    const terminalError = (() => {
+      try {
+        beginTerminalInstall('/repo')
+      } catch (error) {
+        return error as Error
+      }
+      throw new Error('expected terminal install to be fenced')
+    })()
+    const watcherError = (() => {
+      try {
+        beginWatcherInstall('/repo')
+      } catch (error) {
+        return error as Error
+      }
+      throw new Error('expected watcher install to be fenced')
+    })()
+
+    expect(isWorktreeRemovalFenceError(terminalError.message)).toBe(true)
+    expect(isWorktreeRemovalFenceError(watcherError.message)).toBe(true)
+
+    removal.release()
+  })
 })

src/main/ipc/watcher-removal-gate.ts

@@ -2,6 +2,10 @@ import {
   isPathInsideOrEqual,
   normalizeRuntimePathForComparison
 } from '../../shared/cross-platform-path'
+import {
+  TERMINAL_REMOVAL_IN_PROGRESS_MESSAGE,
+  WATCHER_REMOVAL_IN_PROGRESS_MESSAGE
+} from '../../shared/worktree-removal-fence-error'
 
 type WatcherRemovalGateState = {
   connectionId: string | null
@@ -20,7 +24,7 @@ export class WatcherRemovalInProgressError extends Error {
   readonly code = 'watcher_removal_in_progress'
 
   constructor() {
-    super('File watcher cannot start while the worktree is being removed')
+    super(WATCHER_REMOVAL_IN_PROGRESS_MESSAGE)
     this.name = 'WatcherRemovalInProgressError'
   }
 }
@@ -29,7 +33,7 @@ export class TerminalRemovalInProgressError extends Error {
   readonly code = 'terminal_removal_in_progress'
 
   constructor() {
-    super('Terminal cannot start while the worktree is being removed')
+    super(TERMINAL_REMOVAL_IN_PROGRESS_MESSAGE)
     this.name = 'TerminalRemovalInProgressError'
   }
 }

src/renderer/src/components/terminal-pane/pty-connection.test.ts

@@ -121,6 +121,7 @@ type StoreState = {
   ptyIdsByTabId?: Record<string, string[]>
   terminalLayoutsByTabId?: Record<string, TerminalLayoutSnapshot>
   unreadTerminalTabs?: Record<string, true>
+  deleteStateByWorktreeId?: Record<string, { isDeleting?: boolean; phase?: string }>
   worktreesByRepo: Record<
     string,
     {
@@ -783,6 +784,7 @@ describe('connectPanePty', () => {
         }
       },
       unreadTerminalTabs: {},
+      deleteStateByWorktreeId: {},
       worktreesByRepo: {
         repo1: [{ id: 'wt-1', repoId: 'repo1', path: '/tmp/wt-1', displayName: 'feat/notis' }]
       },
@@ -1024,6 +1026,91 @@ describe('connectPanePty', () => {
     expect(staleTracker.flags).toBe(0)
   })
 
+  // Why: deleting a worktree kills its PTYs for the filesystem teardown; the
+  // renderer must not race a doomed respawn into a directory main is deleting
+  // (main fences it with TerminalRemovalInProgressError and the pane is about to
+  // unmount). See docs — bad UI was the raw fence error flashing on the tab.
+  it('skips a fresh spawn while the pane worktree is being deleted', async () => {
+    const { connectPanePty } = await import('./pty-connection')
+    const transport = createMockTransport()
+    transportFactoryQueue.push(transport)
+    mockStoreState = {
+      ...mockStoreState,
+      deleteStateByWorktreeId: { 'wt-1': { isDeleting: true, phase: 'deleting' } }
+    }
+    // Why: a unique tab id keeps this pane's key clear of other tests' pendingSpawnByPaneKey entries so the connect deterministically fresh-spawns.
+    const deps = createDeps({ tabId: 'tab-removal-skip-spawn' })
+
+    connectPanePty(createPane(1) as never, createManager(1) as never, deps as never)
+    await flushAsyncTicks()
+
+    expect(transport.connect).not.toHaveBeenCalled()
+    expect(deps.onPtyErrorRef.current).not.toHaveBeenCalled()
+  })
+
+  it('fresh-spawns normally when the pane worktree is not being deleted', async () => {
+    const { connectPanePty } = await import('./pty-connection')
+    const transport = createMockTransport()
+    transportFactoryQueue.push(transport)
+    // Why: unique tab id → deterministic fresh spawn (mirrors the skip test's control).
+    const deps = createDeps({ tabId: 'tab-removal-control-spawn' })
+
+    connectPanePty(createPane(1) as never, createManager(1) as never, deps as never)
+    await flushAsyncTicks()
+
+    expect(transport.connect).toHaveBeenCalled()
+  })
+
+  // Why: a doomed pane (or a child pane whose parent worktree is being removed,
+  // which startFreshSpawn's own-worktree skip cannot see) can still race a spawn
+  // that main fences. reportError must swallow that fence so the tab never flashes
+  // the raw "Terminal cannot start while the worktree is being removed" banner.
+  it('swallows a worktree-removal fence error instead of surfacing it', async () => {
+    const { connectPanePty } = await import('./pty-connection')
+    const { TERMINAL_REMOVAL_IN_PROGRESS_MESSAGE } =
+      await import('../../../../shared/worktree-removal-fence-error')
+    const transport = createMockTransport()
+    const capturedOnError: { current: ((message: string) => void) | null } = { current: null }
+    transport.connect.mockImplementation(async ({ callbacks }: { callbacks: ConnectCallbacks }) => {
+      capturedOnError.current = callbacks.onError ?? null
+      return 'pty-1'
+    })
+    transportFactoryQueue.push(transport)
+    const deps = createDeps({ tabId: 'tab-fence-swallow' })
+
+    connectPanePty(createPane(1) as never, createManager(1) as never, deps as never)
+    await flushAsyncTicks()
+
+    // Why: assert the callback was captured before invoking — optional invocation
+    // would let this test false-pass (not.toHaveBeenCalled trivially true) if the
+    // transport onError wiring ever broke, exercising no suppression at all.
+    expect(capturedOnError.current).toBeTypeOf('function')
+    // Electron wraps the rejected ipcMain error with its own prefix; still swallowed.
+    capturedOnError.current!(
+      `Error invoking remote method 'pty:spawn': Error: ${TERMINAL_REMOVAL_IN_PROGRESS_MESSAGE}`
+    )
+    expect(deps.onPtyErrorRef.current).not.toHaveBeenCalled()
+  })
+
+  it('still surfaces non-fence spawn errors through the pane error sink', async () => {
+    const { connectPanePty } = await import('./pty-connection')
+    const transport = createMockTransport()
+    const capturedOnError: { current: ((message: string) => void) | null } = { current: null }
+    transport.connect.mockImplementation(async ({ callbacks }: { callbacks: ConnectCallbacks }) => {
+      capturedOnError.current = callbacks.onError ?? null
+      return 'pty-1'
+    })
+    transportFactoryQueue.push(transport)
+    const deps = createDeps({ tabId: 'tab-real-error-surface' })
+
+    connectPanePty(createPane(1) as never, createManager(1) as never, deps as never)
+    await flushAsyncTicks()
+
+    expect(capturedOnError.current).toBeTypeOf('function')
+    capturedOnError.current!('shell exited with code 1')
+    expect(deps.onPtyErrorRef.current).toHaveBeenCalledWith(1, 'shell exited with code 1')
+  })
+
   it('threads the resolved local project runtime into IPC terminal transport options', async () => {
     const { connectPanePty } = await import('./pty-connection')
     const transport = createMockTransport()

src/renderer/src/components/terminal-pane/pty-connection.ts

@@ -13,6 +13,7 @@ import { parseWorkspaceKey } from '../../../../shared/workspace-scope'
 import { TerminalKittyKeyboardModeTracker } from '../../../../shared/terminal-kitty-keyboard-mode-tracker'
 import { isRuntimeOwnedSshTargetId } from '../../../../shared/execution-host'
 import { createTerminalZeroDimensionsMessage } from '../../../../shared/terminal-zero-dimensions-diagnostic'
+import { isWorktreeRemovalFenceError } from '../../../../shared/worktree-removal-fence-error'
 import { parseTerminalOscColorQuery } from '../../../../shared/terminal-osc-color-reply'
 import {
   HIDDEN_STARTUP_RENDERER_QUERY_PENDING_CHARS,
@@ -4203,6 +4204,14 @@ export function connectPanePty(
       if (disposed) {
         return
       }
+      if (isWorktreeRemovalFenceError(message)) {
+        // Why: main fences a spawn/reattach whose worktree (or an overlapping
+        // parent/child root) is being deleted. That is expected teardown, not a
+        // user-facing failure — the pane unmounts once removal completes, so never
+        // surface the raw fence error. Covers the parent-removal-fences-child case
+        // that startFreshSpawn's own-worktree isDeleting skip cannot see.
+        return
+      }
       deps.onPtyErrorRef?.current?.(pane.id, message)
     }
 
@@ -4740,6 +4749,13 @@ export function connectPanePty(
       startupOverride?: PendingStartupCommand | null,
       options: FreshSpawnOptions = {}
     ): Promise<string | null> => {
+      if (useAppStore.getState().deleteStateByWorktreeId?.[deps.worktreeId]?.isDeleting) {
+        // Why: the worktree is being deleted; its PTYs were just killed for the
+        // filesystem teardown. A fresh shell must not spawn into a directory the
+        // removal is about to delete (main fences it anyway), and the pane is
+        // about to unmount — so skip the doomed respawn instead of racing it.
+        return Promise.resolve(null)
+      }
       clearPaneMode2031State()
       clearHiddenOutputRestoreState()
       // Why: a canceled old replay clear can preserve xterm's native

src/shared/worktree-removal-fence-error.test.ts

@@ -0,0 +1,25 @@
+import { describe, expect, it } from 'vitest'
+import {
+  TERMINAL_REMOVAL_IN_PROGRESS_MESSAGE,
+  WATCHER_REMOVAL_IN_PROGRESS_MESSAGE,
+  isWorktreeRemovalFenceError
+} from './worktree-removal-fence-error'
+
+describe('isWorktreeRemovalFenceError', () => {
+  it('recognizes the raw terminal and watcher fence messages', () => {
+    expect(isWorktreeRemovalFenceError(TERMINAL_REMOVAL_IN_PROGRESS_MESSAGE)).toBe(true)
+    expect(isWorktreeRemovalFenceError(WATCHER_REMOVAL_IN_PROGRESS_MESSAGE)).toBe(true)
+  })
+
+  it('recognizes the message after Electron IPC prefixes the reject', () => {
+    // Electron wraps a rejected ipcMain.handle error with its own prefix.
+    const wrapped = `Error invoking remote method 'pty:spawn': Error: ${TERMINAL_REMOVAL_IN_PROGRESS_MESSAGE}`
+    expect(isWorktreeRemovalFenceError(wrapped)).toBe(true)
+  })
+
+  it('does not match unrelated terminal errors', () => {
+    expect(isWorktreeRemovalFenceError('Failed to save terminal session state')).toBe(false)
+    expect(isWorktreeRemovalFenceError('shell exited with code 1')).toBe(false)
+    expect(isWorktreeRemovalFenceError('')).toBe(false)
+  })
+})

src/shared/worktree-removal-fence-error.ts

@@ -0,0 +1,18 @@
+// Shared between main (which throws these at the PTY/watcher install fence while
+// a worktree is being removed) and the renderer (which recognizes them so a
+// doomed pane never surfaces the fence as a user-facing terminal error).
+
+export const TERMINAL_REMOVAL_IN_PROGRESS_MESSAGE =
+  'Terminal cannot start while the worktree is being removed'
+
+export const WATCHER_REMOVAL_IN_PROGRESS_MESSAGE =
+  'File watcher cannot start while the worktree is being removed'
+
+// Why: both fence messages end with this tail. Matching the tail catches the
+// terminal and watcher variants even after Electron IPC prefixes the rejected
+// error with its own "Error invoking remote method ..." text.
+const REMOVAL_IN_PROGRESS_FENCE_TAIL = 'cannot start while the worktree is being removed'
+
+export function isWorktreeRemovalFenceError(message: string): boolean {
+  return message.includes(REMOVAL_IN_PROGRESS_FENCE_TAIL)
+}