From c3620f0954ae283b3b71b295a241f6a9596af6aa Mon Sep 17 00:00:00 2001
From: Neil <4138956+nwparker@users.noreply.github.com>
Date: Wed, 22 Jul 2026 22:55:35 -0700
Subject: [PATCH] fix(worktree): verify exited PTYs before blocking deletion
 (#10106)

---
 src/main/runtime/worktree-teardown.test.ts | 47 ++++++++++++++++++++++
 src/main/runtime/worktree-teardown.ts      | 35 +++++++++++++---
 2 files changed, 77 insertions(+), 5 deletions(-)

diff --git a/src/main/runtime/worktree-teardown.test.ts b/src/main/runtime/worktree-teardown.test.ts
index 1025c6be0..606b62d06 100644
--- a/src/main/runtime/worktree-teardown.test.ts
+++ b/src/main/runtime/worktree-teardown.test.ts
@@ -375,6 +375,53 @@ describe('killAllProcessesForWorktree', () => {
     expect(localProvider.shutdown).toHaveBeenCalledTimes(1)
   })
 
+  it('accepts a failed Windows stop when a fresh inventory proves the PTY exited', async () => {
+    const worktreeId = 'repo-1::C:/Users/User/orca/workspaces/repo/feature'
+    const ptyId = `${worktreeId}@@windows-pty`
+    const stopTerminalsForWorktree = vi.fn(
+      async (
+        _worktreeId: string,
+        options: {
+          stopPty: (
+            ptyId: string,
+            stop: () => boolean
+          ) => Promise<{ stopped: boolean; owner: boolean }>
+        }
+      ) => ({
+        stopped: (await options.stopPty(ptyId, () => false)).owner ? 1 : 0
+      })
+    )
+    const runtime = {
+      stopTerminalsForWorktree
+    } as unknown as Parameters<typeof killAllProcessesForWorktree>[1]['runtime']
+    let inventoryCount = 0
+    const localProvider = createProviderStub(async () => {
+      inventoryCount += 1
+      return inventoryCount === 1
+        ? [{ id: ptyId, cwd: 'C:/Users/User/orca/workspaces/repo/feature', title: 'shell' }]
+        : []
+    })
+    ;(localProvider.shutdown as unknown as ReturnType<typeof vi.fn>).mockRejectedValue(
+      new Error(`Session not found: ${ptyId}`)
+    )
+    listRegisteredPtysMock.mockReturnValue([
+      { ptyId, worktreeId, sessionId: null, paneKey: null, pid: 100 }
+    ])
+
+    await expect(
+      killAllProcessesForWorktree(worktreeId, {
+        runtime,
+        localProvider,
+        requirePhysicalStop: true
+      })
+    ).resolves.toEqual({
+      runtimeStopped: 0,
+      providerStopped: 0,
+      registryStopped: 0
+    })
+    expect(localProvider.listProcesses).toHaveBeenCalledTimes(2)
+  })
+
   it('keeps duplicate sweeps behind the runtime physical-stop promise', async () => {
     let releasePhysicalStop: () => void = () => undefined
     const physicalStop = new Promise<boolean>((resolve) => {
diff --git a/src/main/runtime/worktree-teardown.ts b/src/main/runtime/worktree-teardown.ts
index 8dfe63a9f..db6c0f5d6 100644
--- a/src/main/runtime/worktree-teardown.ts
+++ b/src/main/runtime/worktree-teardown.ts
@@ -26,9 +26,8 @@ export type WorktreeTeardownResult = {
 
 export const WORKTREE_PROCESS_SWEEP_TIMEOUT_MS = 10_000
 
-// Why: margin so a bounded daemon RPC rejects BEFORE the sweep deadline and its
-// rejection can propagate — otherwise the outer deadline wins with a confusing
-// "Timed out waiting for physical PTY teardown" instead of the accurate stop failure.
+// Why: reserve time after bounded stop RPCs to recheck whether a reported
+// failure actually left a live PTY before the outer sweep deadline.
 export const WORKTREE_TEARDOWN_RPC_MARGIN_MS = 500
 
 // Absolute deadline (epoch ms) threaded into provider RPCs on the destructive
@@ -150,15 +149,41 @@ export async function killAllProcessesForWorktree(
   result.providerStopped = providerStopped
   result.registryStopped = registryStopped
   if (deps.requirePhysicalStop) {
-    const stops = await Promise.all(stopAttempts.values())
-    if (stops.some((stopped) => !stopped)) {
+    const stopResults = await Promise.all(
+      [...stopAttempts].map(async ([ptyId, stopped]) => [ptyId, await stopped] as const)
+    )
+    const failedPtyIds = stopResults.filter(([, stopped]) => !stopped).map(([ptyId]) => ptyId)
+    const failedPtysExited =
+      failedPtyIds.length === 0 ||
+      (await verifyFailedPtysExited(failedPtyIds, deps.localProvider, deadline))
+    if (!failedPtysExited) {
       throw new Error(`Failed to physically stop every PTY for worktree: ${worktreeId}`)
     }
+    for (const ptyId of failedPtyIds) {
+      clearStoppedPtyState(ptyId, deps.onPtyStopped)
+    }
   }
 
   return result
 }
 
+async function verifyFailedPtysExited(
+  failedPtyIds: readonly string[],
+  provider: IPtyProvider,
+  deadline: number
+): Promise<boolean> {
+  const sessions = await settleBeforeDeadline(
+    () => provider.listProcesses({ deadlineMs: deadline }),
+    null,
+    deadline
+  ).catch(() => null)
+  if (!sessions) {
+    return false
+  }
+  const livePtyIds = new Set(sessions.map((session) => session.id))
+  return failedPtyIds.every((ptyId) => !livePtyIds.has(ptyId))
+}
+
 async function settleBeforeDeadline<T>(
   run: () => Promise<T>,
   fallback: T,