fix(subprocess): close containment review findings

packages/subagent/subagent-acp/src/run.ts

@@ -398,8 +398,8 @@ export async function startAcpRun(request: SubagentStartRequest, spec: AcpRunSpe
     try {
       return await Promise.race([processDone, aborted.promise])
     } catch {
-      // The active protocol failure remains authoritative when exit observation fails.
-      /* v8 ignore next -- a published child.done cannot reject; spawn rejection is consumed before publication. */
+      // A provider rejection after handle publication leaves no direct outcome;
+      // the active protocol failure remains authoritative.
       return processOutcome
     } finally {
       bound.removeEventListener('abort', onObservationAbort)

packages/subprocess/subprocess-local/src/linux-scope.ts

@@ -150,7 +150,9 @@ class SystemdScopeOwner implements BoundProcessOwner {
 
   signal(signal: 'SIGTERM' | 'SIGKILL'): void {
     if (this.stopped) return
-    if (this.direct.running()) this.direct.signal(signal)
+    if (!this.established && !existsSync(this.files.requestPath)) this.established = true
+    const directFallbackRequired = !this.established
+    if (directFallbackRequired && this.direct.running()) this.direct.signal(signal)
     const result = this.runSync(this.systemctl, [
       '--user',
       'kill',
@@ -162,6 +164,7 @@ class SystemdScopeOwner implements BoundProcessOwner {
       if (signal === 'SIGKILL') this.killFailure = undefined
       return
     }
+    if (!directFallbackRequired && this.direct.running()) this.direct.signal(signal)
     if (signal === 'SIGKILL') {
       const output = `${result.stdout}\n${result.stderr}`
       if (!MISSING_UNIT.test(output)) {

packages/subprocess/subprocess-local/src/spawn-runner.ts

@@ -1,6 +1,7 @@
 /** One-shot Linux exec bootstrap and Windows Job-owning subprocess runner. */
 
 import { closeSync } from 'node:fs'
+import koffi from 'koffi'
 import {
   closeHandleChecked,
   isJobEmpty,
@@ -40,6 +41,31 @@ type RunnerHost = Pick<NodeJS.Process, 'env' | 'exitCode' | 'connected' | 'cwd'
   send?: NodeJS.Process['send']
 }
 
+interface UvErrorBindings {
+  translateSystemError(systemError: number): number
+  errorName(error: number): string
+}
+
+let cachedUvErrorBindings: UvErrorBindings | undefined
+
+function loadUvErrorBindings(): UvErrorBindings {
+  if (cachedUvErrorBindings !== undefined) return cachedUvErrorBindings
+  const node = koffi.load(null)
+  cachedUvErrorBindings = {
+    translateSystemError: node.func(
+      'uv_translate_sys_error',
+      'int',
+      ['int'],
+    ) as unknown as UvErrorBindings['translateSystemError'],
+    errorName: node.func(
+      'uv_err_name',
+      'str',
+      ['int'],
+    ) as unknown as UvErrorBindings['errorName'],
+  }
+  return cachedUvErrorBindings
+}
+
 /** Injectable operations used by the protocol-owner tests. */
 export interface SpawnRunnerInternals {
   execve(file: string, argv: string[], env: Record<string, string>): never
@@ -51,6 +77,7 @@ export interface SpawnRunnerInternals {
   isJobEmpty: typeof isJobEmpty
   terminateJob: typeof terminateJob
   closeHandleChecked: typeof closeHandleChecked
+  uvErrorBindings?: UvErrorBindings
 }
 
 const defaultInternals: SpawnRunnerInternals = {
@@ -66,6 +93,8 @@ const defaultInternals: SpawnRunnerInternals = {
   closeHandleChecked,
 }
 
+const NODE_SPAWN_DETAIL_CODES = new Set(['EACCES', 'EAGAIN', 'EMFILE', 'ENFILE', 'ENOENT'])
+
 function nodeSpawnError(
   source: Pick<SerializedRunnerError, 'stack'>,
   syscall: string,
@@ -87,7 +116,12 @@ function nodeSpawnError(
   }
 }
 
-function asSpawnError(error: unknown, program: string, args: readonly string[]): SerializedRunnerError {
+function asSpawnError(
+  error: unknown,
+  program: string,
+  args: readonly string[],
+  internals: Pick<SpawnRunnerInternals, 'uvErrorBindings'>,
+): SerializedRunnerError {
   const serialized = serializeRunnerError(error)
   if (!(error instanceof Win32Error)) {
     return serialized.code === undefined
@@ -97,23 +131,15 @@ function asSpawnError(error: unknown, program: string, args: readonly string[]):
         spawnargs: [...args],
       })
   }
-  if (error.win32Code === 2 || error.win32Code === 3 || error.win32Code === 267) {
-    return nodeSpawnError(serialized, `spawn ${program}`, 'ENOENT', -4058, {
-      path: program,
-      spawnargs: [...args],
-    })
-  }
-  if (error.win32Code === 740) {
-    return nodeSpawnError(serialized, `spawn ${program}`, 'EACCES', -4092, {
+  const uv = internals.uvErrorBindings ?? loadUvErrorBindings()
+  const errno = uv.translateSystemError(error.win32Code)
+  const code = uv.errorName(errno)
+  if (NODE_SPAWN_DETAIL_CODES.has(code)) {
+    return nodeSpawnError(serialized, `spawn ${program}`, code, errno, {
       path: program,
       spawnargs: [...args],
     })
   }
-  const [code, errno] = error.win32Code === 5
-    ? ['EPERM', -4048]
-    : error.win32Code === 193
-      ? ['EFTYPE', -4028]
-      : ['UNKNOWN', -4094]
   return nodeSpawnError(serialized, 'spawn', code, errno, {})
 }
 
@@ -198,7 +224,7 @@ function runLinux(
   } catch (error) {
     writeLinuxStartupError(files, {
       type: 'error',
-      error: asSpawnError(error, argv[0] as string, argv.slice(1)),
+      error: asSpawnError(error, argv[0] as string, argv.slice(1), internals),
     })
     host.exitCode = 127
   }
@@ -329,7 +355,7 @@ class WindowsJobRunner {
       if (this.jobHandle === undefined && error instanceof Win32Error && error.api === 'CreateProcessW') {
         await this.publishTerminalResult({
           type: 'error',
-          error: asSpawnError(error, this.argv[0] as string, this.argv.slice(1)),
+          error: asSpawnError(error, this.argv[0] as string, this.argv.slice(1), this.internals),
         }, 0)
         return
       }

packages/subprocess/subprocess-local/src/spawn.ts

@@ -481,6 +481,7 @@ export function bindManagedProcess(
   }
 
   let graceTimer: ReturnType<typeof setTimeout> | undefined
+  let terminationStarted = false
   let rangeExitObserved = false
   let rangeExitObservation: Promise<void> | undefined
   let settled = false
@@ -520,7 +521,8 @@ export function bindManagedProcess(
   }
 
   const terminateWithReason = (cancellationReason: unknown): void => {
-    if (rangeExitObserved || graceTimer !== undefined) return
+    if (rangeExitObserved || terminationStarted) return
+    terminationStarted = true
     // Keep the shared observation rejection available to waitForExit() without
     // leaking an unhandled rejection when a caller only invokes terminate().
     void observeRangeExit().catch(() => {})

packages/subprocess/subprocess-local/tests/linux-scope.spec.ts

@@ -193,6 +193,28 @@ describe('Linux scope establishment and quiescence', () => {
     expect(existsSync(linuxLaunchFilesFromLocator(requestPath).directory)).toBe(false)
   })
 
+  it('uses the scope alone after establishment and the direct range only when scope signalling fails', async () => {
+    const spawnSync = vi.fn()
+      .mockReturnValueOnce({ status: 0, stdout: '', stderr: '' })
+      .mockReturnValueOnce({ status: 1, stdout: '', stderr: 'scope signal failed' })
+    const { child, result, requestPath } = launch(async () => activeUnit(), {
+      spawnSync: spawnSync as never,
+    })
+    consumeLinuxLaunchRequest(requestPath)
+    const processKill = vi.spyOn(process, 'kill').mockReturnValue(true)
+
+    result.owner.signal('SIGTERM')
+    expect(processKill).not.toHaveBeenCalled()
+
+    result.owner.signal('SIGKILL')
+    expect(processKill).toHaveBeenCalledExactlyOnceWith(-321, 'SIGKILL')
+    expect(spawnSync).toHaveBeenCalledTimes(2)
+
+    child.exit(null, 'SIGKILL')
+    await expect(result.direct).resolves.toEqual({ exitCode: null, signal: 'SIGKILL' })
+    result.owner.cleanup?.()
+  })
+
   it('uses manager-observed unit existence as establishment proof', async () => {
     const { child, result } = launch(async () => activeUnit('inactive'))
     await expect(result.owner.waitForExit()).resolves.toBeUndefined()

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

@@ -105,6 +105,10 @@ function internals(overrides: Partial<SpawnRunnerInternals> = {}): SpawnRunnerIn
     isJobEmpty: vi.fn(() => true),
     terminateJob: vi.fn(),
     closeHandleChecked: vi.fn(),
+    uvErrorBindings: {
+      translateSystemError: vi.fn(systemError => systemError === 2 ? -4058 : -4094),
+      errorName: vi.fn(error => error === -4058 ? 'ENOENT' : 'UNKNOWN'),
+    },
     ...overrides,
   }
 }
@@ -603,20 +607,26 @@ describe('Windows Job runner protocol owner', () => {
     expect(host.exitCode).toBe(0)
   })
 
-  it('maps the bounded Win32 process-creation error classes', async () => {
+  it('uses libuv translation and Node detail-bearing codes for Win32 process-creation errors', async () => {
     for (const [win32Code, code, errno, enriched, program] of [
       [2, 'ENOENT', -4058, true, 'tool.exe'],
-      [3, 'ENOENT', -4058, true, 'tool.exe'],
-      [267, 'ENOENT', -4058, true, 'tool.exe'],
       [740, 'EACCES', -4092, true, '$&.exe'],
+      [10035, 'EAGAIN', -4088, true, 'tool.exe'],
+      [4, 'EMFILE', -4066, true, 'tool.exe'],
+      [12345, 'ENFILE', -4061, true, 'tool.exe'],
       [5, 'EPERM', -4048, false, 'tool.exe'],
       [193, 'EFTYPE', -4028, false, 'tool.exe'],
       [999, 'UNKNOWN', -4094, false, 'tool.exe'],
     ] as const) {
       const host = new FakeRunnerHost()
+      const translateSystemError = vi.fn(() => errno)
+      const errorName = vi.fn(() => code)
       await runWindows(host, internals({
         spawnCurrentTokenJobProcess: vi.fn(() => { throw new Win32Error('CreateProcessW', win32Code) }),
+        uvErrorBindings: { translateSystemError, errorName },
       }), undefined, [program, 'literal arg'])
+      expect(translateSystemError).toHaveBeenCalledExactlyOnceWith(win32Code)
+      expect(errorName).toHaveBeenCalledExactlyOnceWith(errno)
       const syscall = enriched ? `spawn ${program}` : 'spawn'
       expect(host.sent).toMatchObject([{
         type: 'error',
@@ -640,6 +650,52 @@ describe('Windows Job runner protocol owner', () => {
     }
   })
 
+  it('loads the error translation functions from Node-linked libuv', async () => {
+    for (let attempt = 0; attempt < 2; attempt += 1) {
+      const host = new FakeRunnerHost()
+      const native = internals({
+        spawnCurrentTokenJobProcess: vi.fn(() => { throw new Win32Error('CreateProcessW', 2) }),
+      })
+      Reflect.deleteProperty(native, 'uvErrorBindings')
+      await runWindows(host, native)
+      expect(host.sent).toMatchObject([{
+        type: 'error',
+        error: {
+          code: 'ENOENT',
+          errno: process.platform === 'win32' ? -4058 : -2,
+          path: 'tool.exe',
+          spawnargs: ['literal arg'],
+        },
+      }])
+    }
+  })
+
+  it.skipIf(process.platform !== 'win32')('preserves native EMFILE and UNKNOWN translations', async () => {
+    for (const [win32Code, code, errno, enriched] of [
+      [4, 'EMFILE', -4066, true],
+      [999, 'UNKNOWN', -4094, false],
+    ] as const) {
+      const host = new FakeRunnerHost()
+      const native = internals({
+        spawnCurrentTokenJobProcess: vi.fn(() => { throw new Win32Error('CreateProcessW', win32Code) }),
+      })
+      Reflect.deleteProperty(native, 'uvErrorBindings')
+      await runWindows(host, native)
+      expect(host.sent).toMatchObject([{
+        type: 'error',
+        error: { code, errno },
+      }])
+      const result = parseWindowsRunnerResult(host.sent[0])
+      if (result.type !== 'error') throw new Error('expected runner error')
+      if (enriched) {
+        expect(result.error).toMatchObject({ path: 'tool.exe', spawnargs: ['literal arg'] })
+      } else {
+        expect(result.error).not.toHaveProperty('path')
+        expect(result.error).not.toHaveProperty('spawnargs')
+      }
+    }
+  })
+
   it('rejects a Windows runner without an initial IPC channel', async () => {
     const disconnected = new FakeRunnerHost()
     disconnected.connected = false

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

@@ -908,6 +908,44 @@ describe('coverage seams', () => {
     }
   })
 
+  it('does not restart managed termination after the escalation timer fires', async () => {
+    vi.useFakeTimers()
+    try {
+      const direct = Promise.withResolvers<{ exitCode: number; signal: null }>()
+      const stopped = Promise.withResolvers<undefined>()
+      const signal = vi.fn()
+      const handle = bindManagedProcess(spec('true', {
+        graceMs: 10,
+        stdio: { stdin: 'ignore', stdout: 'inherit', stderr: 'inherit' },
+      }), {
+        stdin: null,
+        stdout: null,
+        stderr: null,
+        direct: direct.promise,
+        owner: {
+          signal,
+          waitForExit: () => stopped.promise,
+          terminateForHostExit: vi.fn(),
+        },
+      })
+
+      handle.terminate()
+      await vi.advanceTimersByTimeAsync(10)
+      handle.terminate()
+      await vi.advanceTimersByTimeAsync(10)
+      expect(signal).toHaveBeenCalledTimes(2)
+      expect(signal).toHaveBeenNthCalledWith(1, 'SIGTERM', expect.any(Error))
+      expect(signal).toHaveBeenNthCalledWith(2, 'SIGKILL', undefined)
+
+      stopped.resolve(undefined)
+      await expect(handle.waitForExit()).resolves.toBe(true)
+      direct.resolve({ exitCode: 0, signal: null })
+      await expect(handle.done).resolves.toEqual({ exitCode: 0, signal: null })
+    } finally {
+      vi.useRealTimers()
+    }
+  })
+
   it('delivers an already-aborted managed spawn reason before target settlement', async () => {
     const reason = null
     const controller = new AbortController()