fix(subprocess): close runner stdio handles

packages/subprocess/subprocess-local/tests/spawn-runner.spec.ts

@@ -342,6 +342,41 @@ describe('runner launch inputs', () => {
       candidate === 'C:\\target\\tool.exe')).toBe('C:\\target\\tool.exe')
     expect(resolveWindowsExecutable('.\\missing', 'C:\\target', {}, () => false))
       .toBe('C:\\target\\.\\missing')
+
+    expect(resolveWindowsExecutable('tool', 'C:\\target', {
+      PATH: ';;C:\\bin',
+    }, candidate => candidate === 'C:\\bin\\tool.exe')).toBe('C:\\bin\\tool.exe')
+    expect(resolveWindowsExecutable('tool', 'C:\\target', {
+      PATH: '"";C:\\bin',
+    }, candidate => candidate === 'C:\\bin\\tool.exe')).toBe('C:\\bin\\tool.exe')
+    expect(resolveWindowsExecutable('tool', 'C:\\target', {
+      PATH: '"unterminated',
+    }, candidate => candidate === 'C:\\target\\unterminated\\tool.exe'))
+      .toBe('C:\\target\\unterminated\\tool.exe')
+    expect(resolveWindowsExecutable('\\\\server\\share\\tool', 'C:\\target', {}, candidate =>
+      candidate === '\\\\server\\share\\tool.exe')).toBe('\\\\server\\share\\tool.exe')
+    expect(resolveWindowsExecutable('\\tools\\tool', 'C:\\target', {}, candidate =>
+      candidate === 'C:\\tools\\tool.exe')).toBe('C:\\tools\\tool.exe')
+    expect(resolveWindowsExecutable('C:tools\\tool', 'C:\\target', {}, candidate =>
+      candidate === 'C:\\target\\tools\\tool.exe')).toBe('C:\\target\\tools\\tool.exe')
+
+    const noSearchEnvironment = { NoDefaultCurrentDirectoryInExePath: '1' }
+    expect(resolveWindowsExecutable('missing', 'C:\\target', {}, () => false, noSearchEnvironment))
+      .toBe('C:\\target\\missing.exe')
+    expect(resolveWindowsExecutable('missing.cmd', 'C:\\target', {}, () => false, noSearchEnvironment))
+      .toBe('C:\\target\\missing.cmd')
+
+    const directory = mkdtempSync(join(tmpdir(), 'dsh-windows-resolver-'))
+    scratch.push(directory)
+    const executable = join(directory, 'direct.exe')
+    const directoryCandidate = join(directory, 'directory')
+    const missingExecutable = join(directory, 'missing.exe')
+    writeFileSync(executable, '')
+    mkdirSync(`${directoryCandidate}.com`)
+    writeFileSync(`${directoryCandidate}.exe`, '')
+    expect(resolveWindowsExecutable(executable, '', {})).toBe(executable)
+    expect(resolveWindowsExecutable(directoryCandidate, '', {})).toBe(`${directoryCandidate}.exe`)
+    expect(resolveWindowsExecutable(missingExecutable, '', {})).toBe(missingExecutable)
   })
 })
 

packages/subprocess/win32-process/src/process.ts

@@ -1,6 +1,7 @@
 /** Typed Win32 process operations over the shared binding table. */
 
 import koffi from 'koffi'
