mirror of https://github.com/garrytan/gstack.git
fix(windows): forward windowsHide through the bun-polyfill spawn shims
windowsHide is the one spawn option where Node's default is the opposite of Bun's: Node shows the child's console window, Bun.spawn hides it. The polyfill's spawn and spawnSync shims dropped the option entirely, so the Node fallback path (dist/bun-polyfill.cjs) silently inverted the behavior on the one platform the shim exists to serve — every watchdog respawn of the terminal agent popped a visible bun.exe console window. Three sites fixed: - Bun.spawnSync shim: forwards windowsHide with Bun-matching default true - Bun.spawn shim: same (stdio:'ignore' silences output but does NOT suppress the console window on Windows) - spawnTerminalAgent in terminal-agent-control.ts: explicit windowsHide: true, so the Node fallback path behaves like Bun-native An explicit windowsHide: false is honored at both shims. Three focused tests pin the default-true, default-true-sync, and explicit-false paths by intercepting child_process in a subprocess; the test file's require path now uses forward slashes so it survives interpolation into a JS string literal on Windows. Supersedes PRs #2523, #2294 and #2290, which each covered a subset of these sites. Contributed by @jerrynicholsai (PR #2539); earlier fixes by @jwilk-hrep, @rroojrooj and @WimvandenHeijkant covered subsets of the same sites. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
parent
f96fd46b1c
commit
15c9455a69
|
|
@ -75,6 +75,10 @@ globalThis.Bun = {
|
|||
timeout: options.timeout,
|
||||
env: options.env,
|
||||
cwd: options.cwd,
|
||||
// Node defaults windowsHide to false; Bun.spawn hides the console
|
||||
// window. Without this the shim silently inverts the behavior on the
|
||||
// one platform it exists to serve. See the spawn() note below.
|
||||
windowsHide: options.windowsHide !== false,
|
||||
});
|
||||
|
||||
return {
|
||||
|
|
@ -91,6 +95,11 @@ globalThis.Bun = {
|
|||
stdio,
|
||||
env: options.env,
|
||||
cwd: options.cwd,
|
||||
// stdio:'ignore' silences a child's output but does not suppress its
|
||||
// console window on Windows. The terminal-agent respawn (server.ts
|
||||
// watchdog, 60s ticker) therefore popped a visible bun.exe window on
|
||||
// every respawn until this was forwarded.
|
||||
windowsHide: options.windowsHide !== false,
|
||||
});
|
||||
|
||||
return {
|
||||
|
|
|
|||
|
|
@ -77,6 +77,10 @@ export function spawnTerminalAgent(opts: {
|
|||
...(opts.extraEnv || {}),
|
||||
},
|
||||
stdio: ['ignore', 'ignore', 'ignore'],
|
||||
// Explicit for the Node fallback path (dist/bun-polyfill.cjs), where the
|
||||
// host default is the opposite of Bun's. A visible console window on every
|
||||
// watchdog respawn is the symptom when this is missing.
|
||||
windowsHide: true,
|
||||
});
|
||||
proc.unref?.();
|
||||
return proc.pid ?? null;
|
||||
|
|
|
|||
|
|
@ -3,6 +3,9 @@ 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');
|
||||
// Forward slashes so the path survives interpolation into a JS string literal
|
||||
// on Windows, which is the platform this polyfill exists for.
|
||||
const requirePath = polyfillPath.replace(/\\/g, '/');
|
||||
|
||||
describe('bun-polyfill', () => {
|
||||
// We test the polyfill by requiring it in a subprocess under Node.js
|
||||
|
|
@ -10,7 +13,7 @@ describe('bun-polyfill', () => {
|
|||
|
||||
test('Bun.sleep resolves after delay', async () => {
|
||||
const result = Bun.spawnSync(['node', '-e', `
|
||||
require('${polyfillPath}');
|
||||
require('${requirePath}');
|
||||
(async () => {
|
||||
const start = Date.now();
|
||||
await Bun.sleep(50);
|
||||
|
|
@ -24,7 +27,7 @@ describe('bun-polyfill', () => {
|
|||
|
||||
test('Bun.spawnSync runs a command and returns stdout', () => {
|
||||
const result = Bun.spawnSync(['node', '-e', `
|
||||
require('${polyfillPath}');
|
||||
require('${requirePath}');
|
||||
const r = Bun.spawnSync(['echo', 'hello'], { stdout: 'pipe' });
|
||||
console.log(r.stdout.toString().trim());
|
||||
console.log('exit:' + r.exitCode);
|
||||
|
|
@ -36,7 +39,7 @@ describe('bun-polyfill', () => {
|
|||
|
||||
test('Bun.spawn launches a process with pid', async () => {
|
||||
const result = Bun.spawnSync(['node', '-e', `
|
||||
require('${polyfillPath}');
|
||||
require('${requirePath}');
|
||||
const p = Bun.spawn(['echo', 'test'], { stdio: ['pipe', 'pipe', 'pipe'] });
|
||||
console.log(typeof p.pid === 'number' ? 'HAS_PID' : 'NO_PID');
|
||||
console.log(typeof p.kill === 'function' ? 'HAS_KILL' : 'NO_KILL');
|
||||
|
|
@ -48,9 +51,52 @@ describe('bun-polyfill', () => {
|
|||
expect(lines[2]).toBe('HAS_UNREF');
|
||||
});
|
||||
|
||||
// windowsHide is the one option where Node's default is the opposite of
|
||||
// Bun's: Node shows the child's console window, Bun hides it. Dropping it
|
||||
// in translation makes every spawned child pop a window on Windows, which
|
||||
// is the platform this whole file exists for. Both shims are covered.
|
||||
test('Bun.spawn defaults windowsHide to true', () => {
|
||||
const result = Bun.spawnSync(['node', '-e', `
|
||||
const cp = require('child_process');
|
||||
const orig = cp.spawn;
|
||||
let seen;
|
||||
cp.spawn = (c, a, o) => { seen = o; return orig(c, a, o); };
|
||||
require('${requirePath}');
|
||||
Bun.spawn(['node', '-e', ''], { stdio: ['ignore', 'ignore', 'ignore'] });
|
||||
console.log('windowsHide:' + seen.windowsHide);
|
||||
`], { stdout: 'pipe', stderr: 'pipe' });
|
||||
expect(result.stdout.toString().trim()).toBe('windowsHide:true');
|
||||
});
|
||||
|
||||
test('Bun.spawnSync defaults windowsHide to true', () => {
|
||||
const result = Bun.spawnSync(['node', '-e', `
|
||||
const cp = require('child_process');
|
||||
const orig = cp.spawnSync;
|
||||
let seen;
|
||||
cp.spawnSync = (c, a, o) => { seen = o; return orig(c, a, o); };
|
||||
require('${requirePath}');
|
||||
Bun.spawnSync(['node', '-e', '']);
|
||||
console.log('windowsHide:' + seen.windowsHide);
|
||||
`], { stdout: 'pipe', stderr: 'pipe' });
|
||||
expect(result.stdout.toString().trim()).toBe('windowsHide:true');
|
||||
});
|
||||
|
||||
test('an explicit windowsHide:false is honored', () => {
|
||||
const result = Bun.spawnSync(['node', '-e', `
|
||||
const cp = require('child_process');
|
||||
const orig = cp.spawn;
|
||||
let seen;
|
||||
cp.spawn = (c, a, o) => { seen = o; return orig(c, a, o); };
|
||||
require('${requirePath}');
|
||||
Bun.spawn(['node', '-e', ''], { stdio: ['ignore', 'ignore', 'ignore'], windowsHide: false });
|
||||
console.log('windowsHide:' + seen.windowsHide);
|
||||
`], { stdout: 'pipe', stderr: 'pipe' });
|
||||
expect(result.stdout.toString().trim()).toBe('windowsHide:false');
|
||||
});
|
||||
|
||||
test('Bun.serve creates an HTTP server that responds', async () => {
|
||||
const result = Bun.spawnSync(['node', '-e', `
|
||||
require('${polyfillPath}');
|
||||
require('${requirePath}');
|
||||
const server = Bun.serve({
|
||||
port: 0, // Note: polyfill uses port directly, so we pick one
|
||||
hostname: '127.0.0.1',
|
||||
|
|
|
|||
Loading…
Reference in New Issue