Fix Codex browse runtime on Windows

browse/scripts/build-node-server.sh

@@ -25,6 +25,7 @@ bun build "$SRC_DIR/server.ts" \
   --external playwright \
   --external playwright-core \
   --external diff \
+  --external sharp \
   --external "bun:sqlite" \
   --external "@ngrok/ngrok"
 

browse/src/cli.ts

@@ -11,7 +11,7 @@
 
 import * as fs from 'fs';
 import * as path from 'path';
-import { spawn as nodeSpawn } from 'child_process';
+import { spawn as nodeSpawn, spawnSync } from 'child_process';
 import { safeUnlink, safeUnlinkQuiet, safeKill, isProcessAlive } from './error-handling';
 import { writeSecureFile, mkdirSecure } from './file-permissions';
 import { resolveConfig, ensureStateDir, readVersionHash } from './config';
@@ -21,7 +21,9 @@ import { spawnTerminalAgent } from './terminal-agent-control';
 
 const config = resolveConfig();
 const IS_WINDOWS = process.platform === 'win32';
-const MAX_START_WAIT = IS_WINDOWS ? 15000 : (process.env.CI ? 30000 : 8000); // Node+Chromium takes longer on Windows
+const DEFAULT_START_WAIT = IS_WINDOWS ? 45000 : (process.env.CI ? 30000 : 8000); // Node+Chromium takes longer on Windows
+const MAX_START_WAIT = Number.parseInt(process.env.BROWSE_START_WAIT_MS || '', 10) || DEFAULT_START_WAIT;
+let startedServerThisRun = false;
 
 export function resolveServerScript(
   env: Record<string, string | undefined> = process.env,
@@ -229,16 +231,15 @@ async function startServer(extraEnv?: Record<string, string>): Promise<ServerSta
 
   if (IS_WINDOWS && NODE_SERVER_SCRIPT) {
     // Windows: Bun.spawn() + proc.unref() doesn't truly detach on Windows —
-    // 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.
+    // when the CLI exits, the server dies with it. Use a tiny Node launcher
+    // with { detached: true }, which is the reliable Windows detach path.
     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,` +
       `${extraEnvStr})}).unref()`;
-    Bun.spawnSync(['node', '-e', launcherCode], { stdio: ['ignore', 'ignore', 'ignore'] });
+    spawnSync('node', ['-e', launcherCode], { stdio: 'ignore' });
   } else {
     // macOS/Linux: Bun.spawn().unref() only removes the child from Bun's event
     // loop — it does NOT call setsid(), so the spawned server stays in the
@@ -265,6 +266,7 @@ async function startServer(extraEnv?: Record<string, string>): Promise<ServerSta
   while (Date.now() - start < MAX_START_WAIT) {
     const state = readState();
     if (state && await isServerHealthy(state.port)) {
+      startedServerThisRun = true;
       return state;
     }
     await Bun.sleep(100);
@@ -384,7 +386,10 @@ async function ensureServer(flags?: GlobalFlags): Promise<ServerState> {
     const start = Date.now();
     while (Date.now() - start < MAX_START_WAIT) {
       const freshState = readState();
-      if (freshState && await isServerHealthy(freshState.port)) return freshState;
+      if (freshState && await isServerHealthy(freshState.port)) {
+        startedServerThisRun = true;
+        return freshState;
+      }
       await Bun.sleep(200);
     }
     throw new Error('Timed out waiting for another instance to start the server');
@@ -394,6 +399,7 @@ async function ensureServer(flags?: GlobalFlags): Promise<ServerState> {
     // Re-read state under lock in case another process just started the server
     const freshState = readState();
     if (freshState && await isServerHealthy(freshState.port)) {
+      startedServerThisRun = true;
       return freshState;
     }
 
@@ -405,8 +411,6 @@ async function ensureServer(flags?: GlobalFlags): Promise<ServerState> {
       console.error(`[browse] Starting server with proxy ${flags.redactedProxyUrl}${flags.headed ? ' (headed)' : ''}...`);
     } else if (flags?.headed) {
       console.error('[browse] Starting server in headed mode...');
-    } else {
-      console.error('[browse] Starting server...');
     }
     return await startServer(extraEnv);
   } finally {
@@ -469,10 +473,8 @@ async function sendCommand(state: ServerState, command: string, args: string[],
     }
 
     const text = await resp.text();
-
     if (resp.ok) {
-      process.stdout.write(text);
-      if (!text.endsWith('\n')) process.stdout.write('\n');
+      await writeStdout(text);
     } else {
       // Try to parse as JSON error
       try {
@@ -489,8 +491,24 @@ async function sendCommand(state: ServerState, command: string, args: string[],
       console.error('[browse] Command timed out after 30s');
       process.exit(1);
     }
-    // Connection error — server may have crashed
+    // `stop` intentionally tears the daemon down. On Windows/Node the socket
+    // can close before the response body reaches the CLI; treat that as a
+    // successful stop instead of triggering the generic crash-restart path.
     if (err.code === 'ECONNREFUSED' || err.code === 'ECONNRESET' || err.message?.includes('fetch failed')) {
+      if (command === 'stop' && !(await isServerHealthy(state.port))) {
+        safeUnlinkQuiet(config.stateFile);
+        await writeStdout('Server stopped');
+        return;
+      }
+      if (command === 'restart' && !(await isServerHealthy(state.port))) {
+        const restartEnv: Record<string, string> = {};
+        if (_globalFlags?.proxyUrl) restartEnv.BROWSE_PROXY_URL = _globalFlags.proxyUrl;
+        if (_globalFlags?.headed) restartEnv.BROWSE_HEADED = '1';
+        if (_globalFlags?.configHash) restartEnv.BROWSE_CONFIG_HASH = _globalFlags.configHash;
+        await startServer(Object.keys(restartEnv).length ? restartEnv : undefined);
+        await writeStdout('Server restarted');
+        return;
+      }
       if (retries >= 1) throw new Error('[browse] Server crashed twice in a row — aborting');
       console.error('[browse] Server connection lost. Restarting...');
       // Kill the old server to avoid orphaned chromium processes
@@ -513,6 +531,32 @@ async function sendCommand(state: ServerState, command: string, args: string[],
   }
 }
 
+async function writeStdout(text: string): Promise<void> {
+  const output = text.endsWith('\n') ? text : `${text}\n`;
+  fs.writeSync(1, output);
+}
+
+async function handleStopCommand(commandArgs: string[]): Promise<void> {
+  const state = readState();
+  if (!state) {
+    await writeStdout('Server not running');
+    return;
+  }
+
+  if (await isServerHealthy(state.port)) {
+    await sendCommand(state, 'stop', commandArgs);
+    return;
+  }
+
+  if (state.pid && isProcessAlive(state.pid)) {
+    await killServer(state.pid);
+    await writeStdout('Server stopped');
+  } else {
+    await writeStdout('Server not running');
+  }
+  safeUnlinkQuiet(config.stateFile);
+}
+
 // Module-level reference to the resolved global flags from main(). Used by
 // sendCommand's crash-retry path so a daemon restart after ECONNRESET doesn't
 // silently drop --proxy / --headed.
@@ -1220,6 +1264,15 @@ Refs:           After 'snapshot', use @e1, @e2... as selectors:
     process.exit(0);
   }
 
+  // stop must never auto-start a daemon. The generic command path calls
+  // ensureServer(), which is correct for normal browser commands but wrong for
+  // shutdown: `browse stop` from a clean state should be a no-op, not a
+  // start-then-stop cycle that can leave a detached Windows process behind.
+  if (command === 'stop') {
+    await handleStopCommand(commandArgs);
+    process.exit(0);
+  }
+
   // Special case: chain reads from stdin
   if (command === 'chain' && commandArgs.length === 0) {
     const stdin = await Bun.stdin.text();
@@ -1228,6 +1281,18 @@ Refs:           After 'snapshot', use @e1, @e2... as selectors:
 
   let state = await ensureServer(globalFlags);
 
+  if (startedServerThisRun && process.env.BROWSE_SKIP_REEXEC_AFTER_START !== '1') {
+    const result = spawnSync(process.execPath, process.argv.slice(2), {
+      stdio: ['ignore', 'pipe', 'pipe'],
+      encoding: 'utf8',
+      env: { ...process.env, BROWSE_SKIP_REEXEC_AFTER_START: '1' },
+    });
+    if (result.error) throw result.error;
+    if (result.stdout) fs.writeSync(1, result.stdout);
+    if (result.stderr) fs.writeSync(2, result.stderr);
+    process.exit(result.status ?? 1);
+  }
+
   // ─── Pair-Agent (post-server, pre-dispatch) ──────────────
   if (command === 'pair-agent') {
     // Ensure headed mode — the user should see the browser window

browse/src/write-commands.ts

@@ -128,6 +128,24 @@ const CLEANUP_SELECTORS = {
   ],
 };
 
+async function withManualTimeout<T>(
+  promise: Promise<T>,
+  timeoutMs: number
+): Promise<{ timedOut: false; value: T } | { timedOut: true }> {
+  promise.catch(() => {});
+  let timeout: ReturnType<typeof setTimeout> | undefined;
+  try {
+    return await Promise.race([
+      promise.then((value) => ({ timedOut: false as const, value })),
+      new Promise<{ timedOut: true }>((resolve) => {
+        timeout = setTimeout(() => resolve({ timedOut: true }), timeoutMs);
+      }),
+    ]);
+  } finally {
+    if (timeout) clearTimeout(timeout);
+  }
+}
+
 export async function handleWriteCommand(
   command: string,
   args: string[],
@@ -148,9 +166,17 @@ export async function handleWriteCommand(
       // must not leave stale content that could resurrect on a later context recreation.
       session.clearLoadedHtml();
       const normalizedUrl = await validateNavigationUrl(url);
-      const response = await page.goto(normalizedUrl, { waitUntil: 'domcontentloaded', timeout: 15000 });
+      const response = await page.goto(normalizedUrl, { waitUntil: 'commit', timeout: 15000 });
+      const domReady = await withManualTimeout(
+        page.waitForLoadState('domcontentloaded', { timeout: 15000 }),
+        16000
+      );
+      if (domReady.timedOut) {
+        await page.evaluate(() => window.stop()).catch(() => {});
+      }
       const status = response?.status() || 'unknown';
-      return `Navigated to ${normalizedUrl} (${status})`;
+      const suffix = domReady.timedOut ? '; domcontentloaded timed out' : '';
+      return `Navigated to ${normalizedUrl} (${status}${suffix})`;
     }
 
     case 'back': {

browse/test/commands.test.ts

@@ -33,7 +33,7 @@ beforeAll(async () => {
 
   bm = new BrowserManager();
   await bm.launch();
-});
+}, 30000);
 
 afterAll(() => {
   // Force kill browser instead of graceful close (avoids hang)

browse/test/config.test.ts

@@ -315,6 +315,45 @@ describe('startup error log', () => {
   });
 });
 
+describe('cli command dispatch', () => {
+  const cliSource = fs.readFileSync(path.resolve(__dirname, '../src/cli.ts'), 'utf-8');
+
+  test('handles stop before ensureServer so shutdown never auto-starts a daemon', () => {
+    const stopDispatch = cliSource.indexOf('await handleStopCommand(commandArgs)');
+    const ensureServerCall = cliSource.indexOf('let state = await ensureServer(globalFlags)');
+
+    expect(stopDispatch).toBeGreaterThan(-1);
+    expect(ensureServerCall).toBeGreaterThan(-1);
+    expect(stopDispatch).toBeLessThan(ensureServerCall);
+  });
+
+  test('cold-start re-exec preserves command stdout on stdout', () => {
+    expect(cliSource).toContain('if (result.stdout) fs.writeSync(1, result.stdout)');
+    expect(cliSource).not.toContain('IS_WINDOWS ? 2 : 1, result.stdout');
+  });
+
+  test('restart connection loss starts once instead of resending restart', () => {
+    expect(cliSource).toContain("if (command === 'restart' && !(await isServerHealthy(state.port)))");
+    expect(cliSource).toContain("await writeStdout('Server restarted')");
+  });
+
+  test('default headless cold-start does not print a delayed startup banner', () => {
+    expect(cliSource).not.toContain("console.error('[browse] Starting server...')");
+    expect(cliSource).toContain('Starting server in headed mode');
+    expect(cliSource).toContain('Starting server with proxy');
+  });
+});
+
+describe('write command dispatch', () => {
+  const writeSource = fs.readFileSync(path.resolve(__dirname, '../src/write-commands.ts'), 'utf-8');
+
+  test('goto commits first and bounds domcontentloaded wait', () => {
+    expect(writeSource).toContain("page.goto(normalizedUrl, { waitUntil: 'commit', timeout: 15000 })");
+    expect(writeSource).toContain("page.waitForLoadState('domcontentloaded', { timeout: 15000 })");
+    expect(writeSource).toContain("await page.evaluate(() => window.stop()).catch(() => {})");
+  });
+});
+
 describe('resolveGstackHome', () => {
   test('honors GSTACK_HOME env var when set', () => {
     const orig = process.env.GSTACK_HOME;
@@ -367,7 +406,7 @@ describe('resolveChromiumProfile', () => {
     delete process.env.CHROMIUM_PROFILE;
     process.env.GSTACK_HOME = '/tmp/fallback-gstack';
     try {
-      expect(resolveChromiumProfile()).toBe('/tmp/fallback-gstack/chromium-profile');
+      expect(resolveChromiumProfile()).toBe(path.join('/tmp/fallback-gstack', 'chromium-profile'));
     } finally {
       if (origEnv !== undefined) process.env.CHROMIUM_PROFILE = origEnv;
       if (origHome === undefined) delete process.env.GSTACK_HOME;

package.json

@@ -49,6 +49,7 @@
     "marked": "^18.0.2",
     "playwright": "^1.58.2",
     "puppeteer-core": "^24.40.0",
+    "sharp": "^0.34.5",
     "socks": "^2.8.8"
   },
   "engines": {

setup

@@ -668,6 +668,48 @@ create_agents_sidecar() {
   done
 }
 
+link_browse_runtime_assets() {
+  local gstack_dir="$1"
+  local runtime_root="$2"
+
+  mkdir -p "$runtime_root/browse" "$runtime_root/node_modules"
+
+  if [ -d "$gstack_dir/browse/dist" ]; then
+    _link_or_copy "$gstack_dir/browse/dist" "$runtime_root/browse/dist"
+  fi
+  # The compiled Windows CLI still resolves browse/src/server.ts at startup
+  # before it delegates to the Node-compatible server bundle.
+  if [ -d "$gstack_dir/browse/src" ]; then
+    _link_or_copy "$gstack_dir/browse/src" "$runtime_root/browse/src"
+  fi
+  if [ -d "$gstack_dir/browse/bin" ]; then
+    _link_or_copy "$gstack_dir/browse/bin" "$runtime_root/browse/bin"
+  fi
+
+  # server-node.mjs intentionally externalizes these runtime packages.
+  # sharp is used by screenshot size-guard and needs its platform-specific
+  # native @img/* sidecars present in the copied Codex runtime root.
+  for dep in playwright playwright-core diff sharp semver detect-libc; do
+    if [ -d "$gstack_dir/node_modules/$dep" ]; then
+      _link_or_copy "$gstack_dir/node_modules/$dep" "$runtime_root/node_modules/$dep"
+    fi
+  done
+  if [ -d "$gstack_dir/node_modules/@img" ]; then
+    mkdir -p "$runtime_root/node_modules/@img"
+    for scoped_dep in "$gstack_dir/node_modules/@img"/*; do
+      [ -e "$scoped_dep" ] || continue
+      _link_or_copy "$scoped_dep" "$runtime_root/node_modules/@img/$(basename "$scoped_dep")"
+    done
+  fi
+  if [ -d "$gstack_dir/node_modules/@ngrok" ]; then
+    mkdir -p "$runtime_root/node_modules/@ngrok"
+    for scoped_dep in "$gstack_dir/node_modules/@ngrok"/*; do
+      [ -e "$scoped_dep" ] || continue
+      _link_or_copy "$scoped_dep" "$runtime_root/node_modules/@ngrok/$(basename "$scoped_dep")"
+    done
+  fi
+}
+
 # ─── Helper: create a minimal ~/.codex/skills/gstack runtime root ───────────
 # Codex scans ~/.codex/skills recursively. Exposing the whole repo here causes
 # duplicate skills because source SKILL.md files and generated Codex skills are
@@ -693,12 +735,7 @@ create_codex_runtime_root() {
   if [ -d "$gstack_dir/bin" ]; then
     _link_or_copy "$gstack_dir/bin" "$codex_gstack/bin"
   fi
-  if [ -d "$gstack_dir/browse/dist" ]; then
-    _link_or_copy "$gstack_dir/browse/dist" "$codex_gstack/browse/dist"
-  fi
-  if [ -d "$gstack_dir/browse/bin" ]; then
-    _link_or_copy "$gstack_dir/browse/bin" "$codex_gstack/browse/bin"
-  fi
+  link_browse_runtime_assets "$gstack_dir" "$codex_gstack"
   if [ -f "$agents_dir/gstack-upgrade/SKILL.md" ]; then
     _link_or_copy "$agents_dir/gstack-upgrade/SKILL.md" "$codex_gstack/gstack-upgrade/SKILL.md"
   fi