mirror of https://github.com/garrytan/gstack.git
Merge a8b5bc59ec into 2beb636f7c
This commit is contained in:
commit
98f3394929
|
|
@ -190,6 +190,7 @@ async function handleTest(args: string[], ctx: SkillCommandContext): Promise<str
|
|||
stdout: 'pipe',
|
||||
stderr: 'pipe',
|
||||
env: process.env,
|
||||
windowsHide: true, // don't pop a console window on Windows (see win-console-hide.ts)
|
||||
});
|
||||
const exitCode = await proc.exited;
|
||||
const stdout = proc.stdout ? await new Response(proc.stdout).text() : '';
|
||||
|
|
@ -268,6 +269,7 @@ export async function spawnSkill(opts: SpawnSkillOptions): Promise<SpawnSkillRes
|
|||
env,
|
||||
stdout: 'pipe',
|
||||
stderr: 'pipe',
|
||||
windowsHide: true, // don't pop a console window on Windows (see win-console-hide.ts)
|
||||
});
|
||||
|
||||
let timedOut = false;
|
||||
|
|
|
|||
|
|
@ -319,11 +319,19 @@ async function startServer(extraEnv?: Record<string, string>): Promise<ServerSta
|
|||
// when the CLI exits, the server dies with it. Use Node's child_process.spawn
|
||||
// with { detached: true } instead, which is the gold standard for Windows
|
||||
// process independence. Credit: PR #191 by @fqueiro.
|
||||
//
|
||||
// windowsHide: true is required alongside detached. Without it the detached
|
||||
// daemon allocates a console, and on Windows 11 with "Default terminal
|
||||
// application = Windows Terminal" that console is handed off to a visible
|
||||
// Windows Terminal window/tab that lingers for the daemon's whole lifetime.
|
||||
// windowsHide maps to CREATE_NO_WINDOW, which suppresses both the console
|
||||
// and the terminal handoff. The daemon already uses stdio:'ignore', so it
|
||||
// never needs a console.
|
||||
const extraEnvStr = JSON.stringify({ BROWSE_STATE_FILE: config.stateFile, BROWSE_PARENT_PID: parentPid, ...(extraEnv || {}) });
|
||||
const launcherCode =
|
||||
`const{spawn}=require('child_process');` +
|
||||
`spawn(process.execPath,[${JSON.stringify(NODE_SERVER_SCRIPT)}],` +
|
||||
`{detached:true,stdio:['ignore','ignore','ignore'],env:Object.assign({},process.env,` +
|
||||
`{detached:true,windowsHide:true,stdio:['ignore','ignore','ignore'],env:Object.assign({},process.env,` +
|
||||
`${extraEnvStr})}).unref()`;
|
||||
Bun.spawnSync(['node', '-e', launcherCode], { stdio: ['ignore', 'ignore', 'ignore'] });
|
||||
} else {
|
||||
|
|
|
|||
|
|
@ -494,6 +494,10 @@ function reportAttemptTelemetry(record: AttemptRecord): void {
|
|||
const child = spawn(result.cmd, result.cmdArgs, {
|
||||
stdio: 'ignore',
|
||||
detached: true,
|
||||
// windowsHide (CREATE_NO_WINDOW) keeps this fire-and-forget reporter from
|
||||
// popping a console window on Windows 11 when the default terminal is
|
||||
// Windows Terminal. No-op on macOS/Linux. See cli.ts startServer().
|
||||
windowsHide: true,
|
||||
});
|
||||
// unref so this subprocess doesn't hold the event loop open
|
||||
child.unref();
|
||||
|
|
|
|||
|
|
@ -13,6 +13,11 @@
|
|||
* Port: random 10000-60000 (or BROWSE_PORT env for debug override)
|
||||
*/
|
||||
|
||||
// MUST be first: patches child_process.spawn to default windowsHide on Windows
|
||||
// before Playwright (pulled in via BrowserManager below) ever launches the
|
||||
// browser, so the detached daemon's console-less children don't pop stray
|
||||
// terminal windows. No-op off Windows.
|
||||
import './win-console-hide';
|
||||
import { BrowserManager } from './browser-manager';
|
||||
import { handleReadCommand, hasOutArg } from './read-commands';
|
||||
import { handleWriteCommand } from './write-commands';
|
||||
|
|
|
|||
|
|
@ -0,0 +1,60 @@
|
|||
/**
|
||||
* Windows console-window suppression for the browse daemon.
|
||||
*
|
||||
* The daemon is launched detached (see cli.ts startServer), which on Windows
|
||||
* means it runs with NO console. Any console child it then spawns —
|
||||
* chrome-headless-shell.exe via Playwright, `bun` helpers, the taskkill cleanup —
|
||||
* finds no console to inherit, so Windows allocates a fresh VISIBLE console
|
||||
* window for it. That's the stray terminal windows users see during browse use.
|
||||
*
|
||||
* The fix is windowsHide (CREATE_NO_WINDOW) on the CHILD: it suppresses the
|
||||
* window even when the parent has no console. Verified on Windows 11: a child of
|
||||
* a detached/console-less parent is visible without windowsHide and hidden with
|
||||
* it. Playwright does not pass windowsHide (it spawns the browser via
|
||||
* `childProcess.spawn(...)` in playwright-core's processLauncher with no such
|
||||
* option), and we can't pass spawn options through `chromium.launch()`. So we
|
||||
* default windowsHide ON for every child_process spawn in this process.
|
||||
*
|
||||
* Why this works despite Playwright loading first: playwright-core captures
|
||||
* child_process via `__toESM(require("child_process"))`, whose property access
|
||||
* (`childProcess.spawn`) is a live getter that reads the underlying singleton at
|
||||
* call time. Patching the singleton's spawn/spawnSync before the first
|
||||
* `launch()` (i.e. during daemon startup, which is what importing this module
|
||||
* first in server.ts guarantees) is therefore sufficient.
|
||||
*
|
||||
* No-op on macOS/Linux.
|
||||
*/
|
||||
import { createRequire } from 'node:module';
|
||||
|
||||
if (process.platform === 'win32') {
|
||||
// An ESM `import * as cp` namespace is read-only and can't be patched.
|
||||
// createRequire yields the mutable, cached CJS child_process exports — the
|
||||
// same singleton object playwright-core's require("child_process") returns.
|
||||
const require_ = createRequire(import.meta.url);
|
||||
const cp = require_('child_process') as Record<string, unknown>;
|
||||
|
||||
// Normalize spawn(cmd, options) vs spawn(cmd, args, options) and default
|
||||
// windowsHide:true when the caller didn't set it explicitly.
|
||||
const withHide = (args: unknown, options: unknown): [unknown, Record<string, unknown>] => {
|
||||
let opts: Record<string, unknown>;
|
||||
if (args && !Array.isArray(args) && typeof args === 'object') {
|
||||
opts = args as Record<string, unknown>;
|
||||
args = undefined;
|
||||
} else {
|
||||
opts = (options as Record<string, unknown>) || {};
|
||||
}
|
||||
if (opts.windowsHide === undefined) opts.windowsHide = true;
|
||||
return [args, opts];
|
||||
};
|
||||
|
||||
for (const name of ['spawn', 'spawnSync'] as const) {
|
||||
const orig = cp[name] as ((...a: unknown[]) => unknown) & { __gstackHidden?: boolean };
|
||||
if (typeof orig !== 'function' || orig.__gstackHidden) continue;
|
||||
const patched = function (this: unknown, command: unknown, args?: unknown, options?: unknown) {
|
||||
const [a, o] = withHide(args, options);
|
||||
return a === undefined ? orig.call(this, command, o) : orig.call(this, command, a, o);
|
||||
} as ((...a: unknown[]) => unknown) & { __gstackHidden?: boolean };
|
||||
patched.__gstackHidden = true;
|
||||
cp[name] = patched;
|
||||
}
|
||||
}
|
||||
|
|
@ -0,0 +1,60 @@
|
|||
import { describe, test, expect } from 'bun:test';
|
||||
import * as fs from 'fs';
|
||||
import * as path from 'path';
|
||||
|
||||
// REGRESSION TEST for the Windows "stray terminal window" class of bug.
|
||||
//
|
||||
// The browse daemon is launched detached (cli.ts startServer), which on Windows
|
||||
// means it has NO console. Any console child it then spawns — chrome-headless-
|
||||
// shell.exe via Playwright, `bun` helpers, the taskkill cleanup — finds no
|
||||
// console to inherit, so Windows allocates a fresh VISIBLE console window for
|
||||
// it. windowsHide (CREATE_NO_WINDOW) on the CHILD suppresses that window even
|
||||
// when the parent has no console (verified on Windows 11).
|
||||
//
|
||||
// The primary fix is win-console-hide.ts: the daemon patches
|
||||
// child_process.spawn/spawnSync to default windowsHide:true, so Playwright's
|
||||
// browser launch (which we can't pass spawn options to) is covered. gstack's own
|
||||
// Bun.spawn helpers and the detached/telemetry spawns set windowsHide directly.
|
||||
//
|
||||
// Static-grep tripwire — read source, assert the invariant. Pattern mirrors
|
||||
// browse/test/terminal-agent-pid-identity.test.ts. Uses import.meta.dir, NOT
|
||||
// new URL(import.meta.url).pathname (which yields "/C:/..." -> doubled-drive
|
||||
// "C:\C:\..." on Windows).
|
||||
const SRC_DIR = path.resolve(import.meta.dir, '..', 'src');
|
||||
|
||||
function read(file: string): string {
|
||||
return fs.readFileSync(path.join(SRC_DIR, file), 'utf-8');
|
||||
}
|
||||
|
||||
describe('Windows detached spawns hide the console window', () => {
|
||||
test('1. win-console-hide.ts patches spawn and spawnSync with windowsHide', () => {
|
||||
const m = read('win-console-hide.ts');
|
||||
expect(m).toContain("process.platform === 'win32'");
|
||||
expect(m).toMatch(/windowsHide\s*=\s*true/);
|
||||
// Must patch both window-creating entry points.
|
||||
expect(m).toMatch(/'spawn',\s*'spawnSync'/);
|
||||
});
|
||||
|
||||
test('2. server.ts imports win-console-hide before BrowserManager (=> before Playwright)', () => {
|
||||
const s = read('server.ts');
|
||||
const hide = s.indexOf("./win-console-hide");
|
||||
const bm = s.indexOf("./browser-manager");
|
||||
expect(hide).toBeGreaterThanOrEqual(0);
|
||||
expect(bm).toBeGreaterThanOrEqual(0);
|
||||
expect(hide).toBeLessThan(bm); // ordering is load-bearing
|
||||
});
|
||||
|
||||
test('3. browser-skill-commands.ts bun spawns set windowsHide', () => {
|
||||
const c = read('browser-skill-commands.ts');
|
||||
const hits = c.match(/windowsHide:\s*true/g) || [];
|
||||
expect(hits.length).toBeGreaterThanOrEqual(2);
|
||||
});
|
||||
|
||||
test('4. cli.ts Windows server launcher passes windowsHide alongside detached', () => {
|
||||
expect(read('cli.ts')).toContain('detached:true,windowsHide:true,');
|
||||
});
|
||||
|
||||
test('5. security.ts attack-telemetry reporter passes windowsHide alongside detached', () => {
|
||||
expect(read('security.ts')).toMatch(/detached:\s*true,[\s\S]{0,400}?windowsHide:\s*true/);
|
||||
});
|
||||
});
|
||||
Loading…
Reference in New Issue