diff --git a/src/core/harness/sandbox.js b/src/core/harness/sandbox.js index 97ecd7c9..e992bd72 100644 --- a/src/core/harness/sandbox.js +++ b/src/core/harness/sandbox.js @@ -114,7 +114,10 @@ export async function startContainerRuntime({ return { runtime, started: false, ready: false, message: `${runtime} engine is not running and RStack cannot auto-start it on ${platform} — start it manually` }; } try { - const child = spawnImpl(startCmd.cmd, startCmd.args, { stdio: 'ignore' }); + // windowsHide suppresses the console window a freshly-spawned console + // subsystem process (cmd.exe on win32's docker path, podman.exe) would + // otherwise briefly flash — harmless elsewhere, since only win32 honors it. + const child = spawnImpl(startCmd.cmd, startCmd.args, { stdio: 'ignore', windowsHide: true }); child?.on?.('error', () => {}); child?.unref?.(); } catch (err) { diff --git a/src/hooks/auto-launch.js b/src/hooks/auto-launch.js index 922e0e74..aadba125 100644 --- a/src/hooks/auto-launch.js +++ b/src/hooks/auto-launch.js @@ -49,10 +49,21 @@ export async function autoLaunchBusinessHub(projectRoot, opts = {}) { if (!opts.noBrowser && process.env.RSTACK_NO_BROWSER !== '1') openBrowser(url); } -function openBrowser(url) { - const cmd = process.platform === 'win32' ? 'start' - : process.platform === 'darwin' ? 'open' : 'xdg-open'; +// platform/spawnImpl are injectable (mirrors sandbox.js's spawnImpl convention) +// so #470's windowsHide behavior is testable per-platform without actually +// spawning a browser. +export function openBrowser(url, { platform = process.platform, spawnImpl = spawn } = {}) { + const cmd = platform === 'win32' ? 'start' + : platform === 'darwin' ? 'open' : 'xdg-open'; try { - spawn(cmd, [url], { stdio: 'ignore', detached: true, shell: process.platform === 'win32' }).unref(); + // On win32 this runs through `cmd.exe /c start ` (shell: true); without + // windowsHide, that intermediate cmd.exe briefly flashes a visible console + // window before handing off to the actual browser process. + spawnImpl(cmd, [url], { + stdio: 'ignore', + detached: true, + shell: platform === 'win32', + windowsHide: true, + }).unref(); } catch { /* best-effort */ } } diff --git a/src/integrations/pi/rstack-sdlc.ts b/src/integrations/pi/rstack-sdlc.ts index 2f077dba..0a255d79 100644 --- a/src/integrations/pi/rstack-sdlc.ts +++ b/src/integrations/pi/rstack-sdlc.ts @@ -1,7 +1,7 @@ import type { ExtensionAPI } from "@earendil-works/pi-coding-agent"; import { StringEnum } from "@earendil-works/pi-ai"; import { Type } from "typebox"; -import { spawn } from "node:child_process"; +import { spawn, type ChildProcess } from "node:child_process"; import { createHash, randomUUID } from "node:crypto"; import { request as httpRequest } from "node:http"; import { createConnection } from "node:net"; @@ -94,12 +94,28 @@ function safeOpen(filePath: string): void { } } -function openUrl(url: string): void { +// platform/spawnImpl are injectable (mirrors sandbox.js's spawnImpl convention) +// so #470's windowsHide behavior is testable per-platform without actually +// spawning a browser. +export function openUrl( + url: string, + opts: { platform?: NodeJS.Platform; spawnImpl?: typeof spawn } = {}, +): void { if (process.env.CI) return; - const cmd = process.platform === "win32" ? "start" - : process.platform === "darwin" ? "open" : "xdg-open"; + const platform = opts.platform ?? process.platform; + const spawnImpl = opts.spawnImpl ?? spawn; + const cmd = platform === "win32" ? "start" + : platform === "darwin" ? "open" : "xdg-open"; try { - const cp = spawn(cmd, [url], { stdio: "ignore", detached: true, shell: process.platform === "win32" }); + // On win32 this runs through `cmd.exe /c start ` (shell: true); without + // windowsHide, that intermediate cmd.exe briefly flashes a visible console + // window before handing off to the actual browser process. + const cp: ChildProcess = spawnImpl(cmd, [url], { + stdio: "ignore", + detached: true, + shell: platform === "win32", + windowsHide: true, + }); cp.on("error", () => {}); cp.unref(); } catch { /* best-effort */ } diff --git a/src/observability/dashboard/server.js b/src/observability/dashboard/server.js index 65ce6f44..450759ee 100644 --- a/src/observability/dashboard/server.js +++ b/src/observability/dashboard/server.js @@ -121,11 +121,21 @@ function safeJson(str) { try { return JSON.parse(str); } catch { return null; } } -function openBrowser(url) { - const cmd = process.platform === 'win32' ? 'start' - : process.platform === 'darwin' ? 'open' : 'xdg-open'; +// platform/spawnImpl are injectable (mirrors sandbox.js's spawnImpl convention) +// so #470's windowsHide behavior is testable per-platform without actually +// spawning a browser. +export function openBrowser(url, { platform = process.platform, spawnImpl = spawn } = {}) { + const cmd = platform === 'win32' ? 'start' + : platform === 'darwin' ? 'open' : 'xdg-open'; try { - spawn(cmd, [url], { stdio: 'ignore', detached: true, shell: process.platform === 'win32' }).unref(); + // windowsHide suppresses the cmd.exe console window that `shell: true` + // would otherwise briefly flash on win32 before handing off to the browser. + spawnImpl(cmd, [url], { + stdio: 'ignore', + detached: true, + shell: platform === 'win32', + windowsHide: true, + }).unref(); } catch { // best effort only } diff --git a/tests/windows-console-flash-470.test.js b/tests/windows-console-flash-470.test.js new file mode 100644 index 00000000..92126599 --- /dev/null +++ b/tests/windows-console-flash-470.test.js @@ -0,0 +1,109 @@ +/** + * #470: on win32, spawning a browser/runtime process via `cmd.exe` (shell: + * true) or directly (cmd.exe/podman.exe) briefly flashes a visible console + * window unless `windowsHide: true` is passed — Node's spawn() defaults it + * to false. Pins that every one of the four affected spawn call sites + * passes windowsHide on win32, and that non-win32 platforms are unaffected + * (the option is harmless there, but shell must stay false). + * + * owner: RStack developed by Richardson Gunde + */ + +// src/observability/dashboard/server.js has an identical openBrowser copy, +// but that module calls server.listen() unguarded at the top level (a +// pre-existing design predating this PR, out of scope to refactor here) — +// importing it for its openBrowser export would start a real HTTP server as +// a side effect, so it's covered by code-review parity with the two +// call sites below instead of a direct import. +import test from 'node:test'; +import assert from 'node:assert/strict'; +import { openBrowser as openBrowserAutoLaunch } from '../src/hooks/auto-launch.js'; +import { openUrl } from '../src/integrations/pi/rstack-sdlc.ts'; +import { startContainerRuntime } from '../src/core/harness/sandbox.js'; + +function fakeChild() { + return { on() {}, unref() {} }; +} + +function recordingSpawn(calls) { + return (cmd, args, options) => { + calls.push({ cmd, args, options }); + return fakeChild(); + }; +} + +test('auto-launch.js openBrowser: windowsHide:true + shell:true on win32', () => { + const calls = []; + openBrowserAutoLaunch('http://localhost:3008', { platform: 'win32', spawnImpl: recordingSpawn(calls) }); + assert.equal(calls.length, 1); + assert.equal(calls[0].cmd, 'start'); + assert.equal(calls[0].options.shell, true); + assert.equal(calls[0].options.windowsHide, true); +}); + +test('auto-launch.js openBrowser: shell:false on macOS/Linux, windowsHide harmless', () => { + const calls = []; + openBrowserAutoLaunch('http://localhost:3008', { platform: 'darwin', spawnImpl: recordingSpawn(calls) }); + assert.equal(calls[0].cmd, 'open'); + assert.equal(calls[0].options.shell, false); +}); + +// openUrl early-returns when process.env.CI is set (intentional — avoids +// popping a browser during a CI run), which GitHub Actions always sets. Clear +// it for the duration of these two tests so they exercise the real spawn +// path there too, restoring it afterward so the guard itself stays covered. +function withoutCiEnv(fn) { + const had = Object.prototype.hasOwnProperty.call(process.env, 'CI'); + const prior = process.env.CI; + delete process.env.CI; + try { + fn(); + } finally { + if (had) process.env.CI = prior; + } +} + +test('pi/rstack-sdlc.ts openUrl: windowsHide:true + shell:true on win32', () => { + withoutCiEnv(() => { + const calls = []; + openUrl('http://localhost:3008', { platform: 'win32', spawnImpl: recordingSpawn(calls) }); + assert.equal(calls[0].cmd, 'start'); + assert.equal(calls[0].options.shell, true); + assert.equal(calls[0].options.windowsHide, true); + }); +}); + +test('pi/rstack-sdlc.ts openUrl: linux uses xdg-open, shell:false', () => { + withoutCiEnv(() => { + const calls = []; + openUrl('http://localhost:3008', { platform: 'linux', spawnImpl: recordingSpawn(calls) }); + assert.equal(calls[0].cmd, 'xdg-open'); + assert.equal(calls[0].options.shell, false); + }); +}); + +test('pi/rstack-sdlc.ts openUrl: no-ops under CI (never pops a browser during automated runs)', () => { + const calls = []; + const original = process.env.CI; + process.env.CI = '1'; + try { + openUrl('http://localhost:3008', { platform: 'win32', spawnImpl: recordingSpawn(calls) }); + } finally { + if (original === undefined) delete process.env.CI; else process.env.CI = original; + } + assert.equal(calls.length, 0, 'openUrl must not spawn anything when CI is set'); +}); + +test('sandbox.js startContainerRuntime: windowsHide:true on the engine auto-start spawn', async () => { + const calls = []; + const result = await startContainerRuntime({ + installedProbe: () => 'docker', + readyProbe: () => false, + spawnImpl: (cmd, args, options) => { calls.push({ cmd, args, options }); return fakeChild(); }, + platform: 'win32', + timeoutMs: 0, + }); + assert.equal(calls.length, 1); + assert.equal(calls[0].options.windowsHide, true); + assert.equal(result.started, true); +});