From af96218cfbdcffd6026ca4710f3e41ed6d559628 Mon Sep 17 00:00:00 2001 From: Brennan Benson <79079362+brennanb2025@users.noreply.github.com> Date: Fri, 24 Jul 2026 21:39:58 -0700 Subject: [PATCH] fix(terminal): verify Windows PTY root identity before taskkill /T /F (#10484) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix(terminal): verify Windows PTY root identity before taskkill /T /F killWithDescendantSweep guarded its Windows tree kill with ownsRoot() alone, which is JS state only. node-pty's ConPTY exit watcher closes the last shell handle before it queues the JS exit callback, so Windows can recycle the PID while the session map still looks live — force-killing an unrelated process and its whole descendant tree. Walk the recycled PID's ancestry back to this process before taskkill: skip the sweep when the root is gone or resolves to a stranger, and keep the sweep when identity is unknown so #10004 orphan cleanup still runs. Also gate the local provider's ownsRoot on observed physical exit. * fix(terminal): dedupe the Windows root-identity scan, drop dead exit gate Review fixes on the PID-identity guard. The probe read the process table through a new uncached export, bypassing the reader that worktree teardown depends on: worktree-teardown.ts fans out 32-wide inside a 10s deadline, so a delete forked 32 powershell cold-starts (the churn windows-foreground-process-rows.ts:25-32 warns about, #6288/#6667). getFreshSnapshot() already guarantees a scan that starts after the request -- the exact property the bypass existed for -- and coalesces concurrent callers, so use it. Measured on the new test: 32 scans -> 1. The PhysicalExitTracker.hasExited gate could never fire. markExited() is only reached at local-pty-provider.ts:985/:1431, and both are followed synchronously by clearPtyState(), which deletes the ptyProcesses entry -- so ownsRoot's map check is already false whenever hasExited is true. Reverting it broke no test. Drop it and the shared getter it added; the identity probe already covers every ownsRoot caller from inside killWithDescendantSweep. Also point the Windows terminal-restart E2E job at the files that own this behavior, so a change to the new Windows-only module runs the one job that executes on a real Windows host. * docs(terminal): state what the Windows root probe actually proves The probe checks subtree membership, not root identity: a recycle that lands on another Orca descendant (another pane's shell, an agent CLI, a git.exe we spawned) still reads `own`, and that is not remote during teardown when Orca is itself allocating pids. It bounds the blast radius rather than closing the class. Say so at the type and at classifyWindowsTreeKillTarget, and name what a real close would need (a CreationDate baseline -- the analogue of the POSIX lstart check already used here -- or an inherited handle / Job Object). Also note why our own pid must classify `foreign`. * ci(windows): trigger the terminal-restart E2E on the shared snapshot reader The Windows root-identity probe now reads through getFreshSnapshot, so an edit to that module changes Windows teardown behavior without touching any path the job already watches. * test(terminal): guard the teardown probe against a reintroduced scan bypass The existing volume guard covers queryWindowsProcessRowsFresh directly, but the identity-probe cases all inject readRows, so nothing exercised the DEFAULT reader wiring -- a bypass reintroduced inside windows-pty-root-identity would have gone unnoticed. Drive verifyWindowsTreeKillTarget 32-wide through the real reader and assert one scan. Verified it fails at 32 when the bypass is put back. (cherry picked from commit 2653794c8251dc3aa5f9661ef573a08c1baeca15) --- .../windows-terminal-restart-e2e.yml | 4 + .../windows-foreground-process-rows.test.ts | 43 ++++ .../windows-foreground-process-rows.ts | 10 + src/main/pty-descendant-termination.test.ts | 100 +++++++++- src/main/pty-descendant-termination.ts | 24 ++- src/main/windows-pty-root-identity.test.ts | 188 ++++++++++++++++++ src/main/windows-pty-root-identity.ts | 137 +++++++++++++ 7 files changed, 500 insertions(+), 6 deletions(-) create mode 100644 src/main/windows-pty-root-identity.test.ts create mode 100644 src/main/windows-pty-root-identity.ts diff --git a/.github/workflows/windows-terminal-restart-e2e.yml b/.github/workflows/windows-terminal-restart-e2e.yml index 73543b5e07e..898f81bc8eb 100644 --- a/.github/workflows/windows-terminal-restart-e2e.yml +++ b/.github/workflows/windows-terminal-restart-e2e.yml @@ -28,9 +28,13 @@ on: - 'src/main/ipc/pty*.ts' - 'src/main/providers/**' - 'src/main/pty/**' + - 'src/main/pty-descendant-termination.ts' + - 'src/main/windows-process-tree-kill.ts' + - 'src/main/windows-pty-root-identity.ts' - 'src/preload/**' - 'src/renderer/src/components/terminal-pane/**' - 'src/renderer/src/lib/pane-manager/**' + - 'src/shared/process-table-snapshot.ts' - 'src/shared/pty-session-id-format.ts' workflow_dispatch: inputs: diff --git a/src/main/providers/windows-foreground-process-rows.test.ts b/src/main/providers/windows-foreground-process-rows.test.ts index 61482224e50..7c2b8a9b1b6 100644 --- a/src/main/providers/windows-foreground-process-rows.test.ts +++ b/src/main/providers/windows-foreground-process-rows.test.ts @@ -11,6 +11,7 @@ vi.mock('child_process', () => ({ execFile: execFileMock })) import { queryWindowsProcessDescendants, + queryWindowsProcessRowsFresh, resetWindowsProcessRowsSnapshotForTests } from './windows-foreground-process-rows' @@ -97,3 +98,45 @@ describe('windows foreground process rows spawn options', () => { expect(optionsForCommand('wmic')).toMatchObject({ windowsHide: true }) }) }) + +// Regression guard: the PID-identity probe that gates `taskkill /T /F` needs rows +// from a scan started after it asked, but worktree delete tears down PTYs 32-wide. +// Reading the table uncached would fork 32 powershell cold-starts per delete. +describe('queryWindowsProcessRowsFresh', () => { + let platform: PropertyDescriptor | undefined + + beforeEach(() => { + execFileMock.mockReset() + resetWindowsProcessRowsSnapshotForTests() + platform = Object.getOwnPropertyDescriptor(process, 'platform') + Object.defineProperty(process, 'platform', { configurable: true, value: 'win32' }) + execFileMock.mockImplementation((_cmd: string, _args, _opts, cb: ExecFileCallback) => { + cb(null, { stdout: POWERSHELL_ROWS_JSON, stderr: '' }) + }) + }) + + afterEach(() => { + if (platform) { + Object.defineProperty(process, 'platform', platform) + } + }) + + const powershellScanCount = (): number => + execFileMock.mock.calls.filter((call) => call[0] === 'powershell.exe').length + + it('collapses a burst of concurrent identity probes into one scan', async () => { + const rows = await Promise.all(Array.from({ length: 32 }, () => queryWindowsProcessRowsFresh())) + + expect(powershellScanCount()).toBe(1) + expect(rows[31]?.map((row) => row.pid)).toEqual([100, 200]) + }) + + it('never answers from the TTL cache, which can predate the recycle it detects', async () => { + await queryWindowsProcessDescendants(100) + expect(powershellScanCount()).toBe(1) + + await queryWindowsProcessRowsFresh() + + expect(powershellScanCount()).toBe(2) + }) +}) diff --git a/src/main/providers/windows-foreground-process-rows.ts b/src/main/providers/windows-foreground-process-rows.ts index 97aea1dccc1..7e4cdd2e48d 100644 --- a/src/main/providers/windows-foreground-process-rows.ts +++ b/src/main/providers/windows-foreground-process-rows.ts @@ -47,6 +47,16 @@ const windowsProcessRowsReader = createProcessTableSnapshotReader Date.now() }) +/** + * Rows from a scan that starts after this call. PID-identity checks in teardown + * must not reuse a cached row — it can predate the very recycle it detects — but + * they must still dedupe: a worktree delete tears down PTYs 32-wide, so a bypass + * would fork that many powershell cold-starts. Rejects when both probes fail. + */ +export function queryWindowsProcessRowsFresh(): Promise { + return windowsProcessRowsReader.getFreshSnapshot() +} + export async function queryWindowsProcessDescendants( rootPid: number, options: { fresh?: boolean } = {} diff --git a/src/main/pty-descendant-termination.test.ts b/src/main/pty-descendant-termination.test.ts index 3955e0329d3..e17e186280f 100644 --- a/src/main/pty-descendant-termination.test.ts +++ b/src/main/pty-descendant-termination.test.ts @@ -405,7 +405,10 @@ describe('killWithDescendantSweep', () => { platform: 'win32', killWindowsTree, sendSignal, - readTable + readTable, + // Pin identity so the assertion holds wherever the suite runs, including a + // real Windows host where the default probe would query this fake pid. + verifyTreeKillTarget: async () => 'own' }) expect(killWindowsTree).toHaveBeenCalledWith(4242) expect(killRoot).toHaveBeenCalledOnce() @@ -428,6 +431,98 @@ describe('killWithDescendantSweep', () => { expect(killRoot).toHaveBeenCalledOnce() }) + it('on Windows skips taskkill when the root pid was recycled by a stranger', async () => { + const killWindowsTree = vi.fn(async () => {}) + const killRoot = vi.fn() + await killWithDescendantSweep(4242, killRoot, { + platform: 'win32', + killWindowsTree, + verifyTreeKillTarget: async () => 'foreign' + }) + expect(killWindowsTree).not.toHaveBeenCalled() + expect(killRoot).toHaveBeenCalledOnce() + }) + + it('on Windows skips taskkill when the root pid is already gone', async () => { + const killWindowsTree = vi.fn(async () => {}) + const killRoot = vi.fn() + await killWithDescendantSweep(4242, killRoot, { + platform: 'win32', + killWindowsTree, + verifyTreeKillTarget: async () => 'absent' + }) + expect(killWindowsTree).not.toHaveBeenCalled() + expect(killRoot).toHaveBeenCalledOnce() + }) + + it('on Windows taskkills a root the OS confirms is still ours', async () => { + const killWindowsTree = vi.fn(async () => {}) + const killRoot = vi.fn() + await killWithDescendantSweep(4242, killRoot, { + platform: 'win32', + killWindowsTree, + verifyTreeKillTarget: async () => 'own' + }) + expect(killWindowsTree).toHaveBeenCalledWith(4242) + expect(killRoot).toHaveBeenCalledOnce() + }) + + it('on Windows still taskkills when identity is unknown, keeping orphan cleanup', async () => { + const killWindowsTree = vi.fn(async () => {}) + const killRoot = vi.fn() + await killWithDescendantSweep(4242, killRoot, { + platform: 'win32', + killWindowsTree, + verifyTreeKillTarget: async () => 'unknown' + }) + expect(killWindowsTree).toHaveBeenCalledWith(4242) + expect(killRoot).toHaveBeenCalledOnce() + }) + + it('on Windows still taskkills when the identity probe throws', async () => { + const killWindowsTree = vi.fn(async () => {}) + const killRoot = vi.fn() + await killWithDescendantSweep(4242, killRoot, { + platform: 'win32', + killWindowsTree, + verifyTreeKillTarget: async () => { + throw new Error('probe exploded') + } + }) + expect(killWindowsTree).toHaveBeenCalledWith(4242) + expect(killRoot).toHaveBeenCalledOnce() + }) + + it('on Windows re-checks ownership lost while the identity probe ran', async () => { + const killWindowsTree = vi.fn(async () => {}) + const killRoot = vi.fn() + let alive = true + await killWithDescendantSweep(4242, killRoot, { + platform: 'win32', + killWindowsTree, + ownsRoot: () => alive, + verifyTreeKillTarget: async () => { + alive = false + return 'own' + } + }) + expect(killWindowsTree).not.toHaveBeenCalled() + expect(killRoot).toHaveBeenCalledOnce() + }) + + it('on Windows does not probe identity once ownership is already lost', async () => { + const verifyTreeKillTarget = vi.fn(async () => 'own' as const) + const killRoot = vi.fn() + await killWithDescendantSweep(4242, killRoot, { + platform: 'win32', + killWindowsTree: vi.fn(async () => {}), + ownsRoot: () => false, + verifyTreeKillTarget + }) + expect(verifyTreeKillTarget).not.toHaveBeenCalled() + expect(killRoot).toHaveBeenCalledOnce() + }) + it('on Windows still kills the root when taskkill fails', async () => { const killWindowsTree = vi.fn(async () => { throw new Error('taskkill failed') @@ -436,7 +531,8 @@ describe('killWithDescendantSweep', () => { await expect( killWithDescendantSweep(99, killRoot, { platform: 'win32', - killWindowsTree + killWindowsTree, + verifyTreeKillTarget: async () => 'own' }) ).resolves.toBeUndefined() expect(killRoot).toHaveBeenCalledOnce() diff --git a/src/main/pty-descendant-termination.ts b/src/main/pty-descendant-termination.ts index 6bd162553c5..db1f7638ae1 100644 --- a/src/main/pty-descendant-termination.ts +++ b/src/main/pty-descendant-termination.ts @@ -1,5 +1,9 @@ import { execFile } from 'node:child_process' import { terminateWindowsProcessTree, type WindowsTreeKiller } from './windows-process-tree-kill' +import { + verifyWindowsTreeKillTarget, + type WindowsTreeKillTarget +} from './windows-pty-root-identity' export const DESCENDANT_KILL_GRACE_MS = 2_000 export const DESCENDANT_SNAPSHOT_TIMEOUT_MS = 1_000 @@ -226,13 +230,16 @@ type KillSweepDeps = SnapshotDeps & ownsRoot?: () => boolean /** Injectable Windows tree killer (defaults to taskkill /T /F). */ killWindowsTree?: WindowsTreeKiller + /** Injectable Windows root-identity probe (defaults to a live process query). */ + verifyTreeKillTarget?: (rootPid: number) => Promise } /** * 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. + * 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. * 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. @@ -246,9 +253,18 @@ export async function killWithDescendantSweep( if (platform === 'win32') { try { if ((deps.ownsRoot?.() ?? true) && Number.isInteger(rootPid) && rootPid > 0) { - const killTree = deps.killWindowsTree ?? terminateWindowsProcessTree - // Why: taskkill may race an already-exited tree; never block killRoot on that. - await killTree(rootPid).catch(() => {}) + // Why: ownsRoot() is JS state only, and node-pty's ConPTY exit watcher closes + // the last shell handle before it queues the JS exit callback — Windows may + // already have recycled this PID while the map still looks live. taskkill /T /F + // on a recycled PID force-kills an unrelated tree, so demand OS identity first. + 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)) { + const killTree = deps.killWindowsTree ?? terminateWindowsProcessTree + // Why: taskkill may race an already-exited tree; never block killRoot on that. + await killTree(rootPid).catch(() => {}) + } } } finally { killRoot() diff --git a/src/main/windows-pty-root-identity.test.ts b/src/main/windows-pty-root-identity.test.ts new file mode 100644 index 00000000000..d6852b91d98 --- /dev/null +++ b/src/main/windows-pty-root-identity.test.ts @@ -0,0 +1,188 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' + +const { execFileMock, powershellScanCount } = vi.hoisted(() => ({ + execFileMock: vi.fn(), + powershellScanCount: { value: 0 } +})) + +vi.mock('child_process', () => ({ execFile: execFileMock })) + +import { resetWindowsProcessRowsSnapshotForTests } from './providers/windows-foreground-process-rows' +import { + classifyWindowsTreeKillTarget, + verifyWindowsTreeKillTarget, + WINDOWS_ROOT_IDENTITY_TIMEOUT_MS +} from './windows-pty-root-identity' + +const ORCA_PID = 5000 + +function link(pid: number, ppid: number): { pid: number; ppid: number } { + return { pid, ppid } +} + +/** Windows: services.exe → svchost.exe, a chain that never reaches Orca. */ +const SYSTEM_CHAIN = [link(4, 0), link(700, 4), link(900, 700)] + +describe('classifyWindowsTreeKillTarget', () => { + it('accepts a ConPTY shell spawned directly by this process', () => { + const rows = [...SYSTEM_CHAIN, link(ORCA_PID, 900), link(4242, ORCA_PID)] + expect(classifyWindowsTreeKillTarget(4242, rows, ORCA_PID)).toBe('own') + }) + + it('accepts a winpty shell reached through the winpty-agent hop', () => { + // node-pty falls back to winpty below Windows build 18309, so the shell's + // parent is winpty-agent.exe rather than Orca itself. + const rows = [link(ORCA_PID, 900), link(6100, ORCA_PID), link(4242, 6100)] + 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') + }) + + it('reports an exited root as absent rather than sweeping a stale pid', () => { + const rows = [...SYSTEM_CHAIN, link(ORCA_PID, 900)] + expect(classifyWindowsTreeKillTarget(4242, rows, ORCA_PID)).toBe('absent') + }) + + it('rejects a recycled pid whose parent has itself already exited', () => { + // The orphan's ppid names a vacated pid, so the chain dead-ends away from us. + const rows = [link(ORCA_PID, 900), link(4242, 31337)] + expect(classifyWindowsTreeKillTarget(4242, rows, ORCA_PID)).toBe('foreign') + }) + + it('rejects a chain longer than the ConPTY/winpty depth even if Orca is above it', () => { + const rows = [ + link(ORCA_PID, 900), + link(10, ORCA_PID), + link(11, 10), + link(12, 11), + link(13, 12), + link(4242, 13) + ] + expect(classifyWindowsTreeKillTarget(4242, rows, ORCA_PID)).toBe('foreign') + }) + + it('never force-kills this process tree when the root pid is our own pid', () => { + const rows = [link(ORCA_PID, 900)] + expect(classifyWindowsTreeKillTarget(ORCA_PID, rows, ORCA_PID)).toBe('foreign') + }) + + it.each([0, -1, 1.5, Number.NaN])('rejects the invalid root pid %s', (rootPid) => { + expect(classifyWindowsTreeKillTarget(rootPid, [link(4242, ORCA_PID)], ORCA_PID)).toBe('foreign') + }) + + it('treats duplicate rows for the root as unknown, not as ownership', () => { + const rows = [link(4242, ORCA_PID), link(4242, 900)] + expect(classifyWindowsTreeKillTarget(4242, rows, ORCA_PID)).toBe('unknown') + }) + + it('treats a cyclic table as unknown instead of looping', () => { + const rows = [link(4242, 4243), link(4243, 4242)] + expect(classifyWindowsTreeKillTarget(4242, rows, ORCA_PID)).toBe('unknown') + }) +}) + +describe('verifyWindowsTreeKillTarget', () => { + it('classifies a live win32 root against the fresh process table', async () => { + const readRows = vi.fn().mockResolvedValue([link(4242, ORCA_PID)]) + await expect( + verifyWindowsTreeKillTarget(4242, { readRows, ownerPid: ORCA_PID, platform: 'win32' }) + ).resolves.toBe('own') + expect(readRows).toHaveBeenCalledOnce() + }) + + it('detects the recycled-pid case that taskkill /T /F must not touch', async () => { + const readRows = vi.fn().mockResolvedValue([link(4242, 900), link(900, 4)]) + await expect( + verifyWindowsTreeKillTarget(4242, { readRows, ownerPid: ORCA_PID, platform: 'win32' }) + ).resolves.toBe('foreign') + }) + + it('falls open to 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 () => { + 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 () => { + const readRows = vi.fn(() => { + throw new Error('spawn EPERM') + }) + await expect( + verifyWindowsTreeKillTarget(4242, { readRows, ownerPid: ORCA_PID, platform: 'win32' }) + ).resolves.toBe('unknown') + }) + + it('does not let a wedged process query block teardown past the deadline', async () => { + vi.useFakeTimers() + try { + const readRows = vi.fn(() => new Promise(() => {})) + const pending = verifyWindowsTreeKillTarget(4242, { + readRows, + ownerPid: ORCA_PID, + platform: 'win32' + }) + await vi.advanceTimersByTimeAsync(WINDOWS_ROOT_IDENTITY_TIMEOUT_MS) + await expect(pending).resolves.toBe('unknown') + } finally { + vi.useRealTimers() + } + }) + + it('skips the probe off Windows so POSIX teardown keeps its own guards', async () => { + const readRows = vi.fn() + await expect( + verifyWindowsTreeKillTarget(4242, { readRows, ownerPid: ORCA_PID, platform: 'darwin' }) + ).resolves.toBe('unknown') + expect(readRows).not.toHaveBeenCalled() + }) +}) + +// Regression guard on the DEFAULT reader, which the cases above bypass by +// injecting readRows: worktree delete tears down PTYs 32-wide, so a probe that +// reads the table uncached forks 32 powershell cold-starts per delete — the +// churn #6288/#6667 fixed for POSIX. Exercises the real wiring, not a fake. +describe('verifyWindowsTreeKillTarget scan volume', () => { + const ROWS_JSON = JSON.stringify([ + { ProcessId: ORCA_PID, ParentProcessId: 900, Name: 'orca.exe', CommandLine: 'orca.exe' }, + { ProcessId: 4242, ParentProcessId: ORCA_PID, Name: 'pwsh.exe', CommandLine: 'pwsh.exe' } + ]) + + beforeEach(() => { + execFileMock.mockReset() + powershellScanCount.value = 0 + resetWindowsProcessRowsSnapshotForTests() + execFileMock.mockImplementation((cmd: string, ..._rest: unknown[]) => { + const cb = _rest.at(-1) as (e: unknown, r: { stdout: string; stderr: string }) => void + if (cmd === 'powershell.exe') { + powershellScanCount.value += 1 + } + cb(null, { stdout: ROWS_JSON, stderr: '' }) + }) + }) + + afterEach(() => { + resetWindowsProcessRowsSnapshotForTests() + }) + + it('collapses a 32-wide teardown burst into a single process-table scan', async () => { + const verdicts = await Promise.all( + Array.from({ length: 32 }, () => + verifyWindowsTreeKillTarget(4242, { ownerPid: ORCA_PID, platform: 'win32' }) + ) + ) + + expect(powershellScanCount.value).toBe(1) + expect(new Set(verdicts)).toEqual(new Set(['own'])) + }) +}) diff --git a/src/main/windows-pty-root-identity.ts b/src/main/windows-pty-root-identity.ts new file mode 100644 index 00000000000..e8cc16c45a0 --- /dev/null +++ b/src/main/windows-pty-root-identity.ts @@ -0,0 +1,137 @@ +import { queryWindowsProcessRowsFresh } from './providers/windows-foreground-process-rows' + +/** + * Whether a PID still sits inside this process's own subtree. Note this is + * 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. + * - `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. + */ +export type WindowsTreeKillTarget = 'own' | 'absent' | 'foreign' | 'unknown' + +export const WINDOWS_ROOT_IDENTITY_TIMEOUT_MS = 3_000 + +// Why: ConPTY spawns the shell directly from this process (1 hop). node-pty falls +// back to winpty below Windows build 18309, which adds a winpty-agent.exe hop. +const MAX_ANCESTOR_HOPS = 4 + +type ProcessLink = { pid: number; ppid: number } + +export type WindowsProcessLinkReader = () => Promise + +/** + * Classify `rootPid` by walking its ancestry back to `ownerPid`. A recycled PID + * usually belongs to an unrelated process whose chain never passes through Orca, + * 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). + */ +export function classifyWindowsTreeKillTarget( + rootPid: number, + rows: readonly ProcessLink[], + ownerPid: number +): WindowsTreeKillTarget { + // Why: our own pid is never a PTY root, so reading it here means the pid is + // corrupt. `foreign` is the refusing verdict, which is what that must get — + // `taskkill /T /F` on ourselves would take Orca and every pane down with it. + if (!Number.isInteger(rootPid) || rootPid <= 0 || rootPid === ownerPid) { + return 'foreign' + } + const parentByPid = new Map() + for (const row of rows) { + // Duplicate PID rows make ancestry ambiguous, so they never prove ownership. + parentByPid.set(row.pid, parentByPid.has(row.pid) ? null : row.ppid) + } + if (!parentByPid.has(rootPid)) { + return 'absent' + } + + const visited = new Set([rootPid]) + let current = rootPid + for (let hop = 0; hop < MAX_ANCESTOR_HOPS; hop += 1) { + const parent = parentByPid.get(current) + if (parent === null) { + return 'unknown' + } + // Why: a chain that dead-ends elsewhere is positive evidence the PID is not + // ours. Only our own live PID can terminate a chain that started at our child. + if (parent === undefined) { + return 'foreign' + } + if (parent === ownerPid) { + return 'own' + } + if (visited.has(parent)) { + // An inconsistent table (mid-scan PID reuse) proves nothing either way. + return 'unknown' + } + visited.add(parent) + current = parent + } + return 'foreign' +} + +function readLinksBeforeDeadline( + readRows: WindowsProcessLinkReader, + timeoutMs: number +): Promise { + return new Promise((resolve) => { + let settled = false + const finish = (rows: readonly ProcessLink[] | null): void => { + if (settled) { + return + } + settled = true + clearTimeout(timer) + resolve(rows) + } + const timer = setTimeout(() => finish(null), timeoutMs) + timer.unref?.() + try { + void readRows().then( + (rows) => finish(rows), + () => finish(null) + ) + } catch { + finish(null) + } + }) +} + +/** + * 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. + */ +export async function verifyWindowsTreeKillTarget( + rootPid: number, + deps: { + readRows?: WindowsProcessLinkReader + ownerPid?: number + platform?: NodeJS.Platform + timeoutMs?: number + } = {} +): Promise { + // Why: the CIM/wmic probes exist only on Windows, so there is nothing to verify + // off-platform — including suites that drive the win32 branch from POSIX. + if ((deps.platform ?? process.platform) !== 'win32') { + return 'unknown' + } + const rows = await readLinksBeforeDeadline( + deps.readRows ?? queryWindowsProcessRowsFresh, + deps.timeoutMs ?? WINDOWS_ROOT_IDENTITY_TIMEOUT_MS + ) + if (!rows) { + return 'unknown' + } + return classifyWindowsTreeKillTarget(rootPid, rows, deps.ownerPid ?? process.pid) +}