diff --git a/src/v2/index.ts b/src/v2/index.ts index 014656c..457dc87 100644 --- a/src/v2/index.ts +++ b/src/v2/index.ts @@ -46,10 +46,17 @@ export const Plugin: PluginV2 = define({ } 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..2ea3c60 100644 --- a/src/web/server/server.ts +++ b/src/web/server/server.ts @@ -22,6 +22,87 @@ 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 +124,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..da54a68 --- /dev/null +++ b/test/port-fallback.test.ts @@ -0,0 +1,191 @@ +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) + }) +})