From d704f159bdccf2946ce75d66027d77bb90b84422 Mon Sep 17 00:00:00 2001 From: tim Date: Mon, 6 Apr 2026 23:08:30 +0800 Subject: [PATCH 1/3] Clarify sandbox port-bind failures in browse startup Sandboxed localhost binds can fail with EPERM or EACCES, but browse treated every bind error as either port contention or generic exhaustion. This extracts port probing into a dedicated helper, preserves explicit BROWSE_PORT handling, and surfaces permission-denied failures with actionable guidance instead of retrying into a misleading exhaustion message. Constraint: Some sandboxed environments reject 127.0.0.1 binds with EPERM/EACCES instead of EADDRINUSE Rejected: Keep the generic retry loop | misreports permission failures as port exhaustion Confidence: high Scope-risk: narrow Reversibility: clean Directive: Treat EPERM/EACCES as environment restrictions, not port contention, unless startup semantics are redesigned end-to-end Tested: bun test browse/test/findport.test.ts; bun test browse/test/; sandbox and escalated browse startup checks Not-tested: Non-sandbox environments that deny binds for reasons other than EPERM/EACCES --- browse/src/port.ts | 104 +++++++++++++++++++++++++++++++++++ browse/src/server.ts | 41 +------------- browse/test/findport.test.ts | 34 ++++++++++++ 3 files changed, 140 insertions(+), 39 deletions(-) create mode 100644 browse/src/port.ts diff --git a/browse/src/port.ts b/browse/src/port.ts new file mode 100644 index 0000000000..93293dbcd8 --- /dev/null +++ b/browse/src/port.ts @@ -0,0 +1,104 @@ +import * as net from 'net'; + +export type PortCheckResult = + | { ok: true } + | { ok: false; code?: string; message?: string }; + +type CheckPortFn = (port: number, hostname: string) => Promise; + +export interface FindPortOptions { + requestedPort?: number; + hostname?: string; + minPort?: number; + maxPort?: number; + maxRetries?: number; + randomInt?: (minPort: number, maxPort: number) => number; + checkPort?: CheckPortFn; +} + +export function checkPortAvailability(port: number, hostname: string = '127.0.0.1'): Promise { + return new Promise((resolve) => { + const srv = net.createServer(); + srv.once('error', (err: NodeJS.ErrnoException) => { + resolve({ + ok: false, + code: err.code, + message: err.message, + }); + }); + srv.listen(port, hostname, () => { + srv.close(() => resolve({ ok: true })); + }); + }); +} + +function isPermissionError(code?: string): boolean { + return code === 'EPERM' || code === 'EACCES'; +} + +function formatPermissionError(port: number, code?: string, explicit: boolean = false): Error { + const detail = code ? ` (${code})` : ''; + if (explicit) { + return new Error( + `[browse] Port ${port} (from BROWSE_PORT env) was blocked by the environment${detail}. ` + + `Re-run with escalated permissions or outside the sandbox.`, + ); + } + + return new Error( + `[browse] Local port bind was blocked by the environment${detail} while probing port ${port}. ` + + `Re-run with escalated permissions or outside the sandbox.`, + ); +} + +function formatUnexpectedBindError(port: number, message?: string, explicit: boolean = false): Error { + if (explicit) { + return new Error( + `[browse] Port ${port} (from BROWSE_PORT env) could not be bound: ${message || 'unknown error'}`, + ); + } + + return new Error( + `[browse] Unexpected error while probing port ${port}: ${message || 'unknown error'}`, + ); +} + +export async function findPort(options: FindPortOptions = {}): Promise { + const hostname = options.hostname || '127.0.0.1'; + const minPort = options.minPort ?? 10000; + const maxPort = options.maxPort ?? 60000; + const maxRetries = options.maxRetries ?? 5; + const requestedPort = options.requestedPort ?? 0; + const randomInt = options.randomInt || ((min: number, max: number) => min + Math.floor(Math.random() * (max - min))); + const checkPort = options.checkPort || checkPortAvailability; + + if (requestedPort) { + const result = await checkPort(requestedPort, hostname); + if (result.ok) { + return requestedPort; + } + if (isPermissionError(result.code)) { + throw formatPermissionError(requestedPort, result.code, true); + } + if (result.code === 'EADDRINUSE') { + throw new Error(`[browse] Port ${requestedPort} (from BROWSE_PORT env) is in use`); + } + throw formatUnexpectedBindError(requestedPort, result.message, true); + } + + for (let attempt = 0; attempt < maxRetries; attempt++) { + const port = randomInt(minPort, maxPort); + const result = await checkPort(port, hostname); + if (result.ok) { + return port; + } + if (isPermissionError(result.code)) { + throw formatPermissionError(port, result.code); + } + if (result.code !== 'EADDRINUSE') { + throw formatUnexpectedBindError(port, result.message); + } + } + + throw new Error(`[browse] No available port after ${maxRetries} attempts in range ${minPort}-${maxPort}`); +} diff --git a/browse/src/server.ts b/browse/src/server.ts index 55b744aa2c..af7e00594c 100644 --- a/browse/src/server.ts +++ b/browse/src/server.ts @@ -24,10 +24,10 @@ import { handleSnapshot, SNAPSHOT_FLAGS } from './snapshot'; import { resolveConfig, ensureStateDir, readVersionHash } from './config'; import { emitActivity, subscribe, getActivityAfter, getActivityHistory, getSubscriberCount } from './activity'; import { inspectElement, modifyStyle, resetModifications, getModificationHistory, detachSession, type InspectorResult } from './cdp-inspector'; +import { findPort } from './port'; // Bun.spawn used instead of child_process.spawn (compiled bun binaries // fail posix_spawn on all executables including /bin/bash) import * as fs from 'fs'; -import * as net from 'net'; import * as path from 'path'; import * as crypto from 'crypto'; @@ -710,43 +710,6 @@ function emitInspectorEvent(event: any): void { const browserManager = new BrowserManager(); let isShuttingDown = false; -// Test if a port is available by binding and immediately releasing. -// Uses net.createServer instead of Bun.serve to avoid a race condition -// in the Node.js polyfill where listen/close are async but the caller -// expects synchronous bind semantics. See: #486 -function isPortAvailable(port: number, hostname: string = '127.0.0.1'): Promise { - return new Promise((resolve) => { - const srv = net.createServer(); - srv.once('error', () => resolve(false)); - srv.listen(port, hostname, () => { - srv.close(() => resolve(true)); - }); - }); -} - -// Find port: explicit BROWSE_PORT, or random in 10000-60000 -async function findPort(): Promise { - // Explicit port override (for debugging) - if (BROWSE_PORT) { - if (await isPortAvailable(BROWSE_PORT)) { - return BROWSE_PORT; - } - throw new Error(`[browse] Port ${BROWSE_PORT} (from BROWSE_PORT env) is in use`); - } - - // Random port with retry - const MIN_PORT = 10000; - const MAX_PORT = 60000; - const MAX_RETRIES = 5; - for (let attempt = 0; attempt < MAX_RETRIES; attempt++) { - const port = MIN_PORT + Math.floor(Math.random() * (MAX_PORT - MIN_PORT)); - if (await isPortAvailable(port)) { - return port; - } - } - throw new Error(`[browse] No available port after ${MAX_RETRIES} attempts in range ${MIN_PORT}-${MAX_PORT}`); -} - /** * Translate Playwright errors into actionable messages for AI agents. */ @@ -1021,7 +984,7 @@ async function start() { if (err.code !== 'ENOENT') console.debug('[browse] Log cleanup dialog:', err.message); } - const port = await findPort(); + const port = await findPort({ requestedPort: BROWSE_PORT || undefined }); // Launch browser (headless or headed with extension) // BROWSE_HEADLESS_SKIP=1 skips browser launch entirely (for HTTP-only testing) diff --git a/browse/test/findport.test.ts b/browse/test/findport.test.ts index fb3a9cb094..a400c8e79e 100644 --- a/browse/test/findport.test.ts +++ b/browse/test/findport.test.ts @@ -1,6 +1,7 @@ import { describe, test, expect } from 'bun:test'; import * as net from 'net'; import * as path from 'path'; +import { findPort } from '../src/port'; const polyfillPath = path.resolve(import.meta.dir, '../src/bun-polyfill.cjs'); @@ -28,6 +29,39 @@ function getFreePort(): Promise { } describe('findPort / isPortAvailable', () => { + test('findPort surfaces permission-denied errors instead of reporting port exhaustion', async () => { + await expect(findPort({ + maxRetries: 5, + minPort: 12000, + maxPort: 12001, + randomInt: () => 12000, + checkPort: async () => ({ + ok: false, + code: 'EPERM', + message: 'listen EPERM: operation not permitted 127.0.0.1:12000', + }), + })).rejects.toThrow(/blocked by the environment/i); + }); + + test('findPort reports permission-denied on explicit BROWSE_PORT overrides', async () => { + await expect(findPort({ + requestedPort: 43123, + checkPort: async () => ({ + ok: false, + code: 'EACCES', + message: 'listen EACCES: permission denied 127.0.0.1:43123', + }), + })).rejects.toThrow(/43123/); + + await expect(findPort({ + requestedPort: 43123, + checkPort: async () => ({ + ok: false, + code: 'EACCES', + message: 'listen EACCES: permission denied 127.0.0.1:43123', + }), + })).rejects.toThrow(/blocked by the environment/i); + }); test('isPortAvailable returns true for a free port', async () => { // Use the same isPortAvailable logic from server.ts From fbd45600b9960047fdba4bc71059310ce10e55c1 Mon Sep 17 00:00:00 2001 From: tim Date: Tue, 7 Apr 2026 08:27:29 +0800 Subject: [PATCH 2/3] Document sandbox bind failures in browse troubleshooting Browse now reports permission-denied bind failures clearly, but users upgrading from older installs can still encounter the legacy "No available port" message and misdiagnose it as port exhaustion. Add a troubleshooting note that distinguishes sandbox restrictions from real port contention and points users at rebuild or upgrade as the fix. Constraint: README troubleshooting needs to help both fresh installs and stale installs already in the field Rejected: Leave the explanation only in runtime error text | stale installs still surface the old misleading message Confidence: high Scope-risk: narrow Reversibility: clean Directive: Keep troubleshooting aligned with actual browse startup errors when startup diagnostics change Tested: README diff reviewed against current browse error strings Not-tested: Generated skill docs or website copies of this troubleshooting text --- README.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/README.md b/README.md index 69b73024d9..3e8bd19b1d 100644 --- a/README.md +++ b/README.md @@ -319,6 +319,10 @@ Data is stored in [Supabase](https://supabase.com) (open source Firebase alterna **`/browse` fails?** `cd ~/.claude/skills/gstack && bun install && bun run build` +If startup says `Local port bind was blocked by the environment (EPERM)` or `EACCES`, that is usually a sandbox or permission restriction, not real port exhaustion. Re-run with escalated permissions or outside the sandbox. + +If you see the older message `No available port after 5 attempts`, your install is stale. Rebuild or upgrade gstack so browse reports the real bind error. + **Stale install?** Run `/gstack-upgrade` — or set `auto_upgrade: true` in `~/.gstack/config.yaml` **Want shorter commands?** `cd ~/.claude/skills/gstack && ./setup --no-prefix` — switches from `/gstack-qa` to `/qa`. Your choice is remembered for future upgrades. From b4e3c78beb942f6bb9144dec54c0160aa9947e2a Mon Sep 17 00:00:00 2001 From: tim Date: Tue, 7 Apr 2026 11:01:49 +0800 Subject: [PATCH 3/3] Lock explicit browse port selection into CLI startup The port classification fix covers server startup directly, but the user-facing failure path usually goes through the CLI auto-start flow. Add a lifecycle regression test that boots the CLI with an explicit BROWSE_PORT and verifies the spawned server keeps that requested port in its state file. Constraint: The CLI startup path must stay testable without launching Chromium, so the regression uses BROWSE_HEADLESS_SKIP and a temporary state file Rejected: String-match the source for BROWSE_PORT forwarding | would miss real wiring regressions in the subprocess startup path Confidence: high Scope-risk: narrow Reversibility: clean Directive: Keep this test focused on preserved port wiring, not incidental status output formatting Tested: bun test browse/test/commands.test.ts -t "explicit BROWSE_PORT is preserved when CLI auto-starts the server"; bun test browse/test/findport.test.ts Not-tested: Full browse/test/ suite after adding this extra CLI lifecycle coverage --- browse/test/commands.test.ts | 50 ++++++++++++++++++++++++++++++++++++ 1 file changed, 50 insertions(+) diff --git a/browse/test/commands.test.ts b/browse/test/commands.test.ts index c6b916ccfd..f1c188204d 100644 --- a/browse/test/commands.test.ts +++ b/browse/test/commands.test.ts @@ -704,6 +704,56 @@ describe('CLI server script resolution', () => { // ─── CLI lifecycle ────────────────────────────────────────────── describe('CLI lifecycle', () => { + test('explicit BROWSE_PORT is preserved when CLI auto-starts the server', async () => { + const stateFile = `/tmp/browse-test-state-port-${Date.now()}.json`; + + const requestedPort = await new Promise((resolve, reject) => { + const srv = require('net').createServer(); + srv.once('error', reject); + srv.listen(0, '127.0.0.1', () => { + const port = srv.address().port; + srv.close(() => resolve(port)); + }); + }); + + const cliPath = path.resolve(__dirname, '../src/cli.ts'); + const cliEnv: Record = {}; + for (const [k, v] of Object.entries(process.env)) { + if (v !== undefined) cliEnv[k] = v; + } + cliEnv.BROWSE_STATE_FILE = stateFile; + cliEnv.BROWSE_HEADLESS_SKIP = '1'; + cliEnv.BROWSE_PORT = String(requestedPort); + + const result = await new Promise<{ code: number; stdout: string; stderr: string }>((resolve) => { + const proc = spawn('bun', ['run', cliPath, 'status'], { + timeout: 15000, + env: cliEnv, + }); + let stdout = ''; + let stderr = ''; + proc.stdout.on('data', (d) => stdout += d.toString()); + proc.stderr.on('data', (d) => stderr += d.toString()); + proc.on('close', (code) => resolve({ code: code ?? 1, stdout, stderr })); + }); + + let restartedPid: number | null = null; + let restartedPort: number | null = null; + if (fs.existsSync(stateFile)) { + const state = JSON.parse(fs.readFileSync(stateFile, 'utf-8')); + restartedPid = state.pid ?? null; + restartedPort = state.port ?? null; + fs.unlinkSync(stateFile); + } + if (restartedPid) { + try { process.kill(restartedPid, 'SIGTERM'); } catch {} + } + + expect(result.code).toBe(0); + expect(result.stderr).toContain('Starting server'); + expect(restartedPort).toBe(requestedPort); + }, 20000); + test('dead state file triggers a clean restart', async () => { const stateFile = `/tmp/browse-test-state-${Date.now()}.json`; fs.writeFileSync(stateFile, JSON.stringify({