Merge 33143ad923 into a3259400a3

browse/src/bun-polyfill.cjs

@@ -93,11 +93,103 @@ globalThis.Bun = {
       cwd: options.cwd,
     });
 
+    // Drain stdout/stderr eagerly into in-memory buffers. Bun's spawn buffers
+    // these for the consumer; Node's Readables are pull-based, so if the caller
+    // awaits `proc.exited` before reading, anything past the OS pipe buffer
+    // (~16-64 KB) back-pressures the child until it blocks in write() and
+    // `exit` never fires. Eager draining keeps the pipes flowing regardless
+    // of read order; replay below is via fresh Web ReadableStreams.
+    //
+    // Cap the buffer so a runaway child can't OOM the server. 16 MB is
+    // generous: DPAPI outputs are tiny, tasklist is <1 KB, and the
+    // browser-skill consumer has its own 1 MB readCapped. Once the cap is
+    // reached we keep draining the pipe (so the child never blocks) but
+    // discard further bytes. Override via GSTACK_SPAWN_MAX_BUFFER (bytes).
+    const MAX_BUFFER = Math.max(
+      0,
+      parseInt(process.env.GSTACK_SPAWN_MAX_BUFFER || '', 10) || 16 * 1024 * 1024,
+    );
+    const drain = (stream) => {
+      if (!stream) return { done: Promise.resolve(), chunks: [], truncated: false };
+      const state = { chunks: [], bytes: 0, truncated: false };
+      const done = new Promise((resolve) => {
+        stream.on('data', (chunk) => {
+          if (state.bytes >= MAX_BUFFER) { state.truncated = true; return; }
+          if (state.bytes + chunk.length <= MAX_BUFFER) {
+            state.chunks.push(chunk);
+            state.bytes += chunk.length;
+          } else {
+            const remaining = MAX_BUFFER - state.bytes;
+            state.chunks.push(chunk.subarray(0, remaining));
+            state.bytes = MAX_BUFFER;
+            state.truncated = true;
+          }
+        });
+        // Any terminal event resolves: 'end' on normal close, 'error' on a
+        // stream-level error, 'close' as the belt-and-suspenders for spawn
+        // failures where Node fires 'close' but neither 'end' nor 'error'.
+        stream.once('end', resolve);
+        stream.once('error', resolve);
+        stream.once('close', resolve);
+      });
+      return { done, chunks: state.chunks };
+    };
+    const stdoutDrain = drain(proc.stdout);
+    const stderrDrain = drain(proc.stderr);
+
+    // Bun's spawn exposes `proc.exited` as a Promise resolving to the exit
+    // code; several call sites — DPAPI decryption, isBrowserRunning,
+    // browser-skill-commands — `await proc.exited` directly or via
+    // Promise.race with a timeout. Without this, those awaits resolve to
+    // `undefined` immediately and the operation looks like a silent failure.
+    // Resolve only after both pipes have finished draining so consumers that
+    // read stdout AFTER awaiting exit see the full output, not a partial buffer.
+    const exited = new Promise((resolveExited) => {
+      let exitStatus;
+      proc.once('exit', (code, signal) => {
+        // Match Bun: exit code on normal exit; 128 + signal number on signal;
+        // 0 if neither was reported.
+        if (code !== null) exitStatus = code;
+        else if (signal) exitStatus = 128 + (require('os').constants.signals[signal] || 0);
+        else exitStatus = 0;
+      });
+      proc.once('error', () => {
+        if (exitStatus === undefined) exitStatus = 1;
+      });
+      // Wait for either 'exit' (normal child lifecycle) or 'error' (spawn
+      // failure — Node fires error without exit when the binary is missing).
+      // Either path resolves the lifecycle promise; without listening to both
+      // a spawn error hangs `await proc.exited` until the consumer's own
+      // timeout fires.
+      const lifecycle = new Promise((r) => {
+        proc.once('exit', r);
+        proc.once('error', r);
+      });
+      Promise.all([lifecycle, stdoutDrain.done, stderrDrain.done])
+        .then(() => resolveExited(exitStatus !== undefined ? exitStatus : 0));
+    });
+
+    // Replay buffered output as a fresh Web ReadableStream. `start()` awaits
+    // the drain before enqueueing so `new Response(proc.stdout).text()` yields
+    // the complete output regardless of whether the consumer reads before or
+    // after awaiting `proc.exited`. Stream is single-shot (locked after one
+    // read), matching Bun's behavior.
+    const replay = (d) => new ReadableStream({
+      async start(controller) {
+        await d.done;
+        for (const chunk of d.chunks) {
+          controller.enqueue(chunk instanceof Uint8Array ? chunk : new Uint8Array(chunk));
+        }
+        controller.close();
+      },
+    });
+
     return {
       pid: proc.pid,
-      stdout: proc.stdout,
-      stderr: proc.stderr,
+      stdout: replay(stdoutDrain),
+      stderr: replay(stderrDrain),
       stdin: proc.stdin,
+      exited,
       unref() { proc.unref(); },
       kill(signal) { proc.kill(signal); },
     };