+import { closeSync } from 'node:fs'
 import * as abi from './abi.ts'
 import {
   allocProcessInfo,
@@ -484,28 +485,56 @@ export function probeCurrentTokenJobSupport(api: Win32ProcessBindings): void {
   closeHandleChecked(api, job, 'current-token Job capability probe')
 }
 
+interface MaterializedStdioStream {
+  readonly destroyed: boolean
+  destroy(): unknown
+  readonly _handle?: { close(): void } | null
+}
+
 /**
  * Release the runner's Node-owned standard streams after target creation.
- * The target retains its inherited handle copies; destroying the runner's
- * libuv owners permits parent pipe EOF to follow the target rather than the
- * longer-lived runner. Raw `CloseHandle` is insufficient because Node may own
- * a duplicated libuv handle that remains live until its stream is destroyed.
- * @param streams - injectable current-process streams used by tests.
+ * The target retains its inherited handle copies. Node deliberately makes
+ * `process.stdout.destroy()` and `process.stderr.destroy()` leave their libuv
+ * handles open, so the runner must close both the descriptors and materialized
+ * output handles for parent pipe EOF to follow the target.
+ * @param streams - stdin, stdout, and stderr used by the current process.
+ * @param closeDescriptor - injectable descriptor close used by tests.
  */
 export function closeCurrentProcessStandardStreams(
-  streams: ReadonlyArray<{ readonly destroyed: boolean; destroy(): unknown }> = [
+  streams: readonly [MaterializedStdioStream, MaterializedStdioStream, MaterializedStdioStream] = [
     process.stdin,
     process.stdout,
     process.stderr,
   ],
+  closeDescriptor: (fd: number) => void = closeSync,
 ): void {
   const failures: Error[] = []
-  for (const stream of new Set(streams)) {
-    if (stream.destroyed) continue
+  const recordFailure = (error: unknown): void => {
+    failures.push(error instanceof Error ? error : new Error(String(error)))
+  }
+  const [stdin, ...outputs] = streams
+  const outputHandles = new Set(outputs.flatMap(stream => stream.destroyed || stream._handle == null
+    ? []
+    : [stream._handle]))
+  if (!stdin.destroyed) {
     try {
-      stream.destroy()
+      stdin.destroy()
     } catch (error) {
-      failures.push(error instanceof Error ? error : new Error(String(error)))
+      recordFailure(error)
+    }
+  }
+  for (const fd of [0, 1, 2]) {
+    try {
+      closeDescriptor(fd)
+    } catch (error) {
+      if ((error as NodeJS.ErrnoException).code !== 'EBADF') recordFailure(error)
+    }
+  }
+  for (const handle of outputHandles) {
+    try {
+      handle.close()
+    } catch (error) {
+      recordFailure(error)
     }
   }
   if (failures.length === 1) {

packages/subprocess/win32-process/tests/ordinary-process.spec.ts

@@ -213,27 +213,49 @@ describe('ordinary Job process operations', () => {
     expect(closeHandle).toHaveBeenCalledExactlyOnceWith(50n)
   })
 
-  it('destroys each unique live runner standard stream', () => {
-    const alreadyClosed = { destroyed: true, destroy: vi.fn() }
-    const live = { destroyed: false, destroy: vi.fn() }
+  it('closes each runner descriptor and materialized output handle', () => {
+    const closeDescriptor = vi.fn()
+    const input = { destroyed: false, destroy: vi.fn() }
+    const sharedHandle = { close: vi.fn() }
+    const output = { destroyed: false, destroy: vi.fn(), _handle: sharedHandle }
+    const alreadyClosed = { destroyed: true, destroy: vi.fn(), _handle: { close: vi.fn() } }
 
-    expect(() => { closeCurrentProcessStandardStreams([alreadyClosed, live, live]) }).not.toThrow()
+    expect(() => {
+      closeCurrentProcessStandardStreams([input, output, output], closeDescriptor)
+    }).not.toThrow()
+    expect(input.destroy).toHaveBeenCalledOnce()
+    expect(output.destroy).not.toHaveBeenCalled()
+    expect(sharedHandle.close).toHaveBeenCalledOnce()
+    expect(closeDescriptor.mock.calls).toEqual([[0], [1], [2]])
+
+    expect(() => {
+      closeCurrentProcessStandardStreams([alreadyClosed, alreadyClosed, alreadyClosed], closeDescriptor)
+    }).not.toThrow()
     expect(alreadyClosed.destroy).not.toHaveBeenCalled()
-    expect(live.destroy).toHaveBeenCalledOnce()
+    expect(alreadyClosed._handle.close).not.toHaveBeenCalled()
   })
 
   it('reports one or several runner standard-stream close failures', () => {
-    expect(() => { closeCurrentProcessStandardStreams([{
-      destroyed: false,
-      destroy: () => { throw new Error('single close failure') },
-    }]) }).toThrow('single close failure')
+    const closed = { destroyed: true, destroy: vi.fn() }
+    expect(() => {
+      closeCurrentProcessStandardStreams([
+        { destroyed: false, destroy: () => { throw new Error('single close failure') } },
+        closed,
+        closed,
+      ], vi.fn((fd: number) => {
+        if (fd === 0) throw Object.assign(new Error('already closed'), { code: 'EBADF' })
+      }))
+    }).toThrow('single close failure')
 
     let failure: unknown
     try {
       closeCurrentProcessStandardStreams([
-        { destroyed: false, destroy: () => { throw 'raw close failure' } },
-        { destroyed: false, destroy: () => { throw new Error('second close failure') } },
-      ])
+        closed,
+        { destroyed: false, destroy: vi.fn(), _handle: { close: () => { throw 'raw close failure' } } },
+        { destroyed: false, destroy: vi.fn(), _handle: { close: () => { throw new Error('second close failure') } } },
+      ], (fd) => {
+        if (fd === 1) throw new Error('descriptor close failure')
+      })
     } catch (error) {
       failure = error
     }
@@ -245,6 +267,7 @@ describe('ordinary Job process operations', () => {
     expect(errors).toEqual(expect.arrayContaining([
       expect.objectContaining({ message: 'raw close failure' }),
       expect.objectContaining({ message: 'second close failure' }),
+      expect.objectContaining({ message: 'descriptor close failure' }),
     ]))
   })
 })