fix(windows): fail closed on unknown PTY identity (#10674)

* fix(windows): fail closed on unknown PTY identity

* Gate Windows tree-kill on PTY identity verification

Only taskkill when the identity probe returns `own`; skip tree-kill for
`unknown`/`foreign`/`absent` to avoid terminating unrelated processes.
Clarifies the fail-closed behavior across agent and plain shell teardown
paths, matching the POSIX descendant-snapshot discipline.

src/main/daemon/terminal-session-teardown.ts

@@ -49,9 +49,10 @@ export class TerminalSessionTeardown {
    * reap orphaned children (node-pty `useConptyDll` skips the console-process reap), so a
    * live `pnpm i`/`node` survives shell exit, keeps the ConPTY console non-empty, and holds
    * the worktree cwd — failing destructive worktree removal with "Failed to physically stop
-   * every PTY". taskkill /T /F the tree first so physical exit becomes verifiable. Mirrors
-   * the agent path (#10004/#10100). POSIX shells already reach their child pgroup on
-   * forceKill, so they stay on the plain force-kill path.
+   * every PTY". Tree-kill only when the OS identity probe returns `own`; `unknown`/`foreign`/
+   * `absent` skip taskkill and rely on root close alone. Mirrors the agent path
+   * (#10004/#10100). POSIX shells already reach their child pgroup on forceKill, so they
+   * stay on the plain force-kill path.
    */
   private async forceKillPlainShellSession(sessionId: string, session: Session): Promise<void> {
     if (process.platform === 'win32') {

src/main/providers/local-pty-provider.ts

@@ -1123,16 +1123,17 @@ export class LocalPtyProvider implements IPtyProvider {
       this.requestTrackedPtyShutdown(id, proc, operation.immediate)
     }
     if (ptyAgentSessionIds.has(id)) {
-      // Why: POSIX needs a pre-kill descendant snapshot; Windows uses taskkill /T so
-      // agent/MCP orphans cannot hold the worktree cwd after shell stop (#10004).
+      // Why: POSIX needs a pre-kill descendant snapshot; Windows tree-kills only when the
+      // identity probe returns `own` so agent/MCP orphans cannot hold the worktree cwd
+      // (#10004). `unknown`/`foreign`/`absent` skip taskkill and rely on root close alone.
       await killWithDescendantSweep(proc.pid, signalRoot, {
         ownsRoot: () => ptyProcesses.get(id) === proc
       })
     } else if (process.platform === 'win32' && operation.immediate) {
       // Why: a plain shell's ConPTY teardown doesn't reap orphaned children (useConptyDll
       // skips the console reap), so a live `pnpm i`/`node` keeps the ConPTY console alive and
-      // holds the worktree cwd, failing destructive removal. taskkill /T /F clears the tree so
-      // physical stop is verifiable. POSIX shells reach their child pgroup on forceKill (#10004).
+      // holds the worktree cwd. Tree kill runs only when the OS identity probe returns `own`;
+      // otherwise root close alone, and detached children may block physical stop (#10004).
       await killWithDescendantSweep(proc.pid, signalRoot, {
         ownsRoot: () => ptyProcesses.get(id) === proc
       })

src/main/pty-descendant-termination.test.ts

@@ -467,7 +467,7 @@ describe('killWithDescendantSweep', () => {
     expect(killRoot).toHaveBeenCalledOnce()
   })
 
-  it('on Windows still taskkills when identity is unknown, keeping orphan cleanup', async () => {
+  it('on Windows skips taskkill when identity is unknown', async () => {
     const killWindowsTree = vi.fn(async () => {})
     const killRoot = vi.fn()
     await killWithDescendantSweep(4242, killRoot, {
@@ -475,11 +475,11 @@ describe('killWithDescendantSweep', () => {
       killWindowsTree,
       verifyTreeKillTarget: async () => 'unknown'
     })
-    expect(killWindowsTree).toHaveBeenCalledWith(4242)
+    expect(killWindowsTree).not.toHaveBeenCalled()
     expect(killRoot).toHaveBeenCalledOnce()
   })
 
-  it('on Windows still taskkills when the identity probe throws', async () => {
+  it('on Windows skips taskkill when the identity probe throws', async () => {
     const killWindowsTree = vi.fn(async () => {})
     const killRoot = vi.fn()
     await killWithDescendantSweep(4242, killRoot, {
@@ -489,7 +489,7 @@ describe('killWithDescendantSweep', () => {
         throw new Error('probe exploded')
       }
     })
-    expect(killWindowsTree).toHaveBeenCalledWith(4242)
+    expect(killWindowsTree).not.toHaveBeenCalled()
     expect(killRoot).toHaveBeenCalledOnce()
   })
 

src/main/pty-descendant-termination.ts

@@ -204,7 +204,7 @@ type SnapshotDeps = {
  * signalled: once the root dies, surviving descendants reparent to pid 1 and
  * can no longer be found by a ppid walk. Resolves null (never rejects) on
  * Windows, ps failure, or timeout — callers then degrade to shell-only kill
- * on POSIX, or Windows `taskkill /T` via killWithDescendantSweep.
+ * on POSIX, or identity-gated Windows `taskkill /T` via killWithDescendantSweep.
  */
 export async function captureDescendantSnapshot(
   rootPid: number,
@@ -237,9 +237,9 @@ type KillSweepDeps = SnapshotDeps &
 /**
  * Standard agent-session kill sequencing.
  * - POSIX: snapshot the descendant tree, signal members, then killRoot.
- * - Windows: taskkill /T /F walks the ConPTY tree (shell → agent → MCP) so
- *   worktree teardown is not blocked by orphans holding the cwd handle, but only
- *   after the OS confirms the root PID is still ours and not a recycled stranger.
+ * - Windows: taskkill /T /F walks the ConPTY tree only when the identity probe
+ *   returns `own` (and ownsRoot still holds). `unknown`/`foreign`/`absent` skip
+ *   tree kill; killRoot always runs. Detached children may survive probe failure.
  * Callers must not signal the root before this runs on POSIX — a dead root's
  * descendants reparent to pid 1 and become unfindable. Snapshot failure
  * degrades to killRoot alone on POSIX.
@@ -260,7 +260,7 @@ export async function killWithDescendantSweep(
         const verify = deps.verifyTreeKillTarget ?? verifyWindowsTreeKillTarget
         const target = await verify(rootPid).catch((): WindowsTreeKillTarget => 'unknown')
         // Re-check ownership: the identity query awaits, so exit can land meanwhile.
-        if (target !== 'absent' && target !== 'foreign' && (deps.ownsRoot?.() ?? true)) {
+        if (target === 'own' && (deps.ownsRoot?.() ?? true)) {
           const killTree = deps.killWindowsTree ?? terminateWindowsProcessTree
           // Why: taskkill may race an already-exited tree; never block killRoot on that.
           await killTree(rootPid).catch(() => {})

src/main/windows-pty-root-identity.test.ts

@@ -36,6 +36,14 @@ describe('classifyWindowsTreeKillTarget', () => {
     expect(classifyWindowsTreeKillTarget(4242, rows, ORCA_PID)).toBe('own')
   })
 
+  it('documents that a recycled PID under another Orca pane still classifies as own', () => {
+    // Dead PTY root 4242 recycled as a tool under a different pane's agent tree.
+    // Ancestry still reaches us, so taskkill is allowed — wrong process, own tree.
+    // Closing this needs spawn-time CreationDate / Job Object (#10680).
+    const rows = [link(ORCA_PID, 900), link(7000, ORCA_PID), link(7100, 7000), link(4242, 7100)]
+    expect(classifyWindowsTreeKillTarget(4242, rows, ORCA_PID)).toBe('own')
+  })
+
   it('rejects a recycled pid whose ancestry never reaches this process', () => {
     const rows = [...SYSTEM_CHAIN, link(ORCA_PID, 900), link(4242, 900)]
     expect(classifyWindowsTreeKillTarget(4242, rows, ORCA_PID)).toBe('foreign')
@@ -100,21 +108,21 @@ describe('verifyWindowsTreeKillTarget', () => {
     ).resolves.toBe('foreign')
   })
 
-  it('falls open to unknown when both Windows process probes are unavailable', async () => {
+  it('returns unknown when both Windows process probes are unavailable', async () => {
     const readRows = vi.fn().mockResolvedValue(null)
     await expect(
       verifyWindowsTreeKillTarget(4242, { readRows, ownerPid: ORCA_PID, platform: 'win32' })
     ).resolves.toBe('unknown')
   })
 
-  it('falls open to unknown when the process query rejects', async () => {
+  it('returns unknown when the process query rejects', async () => {
     const readRows = vi.fn().mockRejectedValue(new Error('powershell missing'))
     await expect(
       verifyWindowsTreeKillTarget(4242, { readRows, ownerPid: ORCA_PID, platform: 'win32' })
     ).resolves.toBe('unknown')
   })
 
-  it('falls open to unknown when the query throws synchronously', async () => {
+  it('returns unknown when the query throws synchronously', async () => {
     const readRows = vi.fn(() => {
       throw new Error('spawn EPERM')
     })

src/main/windows-pty-root-identity.ts

@@ -5,10 +5,10 @@ import { queryWindowsProcessRowsFresh } from './providers/windows-foreground-pro
  * subtree membership, not root identity: a recycled PID that lands on any other
  * Orca descendant also reads `own`. It bounds the blast radius of a bad
  * `taskkill /T /F` to our own tree; it does not prove we spawned this PTY.
- * - `own`: ancestry reaches us, so the tree is ours to kill.
+ * - `own`: ancestry reaches us, so the tree is eligible for guarded teardown.
  * - `absent`: the PID is gone; `taskkill` would no-op anyway.
  * - `foreign`: the PID resolves to a process we did not start (PID recycle).
- * - `unknown`: no usable evidence; callers keep their prior behavior.
+ * - `unknown`: no usable evidence; callers must not force-kill the tree.
  */
 export type WindowsTreeKillTarget = 'own' | 'absent' | 'foreign' | 'unknown'
 
@@ -28,11 +28,12 @@ export type WindowsProcessLinkReader = () => Promise<readonly ProcessLink[] | nu
  * so it resolves `foreign` and must never reach `taskkill /T /F`. Ambiguous
  * tables resolve `unknown` because ambiguity is not evidence of ownership.
  *
- * Known limit: a recycle that lands on one of our OWN descendants — another
- * pane's shell, an agent CLI, a `git.exe` we spawned — still reads `own`. That
- * is not remote during teardown, when Orca is itself the process allocating
- * pids. Closing it needs real identity (a `Win32_Process.CreationDate` baseline,
- * the analogue of the POSIX `lstart` check, or an inherited handle/Job Object).
+ * Known limit (#10680): a recycle that lands on one of our OWN descendants —
+ * another pane's shell, an agent CLI, a `git.exe` we spawned — still reads
+ * `own`. That is not remote during teardown, when Orca is itself the process
+ * allocating pids. Closing it needs real identity (a `Win32_Process.CreationDate`
+ * baseline, the analogue of the POSIX `lstart` check, or an inherited handle /
+ * Job Object).
  */
 export function classifyWindowsTreeKillTarget(
   rootPid: number,
@@ -110,7 +111,7 @@ function readLinksBeforeDeadline(
  * Verify that `rootPid` still identifies the PTY root this process started,
  * before a `taskkill /T /F` that would otherwise force-kill a recycled PID and
  * its whole descendant tree. Never rejects: an unavailable or slow process query
- * resolves `unknown` so teardown keeps its pre-guard behavior.
+ * resolves `unknown`, which is not permission to force-kill the tree.
  */
 export async function verifyWindowsTreeKillTarget(
   rootPid: number,