browse/test/bun-polyfill.test.ts

@@ -1,8 +1,11 @@
 import { describe, test, expect, afterAll } from 'bun:test';
 import * as path from 'path';
 
-// Load the polyfill into a fresh object (don't clobber globalThis.Bun)
-const polyfillPath = path.resolve(import.meta.dir, '../src/bun-polyfill.cjs');
+// Load the polyfill into a fresh object (don't clobber globalThis.Bun).
+// Forward-slash on Windows so the path interpolates cleanly into the
+// `require('${polyfillPath}')` template literals below — raw backslashes
+// would be interpreted as JS escape sequences in the spawned Node script.
+const polyfillPath = path.resolve(import.meta.dir, '../src/bun-polyfill.cjs').replace(/\\/g, '/');
 
 describe('bun-polyfill', () => {
   // We test the polyfill by requiring it in a subprocess under Node.js
@@ -48,6 +51,127 @@ describe('bun-polyfill', () => {
     expect(lines[2]).toBe('HAS_UNREF');
   });
 
+  // Bun.spawn parity: `proc.exited` is a Promise resolving to the exit code.
+  // The DPAPI helper and isBrowserRunning both `await proc.exited`; without
+  // it the awaits resolve immediately to `undefined` and the caller reads
+  // stdout before the child has produced it — surfacing as a silent failure.
+  test('Bun.spawn exposes proc.exited that resolves to the exit code', async () => {
+    const result = Bun.spawnSync(['node', '-e', `
+      require('${polyfillPath}');
+      (async () => {
+        const p = Bun.spawn(['node', '-e', 'process.exit(0)'], { stdio: ['ignore', 'ignore', 'ignore'] });
+        console.log(typeof p.exited === 'object' && typeof p.exited.then === 'function' ? 'IS_PROMISE' : 'NOT_PROMISE');
+        console.log('exit:' + await p.exited);
+      })();
+    `], { stdout: 'pipe', stderr: 'pipe' });
+    const lines = result.stdout.toString().trim().split('\n');
+    expect(lines[0]).toBe('IS_PROMISE');
+    expect(lines[1]).toBe('exit:0');
+  });
+
+  test('Bun.spawn proc.exited reflects non-zero exit codes', async () => {
+    const result = Bun.spawnSync(['node', '-e', `
+      require('${polyfillPath}');
+      (async () => {
+        const p = Bun.spawn(['node', '-e', 'process.exit(3)'], { stdio: ['ignore', 'ignore', 'ignore'] });
+        console.log('exit:' + await p.exited);
+      })();
+    `], { stdout: 'pipe', stderr: 'pipe' });
+    expect(result.stdout.toString().trim()).toBe('exit:3');
+  });
+
+  test('Bun.spawn proc.exited resolves before reading stdout (no race)', async () => {
+    const result = Bun.spawnSync(['node', '-e', `
+      require('${polyfillPath}');
+      (async () => {
+        // Real-world pattern: write to stdout, then exit. Awaiting proc.exited
+        // before reading must guarantee the bytes are flushed.
+        const p = Bun.spawn(['node', '-e', 'process.stdout.write("ready"); process.exit(0)'], {
+          stdio: ['ignore', 'pipe', 'ignore']
+        });
+        const code = await p.exited;
+        const out = await new Response(p.stdout).text();
+        console.log(out + ':' + code);
+      })();
+    `], { stdout: 'pipe', stderr: 'pipe' });
+    expect(result.stdout.toString().trim()).toBe('ready:0');
+  });
+
+  // Spawn-failure case: Node emits 'error' but not 'exit' when the binary
+  // is missing, so listening only for 'exit' hangs `await proc.exited`
+  // forever. The lifecycle promise must resolve on either event.
+  test('Bun.spawn proc.exited resolves on spawn failure (missing binary)', async () => {
+    const result = Bun.spawnSync(['node', '-e', `
+      require('${polyfillPath}');
+      (async () => {
+        const p = Bun.spawn(['this-binary-does-not-exist-zzz-' + Date.now()], {
+          stdio: ['ignore', 'pipe', 'pipe']
+        });
+        const code = await Promise.race([
+          p.exited,
+          new Promise((_, r) => setTimeout(() => r(new Error('timeout')), 3000))
+        ]).catch(() => 'TIMEOUT');
+        console.log('exit:' + code);
+      })();
+    `], { stdout: 'pipe', stderr: 'pipe' });
+    // Anything other than 'TIMEOUT' (and ideally a non-zero number) means the
+    // lifecycle promise resolved on the spawn error.
+    const out = result.stdout.toString().trim();
+    expect(out).not.toBe('exit:TIMEOUT');
+    expect(out).toMatch(/^exit:\d+$/);
+  });
+
+  // GSTACK_SPAWN_MAX_BUFFER caps the drain so a runaway child can't OOM the
+  // server. Past the cap, the pipe keeps flowing (child doesn't block) but
+  // further bytes are dropped. Set a small cap, write more than that, assert
+  // the captured stdout equals the cap and the child exits cleanly.
+  test('Bun.spawn caps buffered output at GSTACK_SPAWN_MAX_BUFFER', async () => {
+    const result = Bun.spawnSync(['node', '-e', `
+      process.env.GSTACK_SPAWN_MAX_BUFFER = '${1024}';
+      require('${polyfillPath}');
+      (async () => {
+        // Child writes 10 KB; cap is 1 KB; drained output should be exactly 1 KB
+        // and exit should still resolve cleanly (child not back-pressured to death).
+        const p = Bun.spawn(
+          ['node', '-e', 'process.stdout.write("y".repeat(10 * 1024)); process.exit(0)'],
+          { stdio: ['ignore', 'pipe', 'ignore'] }
+        );
+        const code = await Promise.race([
+          p.exited,
+          new Promise((_, r) => setTimeout(() => r(new Error('timeout')), 3000))
+        ]).catch(() => 'TIMEOUT');
+        const out = await new Response(p.stdout).text();
+        console.log(out.length + ':' + code);
+      })();
+    `], { stdout: 'pipe', stderr: 'pipe' });
+    expect(result.stdout.toString().trim()).toBe('1024:0');
+  });
+
+  // Regression for the pipe-blocking case: if the child writes more than the
+  // OS pipe buffer (~16-64 KB) and the polyfill doesn't drain eagerly, the
+  // child blocks in write() and `exit` never fires. 1 MB is well past every
+  // OS pipe buffer size. Pre-fix this test hangs forever; post-fix it returns
+  // in <500ms. Bun's default per-test timeout is 5s — generous here.
+  test('Bun.spawn drains large stdout so proc.exited still resolves', async () => {
+    const result = Bun.spawnSync(['node', '-e', `
+      require('${polyfillPath}');
+      (async () => {
+        const ONE_MB = 1024 * 1024;
+        const p = Bun.spawn(
+          ['node', '-e', 'process.stdout.write("x".repeat(' + ONE_MB + ')); process.exit(0)'],
+          { stdio: ['ignore', 'pipe', 'ignore'] }
+        );
+        const code = await Promise.race([
+          p.exited,
+          new Promise((_, r) => setTimeout(() => r(new Error('timeout')), 10000))
+        ]).catch(e => 'TIMEOUT');
+        const out = await new Response(p.stdout).text();
+        console.log(out.length + ':' + code);
+      })().catch((e) => { console.log('THREW:' + e.message); });
+    `], { stdout: 'pipe', stderr: 'pipe' });
+    expect(result.stdout.toString().trim()).toBe('1048576:0');
+  }, 15000);
+
   test('Bun.serve creates an HTTP server that responds', async () => {
     const result = Bun.spawnSync(['node', '-e', `
       require('${polyfillPath}');