From 9dd0e147339382ea4f39434a5bce4a4219115fee Mon Sep 17 00:00:00 2001
From: YeonGyu-Kim <code.yeon.gyu@gmail.com>
Date: Wed, 6 May 2026 13:28:21 +0900
Subject: [PATCH] fix(bun-spawn-shim): match Bun.spawn semantics in Node
 fallback

Address cubic and oracle review feedback to make the Node/Electron
fallback faithful to Bun.spawn behavior so cross-runtime callers
behave identically.

Changes:
- resolveStdio() default stdio is now ["ignore", "pipe", "inherit"]
  to match Bun.spawn defaults (was ["pipe", "pipe", "pipe"]).
  Prevents hangs in dispatcher and on-complete-hook callers that
  did not explicitly set stdin and would otherwise wait forever
  for input on Node.
- spawn-with-windows-hide.ts uses the same defaults so the Windows
  Node helper aligns with the rest of the shim.
- wrapNodeProcess now rejects proc.exited with the original error
  on "error" events (previously swallowed the error and resolved
  to exit code 1, hiding ENOENT and friends).
- spawnSync result returns the real result.pid instead of -1 and
  exposes stdout/stderr as Buffer | undefined to match Node's
  spawnSync result shape when those streams are not piped.

Tests cover the new defaults, real pid surfacing, and missing
executable error propagation.

Refs cubic review and oracle audit on #3798.
---
 src/shared/bun-spawn-shim.test.ts     | 29 ++++++++++++++++++++++++++-
 src/shared/bun-spawn-shim.ts          | 18 ++++++++---------
 src/shared/spawn-with-windows-hide.ts |  2 +-
 3 files changed, 38 insertions(+), 11 deletions(-)

diff --git a/src/shared/bun-spawn-shim.test.ts b/src/shared/bun-spawn-shim.test.ts
index 06905bd76..238cfb1b1 100644
--- a/src/shared/bun-spawn-shim.test.ts
+++ b/src/shared/bun-spawn-shim.test.ts
@@ -59,6 +59,33 @@ describe("bun-spawn-shim", () => {
 
     expect(result.exitCode).toBe(0)
     expect(result.success).toBe(true)
-    expect(Buffer.from(result.stdout).toString().trim()).toBe("sync-ok")
+    expect(result.stdout).toBeDefined()
+    expect(Buffer.from(result.stdout!).toString().trim()).toBe("sync-ok")
+  })
+
+  test("#given spawnSync command #when it completes #then result.pid is a positive number", () => {
+    const result = spawnSync(["bun", "--version"], { stdout: "pipe", stderr: "pipe" })
+
+    expect(result.pid).toBeGreaterThan(0)
+  })
+
+  test("#given default stdio #when child reads stdin #then it does not hang waiting for input", async () => {
+    const proc = spawn(["cat"], { stdout: "pipe", stderr: "pipe" })
+
+    const exitCode = await proc.exited
+
+    expect(exitCode).toBe(0)
+  })
+
+  test("#given missing executable #when spawn invoked #then the error is surfaced to the caller", async () => {
+    let observedError: unknown
+    try {
+      const proc = spawn(["__omo-shim-missing-binary__"], { stdout: "pipe", stderr: "pipe" })
+      await proc.exited
+    } catch (error) {
+      observedError = error
+    }
+
+    expect(observedError).toBeDefined()
   })
 })
diff --git a/src/shared/bun-spawn-shim.ts b/src/shared/bun-spawn-shim.ts
index 158894f2d..de756793d 100644
--- a/src/shared/bun-spawn-shim.ts
+++ b/src/shared/bun-spawn-shim.ts
@@ -30,8 +30,8 @@ export interface SpawnedProcess {
 
 export interface SpawnSyncResult {
   readonly exitCode: number
-  readonly stdout: Buffer
-  readonly stderr: Buffer
+  readonly stdout: Buffer | undefined
+  readonly stderr: Buffer | undefined
   readonly success: boolean
   readonly pid: number
 }
@@ -81,20 +81,20 @@ function resolveCommand(cmdOrOpts: unknown, optsArg?: unknown): { cmd: string[];
 function resolveStdio(options: SpawnOptions): StdioTuple {
   if (options.stdio) return options.stdio
 
-  return [options.stdin ?? "pipe", options.stdout ?? "pipe", options.stderr ?? "pipe"]
+  return [options.stdin ?? "ignore", options.stdout ?? "pipe", options.stderr ?? "inherit"]
 }
 
 function wrapNodeProcess(proc: ReturnType<typeof nodeSpawn>): SpawnedProcess {
   let exitCode: number | null = null
-  const exited = new Promise<number>((resolve) => {
+  const exited = new Promise<number>((resolve, reject) => {
     proc.on("exit", (code) => {
       exitCode = code ?? 1
       resolve(exitCode)
     })
-    proc.on("error", () => {
+    proc.on("error", (error) => {
       if (exitCode === null) {
         exitCode = 1
-        resolve(1)
+        reject(error)
       }
     })
   })
@@ -158,9 +158,9 @@ export function spawnSync(cmdOrOpts: unknown, opts?: unknown): SpawnSyncResult {
 
   return {
     exitCode: result.status ?? 1,
-    stdout: result.stdout,
-    stderr: result.stderr,
+    stdout: result.stdout ?? undefined,
+    stderr: result.stderr ?? undefined,
     success: (result.status ?? 1) === 0,
-    pid: -1,
+    pid: result.pid ?? -1,
   }
 }
diff --git a/src/shared/spawn-with-windows-hide.ts b/src/shared/spawn-with-windows-hide.ts
index 872c8deeb..f6fec2a7e 100644
--- a/src/shared/spawn-with-windows-hide.ts
+++ b/src/shared/spawn-with-windows-hide.ts
@@ -75,7 +75,7 @@ export function spawnWithWindowsHide(command: string[], options: SpawnOptions):
   const proc = nodeSpawn(cmd, args, {
     cwd: options.cwd,
     env: options.env,
-    stdio: [options.stdin ?? "pipe", options.stdout ?? "pipe", options.stderr ?? "pipe"],
+    stdio: [options.stdin ?? "ignore", options.stdout ?? "pipe", options.stderr ?? "inherit"],
     windowsHide: true,
     shell: true,
   })