From 5ffc3b5b0f697fe2e0fea6a420bc7cd0f8da9b8c Mon Sep 17 00:00:00 2001 From: Lenucksi Date: Sat, 19 Sep 2026 21:01:33 +0200 Subject: [PATCH 1/2] fix(web): fall back to a free port on EADDRINUSE and make v2 setup crash-proof The plugin crashed when its configured web-server port (default 4200) was taken: PTYServer.startWebServer called Bun.serve without EADDRINUSE handling and src/v2/index.ts setup awaited getOrCreateServer without a try/catch, so a busy port rejected createServer and took the whole plugin down. Port binding now keeps its semantics (undefined/0 -> OS-assigned ephemeral port, never a conflict) and for an explicit port retries port+1..port+N (N=10) before falling back to port 0 as a last resort, logging which ports were busy and the finally bound port. EADDRINUSE is detected robustly from Bun's error shape (own code property 'EADDRINUSE', verified at runtime, with a message-based fallback). Before binding an explicit port, a quick /health probe on 127.0.0.1 reports whether another opencode-pty instance already serves it (purely diagnostic; the bind loop stays the TOCTOU-safe authority). The foreign server is deliberately not reused: the PTY manager is in-process per plugin instance, so adopting a foreign server would show a foreign session list. v2 setup wraps autostart in try/catch and continues on failure: tools keep working in-process and the server can still come up later, because getOrCreateServer retries with the new port fallback on the next on-demand command invocation. Adds test/port-fallback.test.ts covering the pure helpers (resolveWebPort, portCandidates, isEaddrinuse), double-bind fallback with health verification, exhausted-fallback (all 11 ports blocked -> OS-assigned), port 0 semantics, and v2 setup resilience with a busy configured port. --- src/v2/index.ts | 18 +++- src/web/server/server.ts | 118 ++++++++++++++++++++++- test/port-fallback.test.ts | 193 +++++++++++++++++++++++++++++++++++++ 3 files changed, 322 insertions(+), 7 deletions(-) create mode 100644 test/port-fallback.test.ts diff --git a/src/v2/index.ts b/src/v2/index.ts index 014656c..5ea7205 100644 --- a/src/v2/index.ts +++ b/src/v2/index.ts @@ -39,17 +39,25 @@ export const Plugin: PluginV2 = define({ }) } - if (ctx.command && typeof ctx.command.transform === 'function') { +if (ctx.command && typeof ctx.command.transform === 'function') { await ctx.command.transform((draft) => { registerV2Commands(draft, ctx.options as OpencodePtyOptions | undefined) }) } if (ctx.options?.autostart) { - await getOrCreateServer({ - port: ctx.options.port, - hostname: ctx.options.hostname, - }) + try { + await getOrCreateServer({ + port: ctx.options.port, + hostname: ctx.options.hostname, + }) + } catch (error) { + // Never let web-server startup failure crash plugin setup: the PTY + // tools stay fully functional in-process, and getOrCreateServer retries + // (now with port fallback) on the next on-demand command invocation. + console.warn('[opencode-pty] web server could not be started:', error) + } + } } }, }) diff --git a/src/web/server/server.ts b/src/web/server/server.ts index 02bf4ae..2b1350e 100644 --- a/src/web/server/server.ts +++ b/src/web/server/server.ts @@ -22,6 +22,86 @@ export interface ServerOptions { hostname?: string } +export const MAX_PORT_FALLBACK_ATTEMPTS = 10 + +/** + * Resolve the requested web-server port. + * + * `undefined` or `0` mean "OS-assigned ephemeral port" and never conflict; + * any other value (or `PTY_WEB_PORT` env var) is treated as an explicit port. + */ +export function resolveWebPort(options?: ServerOptions): number { + const explicit = + options?.port ?? (process.env.PTY_WEB_PORT ? parseInt(process.env.PTY_WEB_PORT, 10) : 0) + if (explicit === undefined || explicit === 0 || Number.isNaN(explicit)) { + return 0 + } + return explicit +} + +/** + * Candidate ports to try for an explicit request: the requested port, the + * next `MAX_PORT_FALLBACK_ATTEMPTS` ports, and finally `0` (OS-assigned) as + * a last resort. Port `0` resolves to just `[0]`. + */ +export function portCandidates(port: number): number[] { + if (port === 0) { + return [0] + } + const candidates = [port] + for (let i = 1; i <= MAX_PORT_FALLBACK_ATTEMPTS && port + i <= 65535; i++) { + candidates.push(port + i) + } + candidates.push(0) + return candidates +} + +/** + * Robust EADDRINUSE detection for Bun.serve bind failures. + * Bun throws an `Error` with an own `code: 'EADDRINUSE'` property and a + * message like "Failed to start server. Is port 42789 in use?"; fall back to + * the message for environments that shape the error differently. + */ +export function isEaddrinuse(error: unknown): boolean { + if (!(error instanceof Error)) { + return false + } + const code = (error as NodeJS.ErrnoException).code + return ( + code === 'EADDRINUSE' || /EADDRINUSE|address already in use|port \d+ in use/i.test(error.message) + ) +} + +/** + * Diagnostic probe, run before binding an explicit port: if an opencode-pty + * server already listens there (identified by the /health response shape), + * log it — the bind loop below will bind the next free port. + * + * Purely informational, so it must stay robust (never throw) and the bind + * loop remains the authority (TOCTOU-safe). The foreign server is deliberately + * NOT reused: the PTY manager is in-process per plugin instance, so adopting + * a foreign server would show a foreign session list. + */ +async function probeExistingServer(port: number): Promise { + try { + const response = await fetch(`http://127.0.0.1:${port}${routes.health.path}`, { + signal: AbortSignal.timeout(500), + }) + if (!response.ok) { + return + } + const body = (await response.json()) as Record + if (body.status === 'healthy' && body.sessions !== undefined && body.websocket !== undefined) { + console.warn( + `[opencode-pty] an opencode-pty server already runs on port ${port} (likely another instance); binding the next free port.` + ) + } + } catch { + // Refused connection / timeout / non-JSON body are the expected cases for + // a free or non-opencode-pty port; keep the probe silent. + } +} + export class PTYServer implements Disposable { public readonly server: Server private readonly staticRoutes: Record @@ -43,14 +123,48 @@ export class PTYServer implements Disposable { public static async createServer(options?: ServerOptions): Promise { const staticRoutes = await buildStaticRoutes() + // Diagnostic only: probe for an existing opencode-pty server on an + // explicit configured port before binding (see probeExistingServer). + const port = resolveWebPort(options) + if (port !== 0) { + await probeExistingServer(port) + } + return new PTYServer(staticRoutes, options) } private startWebServer(): Server { - const port = - this.options?.port ?? (process.env.PTY_WEB_PORT ? parseInt(process.env.PTY_WEB_PORT, 10) : 0) + const port = resolveWebPort(this.options) const hostname = this.options?.hostname ?? process.env.PTY_WEB_HOSTNAME ?? '::1' + // No explicit port: OS-assigned ephemeral port, can never conflict. + if (port === 0) { + return this.bindServer(port, hostname) + } + + let lastError: unknown + for (const candidate of portCandidates(port)) { + try { + const server = this.bindServer(candidate, hostname) + if (candidate !== port) { + console.warn( + `[opencode-pty] configured port ${port} is in use; bound to port ${server.url.port} instead.` + ) + } + return server + } catch (error) { + lastError = error + if (!isEaddrinuse(error)) { + throw error + } + console.warn(`[opencode-pty] port ${candidate} is in use; trying the next port...`) + } + } + // Unreachable: portCandidates always ends with 0, which cannot conflict. + throw lastError + } + + private bindServer(port: number, hostname: string): Server { return Bun.serve({ port, hostname, diff --git a/test/port-fallback.test.ts b/test/port-fallback.test.ts new file mode 100644 index 0000000..ec4e644 --- /dev/null +++ b/test/port-fallback.test.ts @@ -0,0 +1,193 @@ +import { afterEach, describe, expect, it } from 'bun:test' +import net from 'node:net' +import { PTYServer } from '../src/web/server/server.ts' +import { + MAX_PORT_FALLBACK_ATTEMPTS, + isEaddrinuse, + portCandidates, + resolveWebPort, +} from '../src/web/server/server.ts' +import { Plugin, getActiveServer, stopActiveServer } from '../src/v2/index.ts' +import type { PluginContextV2 } from '../src/v2/types.ts' + +/** + * Learn a free port reliably: bind a short-lived listener on port 0, read the + * OS-assigned port, close the listener. The port may be re-claimed by another + * process in the gap, which is exactly the race the bind fallback handles. + */ +async function findFreePort(): Promise { + const listener = net.createServer() + await new Promise((resolve, reject) => { + listener.once('error', reject) + listener.listen(0, '127.0.0.1', resolve) + }) + const port = (listener.address() as net.AddressInfo).port + await new Promise((resolve) => { + listener.close(() => resolve()) + }) + return port +} + +describe('Port resolution helpers', () => { + describe('resolveWebPort', () => { + it('returns 0 (OS-assigned) when no port is configured', () => { + expect(resolveWebPort()).toBe(0) + expect(resolveWebPort({})).toBe(0) + expect(resolveWebPort({ port: undefined })).toBe(0) + }) + + it('returns 0 (OS-assigned) for an explicit port 0', () => { + expect(resolveWebPort({ port: 0 })).toBe(0) + }) + + it('returns 0 (OS-assigned) for NaN', () => { + expect(resolveWebPort({ port: Number.NaN })).toBe(0) + }) + + it('returns the explicit port when configured', () => { + expect(resolveWebPort({ port: 12345 })).toBe(12345) + }) + + it('falls back to the PTY_WEB_PORT env var when no option is given', () => { + const previous = process.env.PTY_WEB_PORT + process.env.PTY_WEB_PORT = '23456' + try { + expect(resolveWebPort({})).toBe(23456) + } finally { + if (previous === undefined) { + delete process.env.PTY_WEB_PORT + } else { + process.env.PTY_WEB_PORT = previous + } + } + }) + }) + + describe('portCandidates', () => { + it('yields only port 0 for port 0', () => { + expect(portCandidates(0)).toEqual([0]) + }) + + it('yields the requested port, the next fallback ports, and 0 as last resort', () => { + const candidates = portCandidates(4200) + expect(candidates[0]).toBe(4200) + expect(candidates).toHaveLength(MAX_PORT_FALLBACK_ATTEMPTS + 2) + expect(candidates[candidates.length - 1]).toBe(0) + expect(candidates).toContain(4200 + MAX_PORT_FALLBACK_ATTEMPTS) + }) + + it('caps the fallback range at port 65535', () => { + expect(portCandidates(65535)).toEqual([65535, 0]) + }) + }) + + describe('isEaddrinuse', () => { + it('detects the Bun.serve error shape (own code property)', () => { + const error = new Error('Failed to start server. Is port 42789 in use?') + ;(error as NodeJS.ErrnoException).code = 'EADDRINUSE' + expect(isEaddrinuse(error)).toBe(true) + }) + + it('detects EADDRINUSE from the message alone', () => { + expect(isEaddrinuse(new Error('Failed to start server. Is port 42789 in use?'))).toBe(true) + expect(isEaddrinuse(new Error('listen EADDRINUSE: address already in use 127.0.0.1:4200'))).toBe( + true + ) + }) + + it('rejects unrelated errors and non-errors', () => { + expect(isEaddrinuse(new Error('something else broke'))).toBe(false) + expect(isEaddrinuse('EADDRINUSE')).toBe(false) + expect(isEaddrinuse(null)).toBe(false) + expect(isEaddrinuse(undefined)).toBe(false) + }) + }) +}) + +describe('Port fallback binding', () => { + it('falls back to the next free port when the explicit port is taken', async () => { + const port = await findFreePort() + + await using first = await PTYServer.createServer({ port, hostname: '127.0.0.1' }) + expect(Number(first.server.url.port)).toBe(port) + + // Same explicit port must resolve (not reject) and bind elsewhere. + await using second = await PTYServer.createServer({ port, hostname: '127.0.0.1' }) + const secondPort = Number(second.server.url.port) + expect(secondPort).not.toBe(port) + + // The second server must be fully functional on its dynamically bound port. + const health = await fetch(`http://127.0.0.1:${secondPort}/health`) + expect(health.status).toBe(200) + const body = (await health.json()) as Record + expect(body.status).toBe('healthy') + }) + + it('binds an OS-assigned port when the explicit port and every fallback are taken', async () => { + const port = await findFreePort() + const blockers: Array> = [] + for (let p = port; p <= port + MAX_PORT_FALLBACK_ATTEMPTS; p++) { + blockers.push( + Bun.serve({ port: p, hostname: '127.0.0.1', fetch: () => new Response('blocked') }) + ) + } + const blocked = new Set(blockers.map((server) => server.port)) + try { + await using server = await PTYServer.createServer({ port, hostname: '127.0.0.1' }) + const bound = Number(server.server.url.port) + expect(blocked.has(bound)).toBe(false) + + const health = await fetch(`http://127.0.0.1:${bound}/health`) + expect(health.status).toBe(200) + } finally { + for (const blocker of blockers) { + blocker.stop(true) + } + } + }) + + it('still binds a random OS-assigned port for an explicit port 0', async () => { + await using server = await PTYServer.createServer({ port: 0, hostname: '127.0.0.1' }) + expect(Number(server.server.url.port)).not.toBe(0) + }) +}) + +describe('V2 plugin setup resilience', () => { + afterEach(() => { + stopActiveServer() + }) + + it('does not throw when autostart is enabled and the configured port is taken', async () => { + const port = await findFreePort() + const blocker = Bun.serve({ port, hostname: '127.0.0.1', fetch: () => new Response('busy') }) + + const warnings: unknown[] = [] + const originalWarn = console.warn + console.warn = (...args: unknown[]) => { + warnings.push(args) + } + try { + const ctx: PluginContextV2 = { + options: { autostart: true, port, hostname: '127.0.0.1' }, + } + + // Must resolve — setup must not throw on a busy configured port. + const result = await Plugin.setup(ctx) + expect(result).toBeUndefined() + } finally { + console.warn = originalWarn + blocker.stop(true) + } + + // The server came up on a fallback port and the warning was logged. + const active = getActiveServer() + expect(active).not.toBeNull() + expect(Number(active?.server.url.port)).not.toBe(port) + expect(warnings.some((args) => String(args).includes('in use'))).toBe(true) + + const health = await fetch( + `http://127.0.0.1:${Number(active?.server.url.port)}/health` + ) + expect(health.status).toBe(200) + }) +}) \ No newline at end of file From fc63c441ff5ed0c19b4a9ddc05ccc1a60458bf65 Mon Sep 17 00:00:00 2001 From: Lenucksi Date: Sat, 19 Sep 2026 21:01:38 +0200 Subject: [PATCH 2/2] style: apply biome formatting to port fallback changes --- src/v2/index.ts | 3 +-- src/web/server/server.ts | 3 ++- test/port-fallback.test.ts | 12 +++++------- 3 files changed, 8 insertions(+), 10 deletions(-) diff --git a/src/v2/index.ts b/src/v2/index.ts index 5ea7205..457dc87 100644 --- a/src/v2/index.ts +++ b/src/v2/index.ts @@ -39,7 +39,7 @@ export const Plugin: PluginV2 = define({ }) } -if (ctx.command && typeof ctx.command.transform === 'function') { + if (ctx.command && typeof ctx.command.transform === 'function') { await ctx.command.transform((draft) => { registerV2Commands(draft, ctx.options as OpencodePtyOptions | undefined) }) @@ -58,7 +58,6 @@ if (ctx.command && typeof ctx.command.transform === 'function') { console.warn('[opencode-pty] web server could not be started:', error) } } - } }, }) diff --git a/src/web/server/server.ts b/src/web/server/server.ts index 2b1350e..2ea3c60 100644 --- a/src/web/server/server.ts +++ b/src/web/server/server.ts @@ -68,7 +68,8 @@ export function isEaddrinuse(error: unknown): boolean { } const code = (error as NodeJS.ErrnoException).code return ( - code === 'EADDRINUSE' || /EADDRINUSE|address already in use|port \d+ in use/i.test(error.message) + code === 'EADDRINUSE' || + /EADDRINUSE|address already in use|port \d+ in use/i.test(error.message) ) } diff --git a/test/port-fallback.test.ts b/test/port-fallback.test.ts index ec4e644..da54a68 100644 --- a/test/port-fallback.test.ts +++ b/test/port-fallback.test.ts @@ -90,9 +90,9 @@ describe('Port resolution helpers', () => { it('detects EADDRINUSE from the message alone', () => { expect(isEaddrinuse(new Error('Failed to start server. Is port 42789 in use?'))).toBe(true) - expect(isEaddrinuse(new Error('listen EADDRINUSE: address already in use 127.0.0.1:4200'))).toBe( - true - ) + expect( + isEaddrinuse(new Error('listen EADDRINUSE: address already in use 127.0.0.1:4200')) + ).toBe(true) }) it('rejects unrelated errors and non-errors', () => { @@ -185,9 +185,7 @@ describe('V2 plugin setup resilience', () => { expect(Number(active?.server.url.port)).not.toBe(port) expect(warnings.some((args) => String(args).includes('in use'))).toBe(true) - const health = await fetch( - `http://127.0.0.1:${Number(active?.server.url.port)}/health` - ) + const health = await fetch(`http://127.0.0.1:${Number(active?.server.url.port)}/health`) expect(health.status).toBe(200) }) -}) \ No newline at end of file +})