From eca0610e44bae8322c87ed491568576c5dc21c90 Mon Sep 17 00:00:00 2001 From: Catfish-75 Date: Sat, 23 May 2026 14:09:38 +0300 Subject: [PATCH 1/3] Fix Codex browse runtime on Windows --- browse/src/cli.ts | 82 ++++++++++++++++++++++++++++++------ browse/test/commands.test.ts | 2 +- browse/test/config.test.ts | 26 +++++++++++- setup | 40 +++++++++++++++--- test/gen-skill-docs.test.ts | 17 ++++++++ 5 files changed, 146 insertions(+), 21 deletions(-) diff --git a/browse/src/cli.ts b/browse/src/cli.ts index 4df950190e..118deed112 100644 --- a/browse/src/cli.ts +++ b/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 = process.env, @@ -229,16 +231,15 @@ async function startServer(extraEnv?: Record): Promise): Promise { 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 { // 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 { 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,15 @@ 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 (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 +522,32 @@ async function sendCommand(state: ServerState, command: string, args: string[], } } +async function writeStdout(text: string): Promise { + const output = text.endsWith('\n') ? text : `${text}\n`; + fs.writeSync(1, output); +} + +async function handleStopCommand(commandArgs: string[]): Promise { + 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 +1255,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 +1272,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 diff --git a/browse/test/commands.test.ts b/browse/test/commands.test.ts index b3870c0ccf..ebfdcdd0c6 100644 --- a/browse/test/commands.test.ts +++ b/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) diff --git a/browse/test/config.test.ts b/browse/test/config.test.ts index 8daa27c3eb..7e6b8108cc 100644 --- a/browse/test/config.test.ts +++ b/browse/test/config.test.ts @@ -315,6 +315,30 @@ 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('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('resolveGstackHome', () => { test('honors GSTACK_HOME env var when set', () => { const orig = process.env.GSTACK_HOME; @@ -367,7 +391,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; diff --git a/setup b/setup index 163865731d..d405a3e03d 100755 --- a/setup +++ b/setup @@ -668,6 +668,39 @@ 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. + for dep in playwright playwright-core diff; 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/@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 +726,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 diff --git a/test/gen-skill-docs.test.ts b/test/gen-skill-docs.test.ts index 0a0c9741ba..1d57288d5d 100644 --- a/test/gen-skill-docs.test.ts +++ b/test/gen-skill-docs.test.ts @@ -2224,6 +2224,23 @@ describe('setup script validation', () => { expect(setupContent).toContain('create_agents_sidecar "$SOURCE_GSTACK_DIR"'); }); + test('Codex runtime root includes browse source and externalized Node deps', () => { + const buildScript = fs.readFileSync(path.join(ROOT, 'browse', 'scripts', 'build-node-server.sh'), 'utf-8'); + const externals = [...buildScript.matchAll(/--external\s+"?([^"\\\s]+)"?/g)] + .map((match) => match[1]) + .filter((dep) => dep !== 'bun:sqlite'); + const fnStart = setupContent.indexOf('link_browse_runtime_assets()'); + const fnEnd = setupContent.indexOf('# ─── Helper: create a minimal ~/.codex/skills/gstack runtime root', fnStart); + const fnBody = setupContent.slice(fnStart, fnEnd); + + expect(fnBody).toContain('browse/src'); + for (const dep of externals) { + const root = dep.startsWith('@') ? dep.split('/')[0] : dep; + expect(fnBody).toContain(root); + } + expect(setupContent).toContain('link_browse_runtime_assets "$gstack_dir" "$codex_gstack"'); + }); + test('link_codex_skill_dirs reads from .agents/skills/', () => { // The Codex link function must reference .agents/skills for generated Codex skills const fnStart = setupContent.indexOf('link_codex_skill_dirs()'); From 7d48890e9b8c07e4b086efdec814ac0aa8d661f8 Mon Sep 17 00:00:00 2001 From: Catfish-75 Date: Fri, 29 May 2026 10:52:46 +0300 Subject: [PATCH 2/3] Fix browse restart and resilient goto --- browse/src/cli.ts | 9 +++++++++ browse/src/write-commands.ts | 30 ++++++++++++++++++++++++++++-- browse/test/config.test.ts | 15 +++++++++++++++ 3 files changed, 52 insertions(+), 2 deletions(-) diff --git a/browse/src/cli.ts b/browse/src/cli.ts index 118deed112..d705dc2ea6 100644 --- a/browse/src/cli.ts +++ b/browse/src/cli.ts @@ -500,6 +500,15 @@ async function sendCommand(state: ServerState, command: string, args: string[], await writeStdout('Server stopped'); return; } + if (command === 'restart' && !(await isServerHealthy(state.port))) { + const restartEnv: Record = {}; + 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 diff --git a/browse/src/write-commands.ts b/browse/src/write-commands.ts index daebd18a0b..ab43c80f4a 100644 --- a/browse/src/write-commands.ts +++ b/browse/src/write-commands.ts @@ -128,6 +128,24 @@ const CLEANUP_SELECTORS = { ], }; +async function withManualTimeout( + promise: Promise, + timeoutMs: number +): Promise<{ timedOut: false; value: T } | { timedOut: true }> { + promise.catch(() => {}); + let timeout: ReturnType | 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': { diff --git a/browse/test/config.test.ts b/browse/test/config.test.ts index 7e6b8108cc..98ac295719 100644 --- a/browse/test/config.test.ts +++ b/browse/test/config.test.ts @@ -332,6 +332,11 @@ describe('cli command dispatch', () => { 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'); @@ -339,6 +344,16 @@ describe('cli command dispatch', () => { }); }); +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; From 6324160dcc6a7c044b480d2414c9cae7c148365c Mon Sep 17 00:00:00 2001 From: Catfish-75 Date: Fri, 29 May 2026 11:04:32 +0300 Subject: [PATCH 3/3] Fix Codex screenshot sharp runtime --- browse/scripts/build-node-server.sh | 1 + bun.lock | 1 + package.json | 1 + setup | 11 ++++++++++- test/gen-skill-docs.test.ts | 9 +++++++-- 5 files changed, 20 insertions(+), 3 deletions(-) diff --git a/browse/scripts/build-node-server.sh b/browse/scripts/build-node-server.sh index 3ab652ac06..31344b24d8 100755 --- a/browse/scripts/build-node-server.sh +++ b/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" diff --git a/bun.lock b/bun.lock index 96fda00aaa..b3320016ea 100644 --- a/bun.lock +++ b/bun.lock @@ -11,6 +11,7 @@ "marked": "^18.0.2", "playwright": "^1.58.2", "puppeteer-core": "^24.40.0", + "sharp": "^0.34.5", "socks": "^2.8.8", }, "devDependencies": { diff --git a/package.json b/package.json index eb77faa516..46713d60e4 100644 --- a/package.json +++ b/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": { diff --git a/setup b/setup index d405a3e03d..bb897d9292 100755 --- a/setup +++ b/setup @@ -687,11 +687,20 @@ link_browse_runtime_assets() { fi # server-node.mjs intentionally externalizes these runtime packages. - for dep in playwright playwright-core diff; do + # 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 diff --git a/test/gen-skill-docs.test.ts b/test/gen-skill-docs.test.ts index 1d57288d5d..ef686f9094 100644 --- a/test/gen-skill-docs.test.ts +++ b/test/gen-skill-docs.test.ts @@ -2238,6 +2238,8 @@ describe('setup script validation', () => { const root = dep.startsWith('@') ? dep.split('/')[0] : dep; expect(fnBody).toContain(root); } + expect(fnBody).toContain('sharp semver detect-libc'); + expect(fnBody).toContain('node_modules/@img'); expect(setupContent).toContain('link_browse_runtime_assets "$gstack_dir" "$codex_gstack"'); }); @@ -2370,9 +2372,12 @@ describe('setup script validation', () => { const fnStart = setupContent.indexOf('create_codex_runtime_root()'); const fnEnd = setupContent.indexOf('}', setupContent.indexOf('done', setupContent.indexOf('review/', fnStart))); const fnBody = setupContent.slice(fnStart, fnEnd); + const runtimeStart = setupContent.indexOf('link_browse_runtime_assets()'); + const runtimeEnd = setupContent.indexOf('# ─── Helper: create a minimal ~/.codex/skills/gstack runtime root', runtimeStart); + const runtimeBody = setupContent.slice(runtimeStart, runtimeEnd); expect(fnBody).toContain('gstack/SKILL.md'); - expect(fnBody).toContain('browse/dist'); - expect(fnBody).toContain('browse/bin'); + expect(runtimeBody).toContain('browse/dist'); + expect(runtimeBody).toContain('browse/bin'); expect(fnBody).toContain('gstack-upgrade/SKILL.md'); // Review runtime assets (individual files, not the whole dir) expect(fnBody).toContain('checklist.md');