From 83bd7f53da45d37b7df440d6c26972ff6079324a Mon Sep 17 00:00:00 2001 From: ncnthien Date: Mon, 25 May 2026 16:16:38 +0700 Subject: [PATCH 1/8] feat: [ENG-2857] surface port-in-use error in brv webui --- src/oclif/commands/webui.ts | 67 +++++++++++-------- src/server/core/domain/errors/webui-error.ts | 16 +++++ src/server/infra/daemon/brv-server.ts | 46 ++++++++++--- src/server/infra/webui/webui-server.ts | 3 +- src/shared/transport/events/index.ts | 3 + src/shared/transport/events/webui-events.ts | 15 +++++ .../core/domain/errors/webui-error.test.ts | 16 +++++ test/unit/infra/webui/webui-server.test.ts | 6 +- 8 files changed, 132 insertions(+), 40 deletions(-) create mode 100644 src/server/core/domain/errors/webui-error.ts create mode 100644 src/shared/transport/events/webui-events.ts create mode 100644 test/unit/core/domain/errors/webui-error.test.ts diff --git a/src/oclif/commands/webui.ts b/src/oclif/commands/webui.ts index 1beb27df7..1d91f7224 100644 --- a/src/oclif/commands/webui.ts +++ b/src/oclif/commands/webui.ts @@ -1,6 +1,11 @@ import {Command, Flags} from '@oclif/core' import open from 'open' +import { + WebuiEvents, + type WebuiGetPortResponse, + type WebuiSetPortResponse, +} from '../../shared/transport/events/webui-events.js' import {formatConnectionError, withDaemonRetry} from '../lib/daemon-client.js' export default class Webui extends Command { @@ -16,33 +21,8 @@ export default class Webui extends Command { public async run(): Promise { const {flags} = await this.parse(Webui) - let webuiPort: number - - try { - // If --port is provided, tell the daemon to switch to that port and persist it - if (flags.port) { - const result = await withDaemonRetry( - async (client) => - client.requestWithAck<{port: number; success: boolean}>('webui:setPort', {port: flags.port}), - {projectPath: process.cwd()}, - ) - webuiPort = result.port - } else { - const result = await withDaemonRetry( - async (client) => client.requestWithAck<{port?: number}>('webui:getPort'), - {projectPath: process.cwd()}, - ) - - if (!result.port) { - this.error('Failed to get web UI port. Use `brv restart` to restart the daemon and try again') - } - - webuiPort = result.port - } - } catch (error) { - this.error(formatConnectionError(error)) - } - + const result = flags.port ? await this.requestSetPort(flags.port) : await this.requestGetPort() + const webuiPort = this.resolvePortOrExit(result) const url = `http://localhost:${webuiPort}` this.log(`ByteRover Web UI: ${url}`) @@ -50,4 +30,37 @@ export default class Webui extends Command { this.log('Could not open browser automatically. Open the URL above manually.') }) } + + private async requestGetPort(): Promise { + try { + return await withDaemonRetry( + async (client) => client.requestWithAck(WebuiEvents.GET_PORT), + {projectPath: process.cwd()}, + ) + } catch (error) { + return this.error(formatConnectionError(error)) + } + } + + private async requestSetPort(port: number): Promise { + try { + return await withDaemonRetry( + async (client) => client.requestWithAck(WebuiEvents.SET_PORT, {port}), + {projectPath: process.cwd()}, + ) + } catch (error) { + return this.error(formatConnectionError(error)) + } + } + + private resolvePortOrExit(result: WebuiGetPortResponse | WebuiSetPortResponse): number { + if ('port' in result) return result.port + if (result.reason === 'port_in_use') { + this.error( + `Web UI port ${result.conflictPort} is already in use. Run \`brv webui --port \` to choose a different port.`, + ) + } + + this.error('Web UI did not start. Run `brv restart` and try again.') + } } diff --git a/src/server/core/domain/errors/webui-error.ts b/src/server/core/domain/errors/webui-error.ts new file mode 100644 index 000000000..07832347d --- /dev/null +++ b/src/server/core/domain/errors/webui-error.ts @@ -0,0 +1,16 @@ +export class WebUiError extends Error { + public constructor(message: string) { + super(message) + this.name = 'WebUiError' + } +} + +export class WebUiPortInUseError extends WebUiError { + public readonly port: number + + public constructor(port: number) { + super(`Web UI port ${port} is already in use`) + this.name = 'WebUiPortInUseError' + this.port = port + } +} diff --git a/src/server/infra/daemon/brv-server.ts b/src/server/infra/daemon/brv-server.ts index 97a4f980f..555337cd7 100644 --- a/src/server/infra/daemon/brv-server.ts +++ b/src/server/infra/daemon/brv-server.ts @@ -34,6 +34,12 @@ import type {BrvConfig} from '../../core/domain/entities/brv-config.js' import {ReviewEvents} from '../../../shared/transport/events/review-events.js' import {TaskEvents, type TaskHeartbeatEvent} from '../../../shared/transport/events/task-events.js' +import { + WebuiEvents, + type WebuiGetPortResponse, + type WebuiSetPortRequest, + type WebuiSetPortResponse, +} from '../../../shared/transport/events/webui-events.js' import { AGENT_IDLE_CHECK_INTERVAL_MS, AGENT_IDLE_TIMEOUT_MS, @@ -42,6 +48,7 @@ import { TASK_HEARTBEAT_INTERVAL_MS, WEBUI_DEFAULT_PORT, } from '../../constants.js' +import {WebUiPortInUseError} from '../../core/domain/errors/webui-error.js' import { type ProviderConfigResponse, type TaskQueryResultEvent, @@ -198,6 +205,7 @@ async function main(): Promise { let authStateStore: AuthStateStore | undefined let agentPool: AgentPool | undefined let webuiServer: undefined | WebUiServer + let webuiBootFailure: undefined | {conflictPort: number; reason: 'port_in_use'} try { // 4a. Construct transport server. start() is deferred to step 11 so all handlers register before sockets connect. @@ -227,10 +235,13 @@ async function main(): Promise { writeWebuiState(webuiPort) log(`Web UI server started on port ${webuiPort}`) } catch (webuiError) { - log( - `Web UI port ${webuiPort} is already in use. Web UI will not be available. Set BRV_WEBUI_PORT= to use a different port.`, - ) - log(`Web UI start error: ${webuiError instanceof Error ? webuiError.message : String(webuiError)}`) + if (webuiError instanceof WebUiPortInUseError) { + webuiBootFailure = {conflictPort: webuiError.port, reason: 'port_in_use'} + log(`Web UI port ${webuiError.port} is already in use — Web UI unavailable`) + } else { + log(`Web UI start error: ${webuiError instanceof Error ? webuiError.message : String(webuiError)}`) + } + webuiServer = undefined } @@ -621,12 +632,14 @@ async function main(): Promise { }) // Web UI port endpoint — used by `brv webui` to discover the stable port - transportServer.onRequest('webui:getPort', () => ({ - port: webuiServer?.getPort(), - })) + transportServer.onRequest(WebuiEvents.GET_PORT, () => { + const port = webuiServer?.getPort() + if (port !== undefined) return {port} + return webuiBootFailure ?? {reason: 'not_started'} + }) // Web UI set port — restarts webui server on new port and persists preference - transportServer.onRequest<{port: number}, {port: number; success: boolean}>('webui:setPort', async (data) => { + transportServer.onRequest(WebuiEvents.SET_PORT, async (data) => { const newPort = data.port // Stop existing webui server if running @@ -645,12 +658,25 @@ async function main(): Promise { // Start on new port webuiServer = new WebUiServer(newApp) - await webuiServer.start(newPort) + try { + await webuiServer.start(newPort) + } catch (error) { + webuiServer = undefined + if (error instanceof WebUiPortInUseError) { + webuiBootFailure = {conflictPort: error.port, reason: 'port_in_use'} + log(`Web UI port ${error.port} is already in use — keeping previous configuration`) + return {conflictPort: error.port, reason: 'port_in_use'} + } + + throw error + } + + webuiBootFailure = undefined writeWebuiState(newPort) writeWebuiPreferredPort(newPort) log(`Web UI server restarted on port ${newPort} (persisted)`) - return {port: newPort, success: true} + return {port: newPort} }) // Debug endpoint — exposes daemon internal state for `brv debug` command diff --git a/src/server/infra/webui/webui-server.ts b/src/server/infra/webui/webui-server.ts index ee793e8da..2c24cfae8 100644 --- a/src/server/infra/webui/webui-server.ts +++ b/src/server/infra/webui/webui-server.ts @@ -3,6 +3,7 @@ import type {Express} from 'express' import {createServer, type Server as HttpServer} from 'node:http' import {TRANSPORT_HOST} from '../../constants.js' +import {WebUiPortInUseError} from '../../core/domain/errors/webui-error.js' /** * Standalone HTTP server for the web UI. @@ -36,7 +37,7 @@ export class WebUiServer { this.httpServer.on('error', (err: NodeJS.ErrnoException) => { if (err.code === 'EADDRINUSE') { - reject(new Error(`Web UI port ${port} is already in use`)) + reject(new WebUiPortInUseError(port)) } else { reject(err) } diff --git a/src/shared/transport/events/index.ts b/src/shared/transport/events/index.ts index 16c2798a9..bc07f17d5 100644 --- a/src/shared/transport/events/index.ts +++ b/src/shared/transport/events/index.ts @@ -28,6 +28,7 @@ export * from './status-events.js' export * from './task-events.js' export * from './team-events.js' export * from './vc-events.js' +export * from './webui-events.js' export * from './worktree-events.js' // Utility exports @@ -57,6 +58,7 @@ import {StatusEvents} from './status-events.js' import {TaskEvents} from './task-events.js' import {TeamEvents} from './team-events.js' import {VcEvents} from './vc-events.js' +import {WebuiEvents} from './webui-events.js' import {WorktreeEvents} from './worktree-events.js' /** @@ -90,6 +92,7 @@ export const AllEventGroups = [ TaskEvents, TeamEvents, VcEvents, + WebuiEvents, WorktreeEvents, ] as const diff --git a/src/shared/transport/events/webui-events.ts b/src/shared/transport/events/webui-events.ts new file mode 100644 index 000000000..3a07ac73f --- /dev/null +++ b/src/shared/transport/events/webui-events.ts @@ -0,0 +1,15 @@ +export const WebuiEvents = { + GET_PORT: 'webui:getPort', + SET_PORT: 'webui:setPort', +} as const + +export type WebuiGetPortResponse = + | {conflictPort: number; reason: 'port_in_use'} + | {port: number} + | {reason: 'not_started'} + +export interface WebuiSetPortRequest { + port: number +} + +export type WebuiSetPortResponse = {conflictPort: number; reason: 'port_in_use'} | {port: number} diff --git a/test/unit/core/domain/errors/webui-error.test.ts b/test/unit/core/domain/errors/webui-error.test.ts new file mode 100644 index 000000000..b91782c63 --- /dev/null +++ b/test/unit/core/domain/errors/webui-error.test.ts @@ -0,0 +1,16 @@ +import {expect} from 'chai' + +import {WebUiError, WebUiPortInUseError} from '../../../../../src/server/core/domain/errors/webui-error.js' + +describe('WebUiPortInUseError', () => { + it('exposes the conflicting port and a descriptive message', () => { + const error = new WebUiPortInUseError(7700) + + expect(error).to.be.an.instanceOf(WebUiError) + expect(error).to.be.an.instanceOf(Error) + expect(error.name).to.equal('WebUiPortInUseError') + expect(error.port).to.equal(7700) + expect(error.message).to.include('7700') + expect(error.message).to.include('in use') + }) +}) diff --git a/test/unit/infra/webui/webui-server.test.ts b/test/unit/infra/webui/webui-server.test.ts index 0df3d3aa1..030759783 100644 --- a/test/unit/infra/webui/webui-server.test.ts +++ b/test/unit/infra/webui/webui-server.test.ts @@ -2,6 +2,7 @@ import {expect} from 'chai' import express from 'express' import {createServer} from 'node:http' +import {WebUiPortInUseError} from '../../../../src/server/core/domain/errors/webui-error.js' import {WebUiServer} from '../../../../src/server/infra/webui/webui-server.js' describe('WebUiServer', () => { @@ -34,7 +35,7 @@ describe('WebUiServer', () => { expect(server.getPort()).to.be.undefined }) - it('should reject with error when port is in use', async () => { + it('should reject with WebUiPortInUseError when port is in use', async () => { // Occupy a port first const blockingServer = createServer() const occupiedPort = await new Promise((resolve, reject) => { @@ -53,7 +54,8 @@ describe('WebUiServer', () => { await server.start(occupiedPort) expect.fail('Expected start to reject') } catch (error) { - expect(error).to.be.an.instanceOf(Error) + expect(error).to.be.an.instanceOf(WebUiPortInUseError) + expect((error as WebUiPortInUseError).port).to.equal(occupiedPort) expect((error as Error).message).to.include('in use') } From 7045f287149cadd292754ebdabbe5297cc1ce751 Mon Sep 17 00:00:00 2001 From: ncnthien Date: Mon, 25 May 2026 16:40:53 +0700 Subject: [PATCH 2/8] =?UTF-8?q?feat:=20[ENG-2857]=20address=20PR=20feedbac?= =?UTF-8?q?k=20=E2=80=94=20explicit=20status=20discriminator,=20typed=20al?= =?UTF-8?q?ready-running=20error,=20stale-state=20fix?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- src/oclif/commands/webui.ts | 8 ++-- src/server/core/domain/errors/webui-error.ts | 7 ++++ src/server/infra/daemon/brv-server.ts | 17 +++++---- src/server/infra/webui/webui-server.ts | 5 ++- src/shared/transport/events/webui-events.ts | 8 ++-- .../core/domain/errors/webui-error.test.ts | 37 ++++++++++++++----- test/unit/infra/webui/webui-server.test.ts | 6 +-- 7 files changed, 57 insertions(+), 31 deletions(-) diff --git a/src/oclif/commands/webui.ts b/src/oclif/commands/webui.ts index 1d91f7224..0591bf8af 100644 --- a/src/oclif/commands/webui.ts +++ b/src/oclif/commands/webui.ts @@ -54,13 +54,13 @@ export default class Webui extends Command { } private resolvePortOrExit(result: WebuiGetPortResponse | WebuiSetPortResponse): number { - if ('port' in result) return result.port - if (result.reason === 'port_in_use') { - this.error( + if (result.status === 'ok') return result.port + if (result.status === 'port_in_use') { + return this.error( `Web UI port ${result.conflictPort} is already in use. Run \`brv webui --port \` to choose a different port.`, ) } - this.error('Web UI did not start. Run `brv restart` and try again.') + return this.error('Web UI did not start. Run `brv restart` and try again.') } } diff --git a/src/server/core/domain/errors/webui-error.ts b/src/server/core/domain/errors/webui-error.ts index 07832347d..6ed2c5560 100644 --- a/src/server/core/domain/errors/webui-error.ts +++ b/src/server/core/domain/errors/webui-error.ts @@ -14,3 +14,10 @@ export class WebUiPortInUseError extends WebUiError { this.port = port } } + +export class WebUiServerAlreadyRunningError extends WebUiError { + public constructor() { + super('Web UI server is already running') + this.name = 'WebUiServerAlreadyRunningError' + } +} diff --git a/src/server/infra/daemon/brv-server.ts b/src/server/infra/daemon/brv-server.ts index 555337cd7..e5c22639e 100644 --- a/src/server/infra/daemon/brv-server.ts +++ b/src/server/infra/daemon/brv-server.ts @@ -205,7 +205,7 @@ async function main(): Promise { let authStateStore: AuthStateStore | undefined let agentPool: AgentPool | undefined let webuiServer: undefined | WebUiServer - let webuiBootFailure: undefined | {conflictPort: number; reason: 'port_in_use'} + let webuiBootFailure: undefined | {conflictPort: number; status: 'port_in_use'} try { // 4a. Construct transport server. start() is deferred to step 11 so all handlers register before sockets connect. @@ -236,7 +236,7 @@ async function main(): Promise { log(`Web UI server started on port ${webuiPort}`) } catch (webuiError) { if (webuiError instanceof WebUiPortInUseError) { - webuiBootFailure = {conflictPort: webuiError.port, reason: 'port_in_use'} + webuiBootFailure = {conflictPort: webuiError.port, status: 'port_in_use'} log(`Web UI port ${webuiError.port} is already in use — Web UI unavailable`) } else { log(`Web UI start error: ${webuiError instanceof Error ? webuiError.message : String(webuiError)}`) @@ -634,8 +634,8 @@ async function main(): Promise { // Web UI port endpoint — used by `brv webui` to discover the stable port transportServer.onRequest(WebuiEvents.GET_PORT, () => { const port = webuiServer?.getPort() - if (port !== undefined) return {port} - return webuiBootFailure ?? {reason: 'not_started'} + if (port !== undefined) return {port, status: 'ok'} + return webuiBootFailure ?? {status: 'not_started'} }) // Web UI set port — restarts webui server on new port and persists preference @@ -663,11 +663,12 @@ async function main(): Promise { } catch (error) { webuiServer = undefined if (error instanceof WebUiPortInUseError) { - webuiBootFailure = {conflictPort: error.port, reason: 'port_in_use'} - log(`Web UI port ${error.port} is already in use — keeping previous configuration`) - return {conflictPort: error.port, reason: 'port_in_use'} + webuiBootFailure = {conflictPort: error.port, status: 'port_in_use'} + log(`Web UI port ${error.port} is already in use — Web UI unavailable`) + return {conflictPort: error.port, status: 'port_in_use'} } + webuiBootFailure = undefined throw error } @@ -676,7 +677,7 @@ async function main(): Promise { writeWebuiPreferredPort(newPort) log(`Web UI server restarted on port ${newPort} (persisted)`) - return {port: newPort} + return {port: newPort, status: 'ok'} }) // Debug endpoint — exposes daemon internal state for `brv debug` command diff --git a/src/server/infra/webui/webui-server.ts b/src/server/infra/webui/webui-server.ts index 2c24cfae8..50b664e4a 100644 --- a/src/server/infra/webui/webui-server.ts +++ b/src/server/infra/webui/webui-server.ts @@ -3,7 +3,7 @@ import type {Express} from 'express' import {createServer, type Server as HttpServer} from 'node:http' import {TRANSPORT_HOST} from '../../constants.js' -import {WebUiPortInUseError} from '../../core/domain/errors/webui-error.js' +import {WebUiPortInUseError, WebUiServerAlreadyRunningError} from '../../core/domain/errors/webui-error.js' /** * Standalone HTTP server for the web UI. @@ -29,13 +29,14 @@ export class WebUiServer { async start(port: number): Promise { if (this.running) { - throw new Error('Web UI server is already running') + throw new WebUiServerAlreadyRunningError() } return new Promise((resolve, reject) => { this.httpServer = createServer(this.app) this.httpServer.on('error', (err: NodeJS.ErrnoException) => { + this.httpServer = undefined if (err.code === 'EADDRINUSE') { reject(new WebUiPortInUseError(port)) } else { diff --git a/src/shared/transport/events/webui-events.ts b/src/shared/transport/events/webui-events.ts index 3a07ac73f..1e059f4d9 100644 --- a/src/shared/transport/events/webui-events.ts +++ b/src/shared/transport/events/webui-events.ts @@ -4,12 +4,12 @@ export const WebuiEvents = { } as const export type WebuiGetPortResponse = - | {conflictPort: number; reason: 'port_in_use'} - | {port: number} - | {reason: 'not_started'} + | {conflictPort: number; status: 'port_in_use'} + | {port: number; status: 'ok'} + | {status: 'not_started'} export interface WebuiSetPortRequest { port: number } -export type WebuiSetPortResponse = {conflictPort: number; reason: 'port_in_use'} | {port: number} +export type WebuiSetPortResponse = {conflictPort: number; status: 'port_in_use'} | {port: number; status: 'ok'} diff --git a/test/unit/core/domain/errors/webui-error.test.ts b/test/unit/core/domain/errors/webui-error.test.ts index b91782c63..d91dd054e 100644 --- a/test/unit/core/domain/errors/webui-error.test.ts +++ b/test/unit/core/domain/errors/webui-error.test.ts @@ -1,16 +1,33 @@ import {expect} from 'chai' -import {WebUiError, WebUiPortInUseError} from '../../../../../src/server/core/domain/errors/webui-error.js' +import { + WebUiError, + WebUiPortInUseError, + WebUiServerAlreadyRunningError, +} from '../../../../../src/server/core/domain/errors/webui-error.js' -describe('WebUiPortInUseError', () => { - it('exposes the conflicting port and a descriptive message', () => { - const error = new WebUiPortInUseError(7700) +describe('webui-error', () => { + describe('WebUiPortInUseError', () => { + it('exposes the conflicting port and a descriptive message', () => { + const error = new WebUiPortInUseError(7700) - expect(error).to.be.an.instanceOf(WebUiError) - expect(error).to.be.an.instanceOf(Error) - expect(error.name).to.equal('WebUiPortInUseError') - expect(error.port).to.equal(7700) - expect(error.message).to.include('7700') - expect(error.message).to.include('in use') + expect(error).to.be.an.instanceOf(WebUiError) + expect(error).to.be.an.instanceOf(Error) + expect(error.name).to.equal('WebUiPortInUseError') + expect(error.port).to.equal(7700) + expect(error.message).to.include('7700') + expect(error.message).to.include('in use') + }) + }) + + describe('WebUiServerAlreadyRunningError', () => { + it('identifies itself by name and extends WebUiError', () => { + const error = new WebUiServerAlreadyRunningError() + + expect(error).to.be.an.instanceOf(WebUiError) + expect(error).to.be.an.instanceOf(Error) + expect(error.name).to.equal('WebUiServerAlreadyRunningError') + expect(error.message).to.include('already running') + }) }) }) diff --git a/test/unit/infra/webui/webui-server.test.ts b/test/unit/infra/webui/webui-server.test.ts index 030759783..8c601cca0 100644 --- a/test/unit/infra/webui/webui-server.test.ts +++ b/test/unit/infra/webui/webui-server.test.ts @@ -2,7 +2,7 @@ import {expect} from 'chai' import express from 'express' import {createServer} from 'node:http' -import {WebUiPortInUseError} from '../../../../src/server/core/domain/errors/webui-error.js' +import {WebUiPortInUseError, WebUiServerAlreadyRunningError} from '../../../../src/server/core/domain/errors/webui-error.js' import {WebUiServer} from '../../../../src/server/infra/webui/webui-server.js' describe('WebUiServer', () => { @@ -60,6 +60,7 @@ describe('WebUiServer', () => { } expect(server.isRunning()).to.be.false + expect(server.getPort()).to.be.undefined } finally { blockingServer.close() } @@ -72,8 +73,7 @@ describe('WebUiServer', () => { await server.start(0) expect.fail('Expected start to reject') } catch (error) { - expect(error).to.be.an.instanceOf(Error) - expect((error as Error).message).to.include('already running') + expect(error).to.be.an.instanceOf(WebUiServerAlreadyRunningError) } }) From 3b3896de56b8d7663d8f827ca7a105e438d5a255 Mon Sep 17 00:00:00 2001 From: ncnthien Date: Mon, 25 May 2026 16:57:12 +0700 Subject: [PATCH 3/8] feat: [ENG-2857] gate httpServer cleanup to startup phase to avoid wedging stop() --- src/server/infra/webui/webui-server.ts | 9 ++++----- test/unit/infra/webui/webui-server.test.ts | 17 +++++++++++++++++ 2 files changed, 21 insertions(+), 5 deletions(-) diff --git a/src/server/infra/webui/webui-server.ts b/src/server/infra/webui/webui-server.ts index 50b664e4a..3175d15d1 100644 --- a/src/server/infra/webui/webui-server.ts +++ b/src/server/infra/webui/webui-server.ts @@ -34,17 +34,16 @@ export class WebUiServer { return new Promise((resolve, reject) => { this.httpServer = createServer(this.app) + let resolved = false this.httpServer.on('error', (err: NodeJS.ErrnoException) => { + if (resolved) return this.httpServer = undefined - if (err.code === 'EADDRINUSE') { - reject(new WebUiPortInUseError(port)) - } else { - reject(err) - } + reject(err.code === 'EADDRINUSE' ? new WebUiPortInUseError(port) : err) }) this.httpServer.listen(port, TRANSPORT_HOST, () => { + resolved = true const addr = this.httpServer?.address() this.port = typeof addr === 'object' && addr !== null ? addr.port : port this.running = true diff --git a/test/unit/infra/webui/webui-server.test.ts b/test/unit/infra/webui/webui-server.test.ts index 8c601cca0..80ff24822 100644 --- a/test/unit/infra/webui/webui-server.test.ts +++ b/test/unit/infra/webui/webui-server.test.ts @@ -81,4 +81,21 @@ describe('WebUiServer', () => { server = new WebUiServer(express()) await server.stop() // should not throw }) + + it('should ignore post-startup error emissions so stop() still cleans up', async () => { + server = new WebUiServer(express()) + await server.start(0) + const boundPort = server.getPort() + expect(boundPort).to.be.a('number') + + const internal = server as unknown as {httpServer?: {emit(event: string, err: Error): boolean}} + internal.httpServer?.emit('error', Object.assign(new Error('ECONNRESET'), {code: 'ECONNRESET'})) + + expect(server.isRunning()).to.be.true + expect(server.getPort()).to.equal(boundPort) + + await server.stop() + expect(server.isRunning()).to.be.false + expect(server.getPort()).to.be.undefined + }) }) From 6d3fa5da8a382a16bc911a9464393ca5b44d42e5 Mon Sep 17 00:00:00 2001 From: ncnthien Date: Mon, 1 Jun 2026 15:00:45 +0700 Subject: [PATCH 4/8] feat: [ENG-2857] auto-fallback Web UI to next port when default is in use --- src/oclif/commands/webui.ts | 9 ++- src/server/constants.ts | 1 + src/server/infra/daemon/brv-server.ts | 69 ++++++++++++++++----- src/shared/transport/events/webui-events.ts | 2 +- 4 files changed, 65 insertions(+), 16 deletions(-) diff --git a/src/oclif/commands/webui.ts b/src/oclif/commands/webui.ts index 0591bf8af..4bd06a4cc 100644 --- a/src/oclif/commands/webui.ts +++ b/src/oclif/commands/webui.ts @@ -54,7 +54,14 @@ export default class Webui extends Command { } private resolvePortOrExit(result: WebuiGetPortResponse | WebuiSetPortResponse): number { - if (result.status === 'ok') return result.port + if (result.status === 'ok') { + if ('requestedPort' in result && result.requestedPort !== undefined && result.requestedPort !== result.port) { + this.log(`Port ${result.requestedPort} was in use — using port ${result.port} instead.`) + } + + return result.port + } + if (result.status === 'port_in_use') { return this.error( `Web UI port ${result.conflictPort} is already in use. Run \`brv webui --port \` to choose a different port.`, diff --git a/src/server/constants.ts b/src/server/constants.ts index 1e091be8e..be4cd5081 100644 --- a/src/server/constants.ts +++ b/src/server/constants.ts @@ -61,6 +61,7 @@ export const PORT_BATCH_SIZE = 20 export const PORT_MAX_ATTEMPTS = 5 // Web UI (stable port, separate from dynamic transport port) export const WEBUI_DEFAULT_PORT = 7700 +export const WEBUI_MAX_FALLBACK_ATTEMPTS = 10 export const WEBUI_STATE_FILE = 'webui.json' // Heartbeat export const HEARTBEAT_FILE = 'heartbeat' diff --git a/src/server/infra/daemon/brv-server.ts b/src/server/infra/daemon/brv-server.ts index 780989b68..15bb332ef 100644 --- a/src/server/infra/daemon/brv-server.ts +++ b/src/server/infra/daemon/brv-server.ts @@ -46,6 +46,7 @@ import { HEARTBEAT_FILE, TASK_HEARTBEAT_INTERVAL_MS, WEBUI_DEFAULT_PORT, + WEBUI_MAX_FALLBACK_ATTEMPTS, } from '../../constants.js' import {WebUiPortInUseError} from '../../core/domain/errors/webui-error.js' import { @@ -150,6 +151,7 @@ function cleanupOldLogs(logsDir: string, keep: number): void { } } +// eslint-disable-next-line complexity async function main(): Promise { // 1. Setup daemon logging at /logs/server-.log const daemonLogsDir = join(getGlobalDataDir(), 'logs') @@ -204,6 +206,7 @@ async function main(): Promise { let agentPool: AgentPool | undefined let webuiServer: undefined | WebUiServer let webuiBootFailure: undefined | {conflictPort: number; status: 'port_in_use'} + let webuiRequestedPort: number | undefined try { // 4a. Construct transport server. start() is deferred to step 11 so all handlers register before sockets connect. @@ -215,12 +218,17 @@ async function main(): Promise { const webuiDistDir = join(projectRoot, 'dist', 'webui') // Port priority: env var > persisted preference > default const webuiPortEnv = process.env.BRV_WEBUI_PORT - const webuiPort = webuiPortEnv + const webuiPreferredPort = webuiPortEnv ? Number.parseInt(webuiPortEnv, 10) : (readWebuiPreferredPort() ?? WEBUI_DEFAULT_PORT) const webuiApp = createWebUiMiddleware({ - getConfig: () => ({daemonPort: port, port: webuiPort, projectCwd: process.cwd(), version}), + getConfig: () => ({ + daemonPort: port, + port: webuiServer?.getPort() ?? webuiPreferredPort, + projectCwd: process.cwd(), + version, + }), webuiDistDir, }) @@ -228,19 +236,46 @@ async function main(): Promise { app.use(webuiApp) webuiServer = new WebUiServer(app) - try { - await webuiServer.start(webuiPort) - writeWebuiState(webuiPort) - log(`Web UI server started on port ${webuiPort}`) - } catch (webuiError) { - if (webuiError instanceof WebUiPortInUseError) { - webuiBootFailure = {conflictPort: webuiError.port, status: 'port_in_use'} - log(`Web UI port ${webuiError.port} is already in use — Web UI unavailable`) - } else { - log(`Web UI start error: ${webuiError instanceof Error ? webuiError.message : String(webuiError)}`) + let webuiStartError: unknown + let webuiActualPort: number | undefined + for (let offset = 0; offset < WEBUI_MAX_FALLBACK_ATTEMPTS; offset++) { + const attemptPort = webuiPreferredPort + offset + try { + // eslint-disable-next-line no-await-in-loop -- intentional sequential fallback + await webuiServer.start(attemptPort) + webuiActualPort = attemptPort + break + } catch (error) { + if (error instanceof WebUiPortInUseError) { + webuiStartError = error + continue + } + + webuiStartError = error + break } + } + if (webuiActualPort === undefined) { webuiServer = undefined + if (webuiStartError instanceof WebUiPortInUseError) { + webuiBootFailure = {conflictPort: webuiPreferredPort, status: 'port_in_use'} + log( + `Web UI ports ${webuiPreferredPort}-${webuiPreferredPort + WEBUI_MAX_FALLBACK_ATTEMPTS - 1} are all in use — Web UI unavailable`, + ) + } else { + log( + `Web UI start error: ${webuiStartError instanceof Error ? webuiStartError.message : String(webuiStartError)}`, + ) + } + } else { + writeWebuiState(webuiActualPort) + if (webuiActualPort === webuiPreferredPort) { + log(`Web UI server started on port ${webuiActualPort}`) + } else { + webuiRequestedPort = webuiPreferredPort + log(`Web UI port ${webuiPreferredPort} was in use — started on ${webuiActualPort} instead`) + } } // 5. Start heartbeat writer. Must run before transport.start(): pollForDaemon SIGTERMs daemons with stale heartbeat. @@ -577,8 +612,13 @@ async function main(): Promise { // Web UI port endpoint — used by `brv webui` to discover the stable port transportServer.onRequest(WebuiEvents.GET_PORT, () => { - const port = webuiServer?.getPort() - if (port !== undefined) return {port, status: 'ok'} + const activePort = webuiServer?.getPort() + if (activePort !== undefined) { + return webuiRequestedPort !== undefined && webuiRequestedPort !== activePort + ? {port: activePort, requestedPort: webuiRequestedPort, status: 'ok'} + : {port: activePort, status: 'ok'} + } + return webuiBootFailure ?? {status: 'not_started'} }) @@ -617,6 +657,7 @@ async function main(): Promise { } webuiBootFailure = undefined + webuiRequestedPort = undefined writeWebuiState(newPort) writeWebuiPreferredPort(newPort) log(`Web UI server restarted on port ${newPort} (persisted)`) diff --git a/src/shared/transport/events/webui-events.ts b/src/shared/transport/events/webui-events.ts index 1e059f4d9..d463532c7 100644 --- a/src/shared/transport/events/webui-events.ts +++ b/src/shared/transport/events/webui-events.ts @@ -5,7 +5,7 @@ export const WebuiEvents = { export type WebuiGetPortResponse = | {conflictPort: number; status: 'port_in_use'} - | {port: number; status: 'ok'} + | {port: number; requestedPort?: number; status: 'ok'} | {status: 'not_started'} export interface WebuiSetPortRequest { From adbb0d304668ff47c116f92b563a52127274a1b4 Mon Sep 17 00:00:00 2001 From: ncnthien Date: Mon, 1 Jun 2026 15:16:59 +0700 Subject: [PATCH 5/8] feat: [ENG-2857] extract testable fallback helper + honor explicit port preference --- src/oclif/commands/webui.ts | 2 +- src/server/infra/daemon/brv-server.ts | 55 +++++-------- .../infra/daemon/start-webui-with-fallback.ts | 29 +++++++ src/shared/transport/events/webui-events.ts | 10 +-- .../daemon/start-webui-with-fallback.test.ts | 78 +++++++++++++++++++ 5 files changed, 134 insertions(+), 40 deletions(-) create mode 100644 src/server/infra/daemon/start-webui-with-fallback.ts create mode 100644 test/unit/infra/daemon/start-webui-with-fallback.test.ts diff --git a/src/oclif/commands/webui.ts b/src/oclif/commands/webui.ts index 4bd06a4cc..278de57e9 100644 --- a/src/oclif/commands/webui.ts +++ b/src/oclif/commands/webui.ts @@ -55,7 +55,7 @@ export default class Webui extends Command { private resolvePortOrExit(result: WebuiGetPortResponse | WebuiSetPortResponse): number { if (result.status === 'ok') { - if ('requestedPort' in result && result.requestedPort !== undefined && result.requestedPort !== result.port) { + if (result.requestedPort !== undefined && result.requestedPort !== result.port) { this.log(`Port ${result.requestedPort} was in use — using port ${result.port} instead.`) } diff --git a/src/server/infra/daemon/brv-server.ts b/src/server/infra/daemon/brv-server.ts index 15bb332ef..2b45e579f 100644 --- a/src/server/infra/daemon/brv-server.ts +++ b/src/server/infra/daemon/brv-server.ts @@ -100,6 +100,7 @@ import {IdleTimeoutPolicy} from './idle-timeout-policy.js' import {selectDaemonPort} from './port-selector.js' import {bootstrapSettings} from './settings-bootstrap.js' import {ShutdownHandler} from './shutdown-handler.js' +import {startWebUiWithFallback} from './start-webui-with-fallback.js' function log(msg: string): void { processLog(`[Daemon] ${msg}`) @@ -151,7 +152,6 @@ function cleanupOldLogs(logsDir: string, keep: number): void { } } -// eslint-disable-next-line complexity async function main(): Promise { // 1. Setup daemon logging at /logs/server-.log const daemonLogsDir = join(getGlobalDataDir(), 'logs') @@ -218,9 +218,11 @@ async function main(): Promise { const webuiDistDir = join(projectRoot, 'dist', 'webui') // Port priority: env var > persisted preference > default const webuiPortEnv = process.env.BRV_WEBUI_PORT - const webuiPreferredPort = webuiPortEnv - ? Number.parseInt(webuiPortEnv, 10) - : (readWebuiPreferredPort() ?? WEBUI_DEFAULT_PORT) + const explicitWebuiPort = webuiPortEnv ? Number.parseInt(webuiPortEnv, 10) : readWebuiPreferredPort() + const webuiPreferredPort = explicitWebuiPort ?? WEBUI_DEFAULT_PORT + // Explicit user intent (env/persisted) is honored strictly. The default + // port is the only one that may fall back to neighbors when busy. + const webuiMaxAttempts = explicitWebuiPort === undefined ? WEBUI_MAX_FALLBACK_ATTEMPTS : 1 const webuiApp = createWebUiMiddleware({ getConfig: () => ({ @@ -236,46 +238,31 @@ async function main(): Promise { app.use(webuiApp) webuiServer = new WebUiServer(app) - let webuiStartError: unknown - let webuiActualPort: number | undefined - for (let offset = 0; offset < WEBUI_MAX_FALLBACK_ATTEMPTS; offset++) { - const attemptPort = webuiPreferredPort + offset - try { - // eslint-disable-next-line no-await-in-loop -- intentional sequential fallback - await webuiServer.start(attemptPort) - webuiActualPort = attemptPort - break - } catch (error) { - if (error instanceof WebUiPortInUseError) { - webuiStartError = error - continue - } + const webuiOutcome = await startWebUiWithFallback(webuiServer, webuiPreferredPort, webuiMaxAttempts) - webuiStartError = error - break + if ('actualPort' in webuiOutcome) { + writeWebuiState(webuiOutcome.actualPort) + if (webuiOutcome.actualPort === webuiOutcome.requestedPort) { + log(`Web UI server started on port ${webuiOutcome.actualPort}`) + } else { + webuiRequestedPort = webuiOutcome.requestedPort + log(`Web UI port ${webuiOutcome.requestedPort} was in use — started on ${webuiOutcome.actualPort} instead`) } - } - - if (webuiActualPort === undefined) { + } else { webuiServer = undefined - if (webuiStartError instanceof WebUiPortInUseError) { + if (webuiOutcome.error instanceof WebUiPortInUseError) { webuiBootFailure = {conflictPort: webuiPreferredPort, status: 'port_in_use'} + const rangeEnd = webuiPreferredPort + webuiMaxAttempts - 1 log( - `Web UI ports ${webuiPreferredPort}-${webuiPreferredPort + WEBUI_MAX_FALLBACK_ATTEMPTS - 1} are all in use — Web UI unavailable`, + webuiMaxAttempts > 1 + ? `Web UI ports ${webuiPreferredPort}-${rangeEnd} are all in use — Web UI unavailable` + : `Web UI port ${webuiPreferredPort} is already in use — Web UI unavailable`, ) } else { log( - `Web UI start error: ${webuiStartError instanceof Error ? webuiStartError.message : String(webuiStartError)}`, + `Web UI start error: ${webuiOutcome.error instanceof Error ? webuiOutcome.error.message : String(webuiOutcome.error)}`, ) } - } else { - writeWebuiState(webuiActualPort) - if (webuiActualPort === webuiPreferredPort) { - log(`Web UI server started on port ${webuiActualPort}`) - } else { - webuiRequestedPort = webuiPreferredPort - log(`Web UI port ${webuiPreferredPort} was in use — started on ${webuiActualPort} instead`) - } } // 5. Start heartbeat writer. Must run before transport.start(): pollForDaemon SIGTERMs daemons with stale heartbeat. diff --git a/src/server/infra/daemon/start-webui-with-fallback.ts b/src/server/infra/daemon/start-webui-with-fallback.ts new file mode 100644 index 000000000..a480c93d5 --- /dev/null +++ b/src/server/infra/daemon/start-webui-with-fallback.ts @@ -0,0 +1,29 @@ +import {WebUiPortInUseError} from '../../core/domain/errors/webui-error.js' + +export interface WebUiStarter { + start(port: number): Promise +} + +export type StartWebUiOutcome = {actualPort: number; requestedPort: number} | {error: unknown} + +export async function startWebUiWithFallback( + server: WebUiStarter, + preferredPort: number, + maxAttempts: number, +): Promise { + let lastError: unknown + for (let offset = 0; offset < maxAttempts; offset++) { + const attemptPort = preferredPort + offset + try { + // eslint-disable-next-line no-await-in-loop -- intentional sequential fallback + await server.start(attemptPort) + return {actualPort: attemptPort, requestedPort: preferredPort} + } catch (error) { + lastError = error + if (error instanceof WebUiPortInUseError) continue + break + } + } + + return {error: lastError} +} diff --git a/src/shared/transport/events/webui-events.ts b/src/shared/transport/events/webui-events.ts index d463532c7..3d99c26d6 100644 --- a/src/shared/transport/events/webui-events.ts +++ b/src/shared/transport/events/webui-events.ts @@ -3,13 +3,13 @@ export const WebuiEvents = { SET_PORT: 'webui:setPort', } as const -export type WebuiGetPortResponse = - | {conflictPort: number; status: 'port_in_use'} - | {port: number; requestedPort?: number; status: 'ok'} - | {status: 'not_started'} +type WebuiOkResponse = {port: number; requestedPort?: number; status: 'ok'} +type WebuiPortInUseResponse = {conflictPort: number; status: 'port_in_use'} + +export type WebuiGetPortResponse = WebuiOkResponse | WebuiPortInUseResponse | {status: 'not_started'} export interface WebuiSetPortRequest { port: number } -export type WebuiSetPortResponse = {conflictPort: number; status: 'port_in_use'} | {port: number; status: 'ok'} +export type WebuiSetPortResponse = WebuiOkResponse | WebuiPortInUseResponse diff --git a/test/unit/infra/daemon/start-webui-with-fallback.test.ts b/test/unit/infra/daemon/start-webui-with-fallback.test.ts new file mode 100644 index 000000000..afee1fd1a --- /dev/null +++ b/test/unit/infra/daemon/start-webui-with-fallback.test.ts @@ -0,0 +1,78 @@ +import {expect} from 'chai' +import express from 'express' +import {restore, type SinonStub, stub} from 'sinon' + +import {WebUiPortInUseError} from '../../../../src/server/core/domain/errors/webui-error.js' +import {startWebUiWithFallback} from '../../../../src/server/infra/daemon/start-webui-with-fallback.js' +import {WebUiServer} from '../../../../src/server/infra/webui/webui-server.js' + +describe('startWebUiWithFallback', () => { + let server: WebUiServer + let startStub: SinonStub + + beforeEach(() => { + server = new WebUiServer(express()) + startStub = stub(server, 'start') + }) + + afterEach(() => { + restore() + }) + + it('returns the preferred port on first-attempt success', async () => { + startStub.resolves() + + const outcome = await startWebUiWithFallback(server, 7700, 10) + + expect(outcome).to.deep.equal({actualPort: 7700, requestedPort: 7700}) + expect(startStub.calledOnceWithExactly(7700)).to.be.true + }) + + it('falls back to the next port when the preferred port is in use', async () => { + startStub.onFirstCall().rejects(new WebUiPortInUseError(7700)) + startStub.onSecondCall().resolves() + + const outcome = await startWebUiWithFallback(server, 7700, 10) + + expect(outcome).to.deep.equal({actualPort: 7701, requestedPort: 7700}) + expect(startStub.callCount).to.equal(2) + expect(startStub.firstCall.args[0]).to.equal(7700) + expect(startStub.secondCall.args[0]).to.equal(7701) + }) + + it('returns the last error when every attempt fails with EADDRINUSE', async () => { + startStub.rejects(new WebUiPortInUseError(7700)) + + const outcome = await startWebUiWithFallback(server, 7700, 3) + + expect('error' in outcome).to.be.true + if ('error' in outcome) { + expect(outcome.error).to.be.an.instanceOf(WebUiPortInUseError) + } + + expect(startStub.callCount).to.equal(3) + }) + + it('treats maxAttempts=1 as strict mode (no fallback)', async () => { + startStub.rejects(new WebUiPortInUseError(9090)) + + const outcome = await startWebUiWithFallback(server, 9090, 1) + + expect('error' in outcome).to.be.true + expect(startStub.calledOnceWithExactly(9090)).to.be.true + }) + + it('breaks immediately on non-EADDRINUSE errors', async () => { + const genericError = new Error('permission denied') + startStub.rejects(genericError) + + const outcome = await startWebUiWithFallback(server, 7700, 10) + + expect('error' in outcome).to.be.true + if ('error' in outcome) { + expect(outcome.error).to.equal(genericError) + } + + expect(startStub.calledOnce).to.be.true + }) +}) From 469796e785ead691ae9aedabd62df97081f15718 Mon Sep 17 00:00:00 2001 From: ncnthien Date: Mon, 1 Jun 2026 15:24:26 +0700 Subject: [PATCH 6/8] feat: [ENG-2857] explicit status on StartWebUiOutcome, split ok variants, IPv4 URL --- src/oclif/commands/webui.ts | 41 +++++++++++-------- src/server/infra/daemon/brv-server.ts | 2 +- .../infra/daemon/start-webui-with-fallback.ts | 8 ++-- src/shared/transport/events/webui-events.ts | 7 ++-- .../daemon/start-webui-with-fallback.test.ts | 14 +++---- 5 files changed, 40 insertions(+), 32 deletions(-) diff --git a/src/oclif/commands/webui.ts b/src/oclif/commands/webui.ts index 278de57e9..2f6492eaf 100644 --- a/src/oclif/commands/webui.ts +++ b/src/oclif/commands/webui.ts @@ -21,9 +21,8 @@ export default class Webui extends Command { public async run(): Promise { const {flags} = await this.parse(Webui) - const result = flags.port ? await this.requestSetPort(flags.port) : await this.requestGetPort() - const webuiPort = this.resolvePortOrExit(result) - const url = `http://localhost:${webuiPort}` + const webuiPort = flags.port ? await this.resolveSetPort(flags.port) : await this.resolveGetPort() + const url = `http://127.0.0.1:${webuiPort}` this.log(`ByteRover Web UI: ${url}`) await open(url).catch(() => { @@ -31,29 +30,17 @@ export default class Webui extends Command { }) } - private async requestGetPort(): Promise { + private async resolveGetPort(): Promise { + let result: WebuiGetPortResponse try { - return await withDaemonRetry( + result = await withDaemonRetry( async (client) => client.requestWithAck(WebuiEvents.GET_PORT), {projectPath: process.cwd()}, ) } catch (error) { return this.error(formatConnectionError(error)) } - } - private async requestSetPort(port: number): Promise { - try { - return await withDaemonRetry( - async (client) => client.requestWithAck(WebuiEvents.SET_PORT, {port}), - {projectPath: process.cwd()}, - ) - } catch (error) { - return this.error(formatConnectionError(error)) - } - } - - private resolvePortOrExit(result: WebuiGetPortResponse | WebuiSetPortResponse): number { if (result.status === 'ok') { if (result.requestedPort !== undefined && result.requestedPort !== result.port) { this.log(`Port ${result.requestedPort} was in use — using port ${result.port} instead.`) @@ -70,4 +57,22 @@ export default class Webui extends Command { return this.error('Web UI did not start. Run `brv restart` and try again.') } + + private async resolveSetPort(port: number): Promise { + let result: WebuiSetPortResponse + try { + result = await withDaemonRetry( + async (client) => client.requestWithAck(WebuiEvents.SET_PORT, {port}), + {projectPath: process.cwd()}, + ) + } catch (error) { + return this.error(formatConnectionError(error)) + } + + if (result.status === 'ok') return result.port + + return this.error( + `Web UI port ${result.conflictPort} is already in use. Run \`brv webui --port \` to choose a different port.`, + ) + } } diff --git a/src/server/infra/daemon/brv-server.ts b/src/server/infra/daemon/brv-server.ts index 2b45e579f..3375aee70 100644 --- a/src/server/infra/daemon/brv-server.ts +++ b/src/server/infra/daemon/brv-server.ts @@ -240,7 +240,7 @@ async function main(): Promise { webuiServer = new WebUiServer(app) const webuiOutcome = await startWebUiWithFallback(webuiServer, webuiPreferredPort, webuiMaxAttempts) - if ('actualPort' in webuiOutcome) { + if (webuiOutcome.status === 'ok') { writeWebuiState(webuiOutcome.actualPort) if (webuiOutcome.actualPort === webuiOutcome.requestedPort) { log(`Web UI server started on port ${webuiOutcome.actualPort}`) diff --git a/src/server/infra/daemon/start-webui-with-fallback.ts b/src/server/infra/daemon/start-webui-with-fallback.ts index a480c93d5..83614ebf1 100644 --- a/src/server/infra/daemon/start-webui-with-fallback.ts +++ b/src/server/infra/daemon/start-webui-with-fallback.ts @@ -4,7 +4,9 @@ export interface WebUiStarter { start(port: number): Promise } -export type StartWebUiOutcome = {actualPort: number; requestedPort: number} | {error: unknown} +export type StartWebUiOutcome = + | {actualPort: number; requestedPort: number; status: 'ok'} + | {error: unknown; status: 'error'} export async function startWebUiWithFallback( server: WebUiStarter, @@ -17,7 +19,7 @@ export async function startWebUiWithFallback( try { // eslint-disable-next-line no-await-in-loop -- intentional sequential fallback await server.start(attemptPort) - return {actualPort: attemptPort, requestedPort: preferredPort} + return {actualPort: attemptPort, requestedPort: preferredPort, status: 'ok'} } catch (error) { lastError = error if (error instanceof WebUiPortInUseError) continue @@ -25,5 +27,5 @@ export async function startWebUiWithFallback( } } - return {error: lastError} + return {error: lastError, status: 'error'} } diff --git a/src/shared/transport/events/webui-events.ts b/src/shared/transport/events/webui-events.ts index 3d99c26d6..27893f6fa 100644 --- a/src/shared/transport/events/webui-events.ts +++ b/src/shared/transport/events/webui-events.ts @@ -3,13 +3,14 @@ export const WebuiEvents = { SET_PORT: 'webui:setPort', } as const -type WebuiOkResponse = {port: number; requestedPort?: number; status: 'ok'} +type WebuiGetPortOkResponse = {port: number; requestedPort?: number; status: 'ok'} type WebuiPortInUseResponse = {conflictPort: number; status: 'port_in_use'} +type WebuiSetPortOkResponse = {port: number; status: 'ok'} -export type WebuiGetPortResponse = WebuiOkResponse | WebuiPortInUseResponse | {status: 'not_started'} +export type WebuiGetPortResponse = WebuiGetPortOkResponse | WebuiPortInUseResponse | {status: 'not_started'} export interface WebuiSetPortRequest { port: number } -export type WebuiSetPortResponse = WebuiOkResponse | WebuiPortInUseResponse +export type WebuiSetPortResponse = WebuiPortInUseResponse | WebuiSetPortOkResponse diff --git a/test/unit/infra/daemon/start-webui-with-fallback.test.ts b/test/unit/infra/daemon/start-webui-with-fallback.test.ts index afee1fd1a..cd0ffab1f 100644 --- a/test/unit/infra/daemon/start-webui-with-fallback.test.ts +++ b/test/unit/infra/daemon/start-webui-with-fallback.test.ts @@ -24,7 +24,7 @@ describe('startWebUiWithFallback', () => { const outcome = await startWebUiWithFallback(server, 7700, 10) - expect(outcome).to.deep.equal({actualPort: 7700, requestedPort: 7700}) + expect(outcome).to.deep.equal({actualPort: 7700, requestedPort: 7700, status: 'ok'}) expect(startStub.calledOnceWithExactly(7700)).to.be.true }) @@ -34,7 +34,7 @@ describe('startWebUiWithFallback', () => { const outcome = await startWebUiWithFallback(server, 7700, 10) - expect(outcome).to.deep.equal({actualPort: 7701, requestedPort: 7700}) + expect(outcome).to.deep.equal({actualPort: 7701, requestedPort: 7700, status: 'ok'}) expect(startStub.callCount).to.equal(2) expect(startStub.firstCall.args[0]).to.equal(7700) expect(startStub.secondCall.args[0]).to.equal(7701) @@ -45,8 +45,8 @@ describe('startWebUiWithFallback', () => { const outcome = await startWebUiWithFallback(server, 7700, 3) - expect('error' in outcome).to.be.true - if ('error' in outcome) { + expect(outcome.status).to.equal('error') + if (outcome.status === 'error') { expect(outcome.error).to.be.an.instanceOf(WebUiPortInUseError) } @@ -58,7 +58,7 @@ describe('startWebUiWithFallback', () => { const outcome = await startWebUiWithFallback(server, 9090, 1) - expect('error' in outcome).to.be.true + expect(outcome.status).to.equal('error') expect(startStub.calledOnceWithExactly(9090)).to.be.true }) @@ -68,8 +68,8 @@ describe('startWebUiWithFallback', () => { const outcome = await startWebUiWithFallback(server, 7700, 10) - expect('error' in outcome).to.be.true - if ('error' in outcome) { + expect(outcome.status).to.equal('error') + if (outcome.status === 'error') { expect(outcome.error).to.equal(genericError) } From 77ba89f35171419580b396030ca8f2c05af39957 Mon Sep 17 00:00:00 2001 From: ncnthien Date: Mon, 1 Jun 2026 15:54:39 +0700 Subject: [PATCH 7/8] feat: [ENG-2857] revert CLI URL to localhost per product preference --- MANUAL_TEST_ENG-2857.md | 164 ++++++++++++++++++++++++++++++++++++ src/oclif/commands/webui.ts | 2 +- 2 files changed, 165 insertions(+), 1 deletion(-) create mode 100644 MANUAL_TEST_ENG-2857.md diff --git a/MANUAL_TEST_ENG-2857.md b/MANUAL_TEST_ENG-2857.md new file mode 100644 index 000000000..cee01f91f --- /dev/null +++ b/MANUAL_TEST_ENG-2857.md @@ -0,0 +1,164 @@ +# Manual Test Plan — ENG-2857 + +PR: https://github.com/campfirein/byterover-cli/pull/704 +Scope: `brv webui` port handling +- Auto-fallback to the next free port when the default (7700) is taken +- Specific, actionable error messages for port conflicts +- Strict behavior for explicit choices (`--port`, `BRV_WEBUI_PORT`, persisted preference) + +--- + +## Pre-test setup + +```sh +# 1) Build so dist/ matches the PR head +npm run build + +# 2) Clean state — stop daemon + clear persisted port preference +./bin/dev.js restart +rm -f "$HOME/Library/Application Support/brv/webui-config.json" +``` + +> **Tip:** All hogs in this plan bind `127.0.0.1` (IPv4) explicitly. A bare `listen(7700)` in Node binds IPv6 by default, which does **not** conflict with our daemon's IPv4 socket and gives a misleading "happy path" result. + +--- + +## Test 1 — Happy path on default port + +```sh +./bin/run.js webui +``` + +**Expect:** +``` +ByteRover Web UI: http://localhost:7700 +``` +Browser opens. + +--- + +## Test 2 — Auto-fallback on the default port + +In a separate terminal, hold port 7700 (IPv4): + +```sh +node -e "require('node:http').createServer((_,r)=>r.end()).listen(7700,'127.0.0.1',()=>console.log('hog'))" +``` + +Back in the brv terminal: + +```sh +./bin/dev.js restart # respawn daemon while 7700 is held +./bin/run.js webui +``` + +**Expect:** +``` +Port 7700 was in use — using port 7701 instead. +ByteRover Web UI: http://localhost:7701 +``` + +The browser should open and successfully reach the Web UI on 7701. + +--- + +## Test 3 — Explicit `--port` is strict (no fallback) + +With 7700 still held from Test 2: + +```sh +./bin/run.js webui --port 7700 +``` + +**Expect:** +``` +Error: Web UI port 7700 is already in use. Run `brv webui --port ` to choose a different port. +``` + +Verify there is **no** silent fallback. The user explicitly typed 7700; we honor their choice strictly. + +--- + +## Test 4 — Explicit env var is strict + +```sh +./bin/dev.js restart +BRV_WEBUI_PORT=7700 ./bin/run.js webui +``` + +**Expect:** Same `Error: Web UI port 7700 is already in use...` message. Env var is treated as explicit intent — no auto-fallback. + +--- + +## Test 5 — Persisted preference is strict + +Free 9090 and clear hogs first, then set up the scenario: + +```sh +# In the hog terminal: Ctrl-C the 7700 hog +npx kill-port 7700 + +# Persist 9090 as the preferred port via setPort +./bin/dev.js restart +./bin/run.js webui --port 9090 +# (expect happy path on 9090) +``` + +Now hog 9090 and restart so the daemon hits the persisted preference at boot: + +```sh +# In the hog terminal: +node -e "require('node:http').createServer((_,r)=>r.end()).listen(9090,'127.0.0.1',()=>console.log('hog'))" + +# In the brv terminal: +./bin/dev.js restart +./bin/run.js webui +``` + +**Expect:** +``` +Error: Web UI port 9090 is already in use. Run `brv webui --port ` to choose a different port. +``` + +The daemon honored the persisted preference strictly instead of silently shifting to 9091. + +--- + +## Test 6 — Connection-failure path still routes through the generic handler + +```sh +./bin/dev.js restart # kill daemon +BRV_IAM_BASE_URL='' ./bin/run.js webui # makes daemon spawn fail +``` + +**Expect:** A daemon-connection error, e.g. +``` +Error: Connection error: Failed to start daemon: timed out waiting for daemon to become ready + Run 'brv restart' if the daemon is unresponsive. +``` + +This must **not** be any of the new port-conflict messages — confirms the new error branches don't accidentally swallow connection-layer errors. + +--- + +## Cleanup + +```sh +# Free any port hogs +npx kill-port 7700 9090 9091 2>/dev/null + +# Reset state +./bin/dev.js restart +rm -f "$HOME/Library/Application Support/brv/webui-config.json" +``` + +--- + +## Pass criteria + +- [ ] Test 1: URL is `http://127.0.0.1:7700`, browser opens. +- [ ] Test 2: Fallback notice prints; URL shifts to 7701; browser opens on 7701. +- [ ] Test 3: `--port 7700` fails strictly, no fallback attempted. +- [ ] Test 4: `BRV_WEBUI_PORT=7700` fails strictly, no fallback attempted. +- [ ] Test 5: Persisted preference 9090 fails strictly when 9090 is held, no shift to 9091. +- [ ] Test 6: Daemon-spawn failure produces a "Connection error: ..." message, not a port message. diff --git a/src/oclif/commands/webui.ts b/src/oclif/commands/webui.ts index 2f6492eaf..38fd7d0ed 100644 --- a/src/oclif/commands/webui.ts +++ b/src/oclif/commands/webui.ts @@ -22,7 +22,7 @@ export default class Webui extends Command { const {flags} = await this.parse(Webui) const webuiPort = flags.port ? await this.resolveSetPort(flags.port) : await this.resolveGetPort() - const url = `http://127.0.0.1:${webuiPort}` + const url = `http://localhost:${webuiPort}` this.log(`ByteRover Web UI: ${url}`) await open(url).catch(() => { From 1bd56b5747ffd5ddf4b419bf2d567cab58cdbee8 Mon Sep 17 00:00:00 2001 From: ncnthien Date: Mon, 1 Jun 2026 15:57:34 +0700 Subject: [PATCH 8/8] feat: [ENG-2857] untrack MANUAL_TEST_ENG-2857.md (local QA artifact) --- MANUAL_TEST_ENG-2857.md | 164 ---------------------------------------- 1 file changed, 164 deletions(-) delete mode 100644 MANUAL_TEST_ENG-2857.md diff --git a/MANUAL_TEST_ENG-2857.md b/MANUAL_TEST_ENG-2857.md deleted file mode 100644 index cee01f91f..000000000 --- a/MANUAL_TEST_ENG-2857.md +++ /dev/null @@ -1,164 +0,0 @@ -# Manual Test Plan — ENG-2857 - -PR: https://github.com/campfirein/byterover-cli/pull/704 -Scope: `brv webui` port handling -- Auto-fallback to the next free port when the default (7700) is taken -- Specific, actionable error messages for port conflicts -- Strict behavior for explicit choices (`--port`, `BRV_WEBUI_PORT`, persisted preference) - ---- - -## Pre-test setup - -```sh -# 1) Build so dist/ matches the PR head -npm run build - -# 2) Clean state — stop daemon + clear persisted port preference -./bin/dev.js restart -rm -f "$HOME/Library/Application Support/brv/webui-config.json" -``` - -> **Tip:** All hogs in this plan bind `127.0.0.1` (IPv4) explicitly. A bare `listen(7700)` in Node binds IPv6 by default, which does **not** conflict with our daemon's IPv4 socket and gives a misleading "happy path" result. - ---- - -## Test 1 — Happy path on default port - -```sh -./bin/run.js webui -``` - -**Expect:** -``` -ByteRover Web UI: http://localhost:7700 -``` -Browser opens. - ---- - -## Test 2 — Auto-fallback on the default port - -In a separate terminal, hold port 7700 (IPv4): - -```sh -node -e "require('node:http').createServer((_,r)=>r.end()).listen(7700,'127.0.0.1',()=>console.log('hog'))" -``` - -Back in the brv terminal: - -```sh -./bin/dev.js restart # respawn daemon while 7700 is held -./bin/run.js webui -``` - -**Expect:** -``` -Port 7700 was in use — using port 7701 instead. -ByteRover Web UI: http://localhost:7701 -``` - -The browser should open and successfully reach the Web UI on 7701. - ---- - -## Test 3 — Explicit `--port` is strict (no fallback) - -With 7700 still held from Test 2: - -```sh -./bin/run.js webui --port 7700 -``` - -**Expect:** -``` -Error: Web UI port 7700 is already in use. Run `brv webui --port ` to choose a different port. -``` - -Verify there is **no** silent fallback. The user explicitly typed 7700; we honor their choice strictly. - ---- - -## Test 4 — Explicit env var is strict - -```sh -./bin/dev.js restart -BRV_WEBUI_PORT=7700 ./bin/run.js webui -``` - -**Expect:** Same `Error: Web UI port 7700 is already in use...` message. Env var is treated as explicit intent — no auto-fallback. - ---- - -## Test 5 — Persisted preference is strict - -Free 9090 and clear hogs first, then set up the scenario: - -```sh -# In the hog terminal: Ctrl-C the 7700 hog -npx kill-port 7700 - -# Persist 9090 as the preferred port via setPort -./bin/dev.js restart -./bin/run.js webui --port 9090 -# (expect happy path on 9090) -``` - -Now hog 9090 and restart so the daemon hits the persisted preference at boot: - -```sh -# In the hog terminal: -node -e "require('node:http').createServer((_,r)=>r.end()).listen(9090,'127.0.0.1',()=>console.log('hog'))" - -# In the brv terminal: -./bin/dev.js restart -./bin/run.js webui -``` - -**Expect:** -``` -Error: Web UI port 9090 is already in use. Run `brv webui --port ` to choose a different port. -``` - -The daemon honored the persisted preference strictly instead of silently shifting to 9091. - ---- - -## Test 6 — Connection-failure path still routes through the generic handler - -```sh -./bin/dev.js restart # kill daemon -BRV_IAM_BASE_URL='' ./bin/run.js webui # makes daemon spawn fail -``` - -**Expect:** A daemon-connection error, e.g. -``` -Error: Connection error: Failed to start daemon: timed out waiting for daemon to become ready - Run 'brv restart' if the daemon is unresponsive. -``` - -This must **not** be any of the new port-conflict messages — confirms the new error branches don't accidentally swallow connection-layer errors. - ---- - -## Cleanup - -```sh -# Free any port hogs -npx kill-port 7700 9090 9091 2>/dev/null - -# Reset state -./bin/dev.js restart -rm -f "$HOME/Library/Application Support/brv/webui-config.json" -``` - ---- - -## Pass criteria - -- [ ] Test 1: URL is `http://127.0.0.1:7700`, browser opens. -- [ ] Test 2: Fallback notice prints; URL shifts to 7701; browser opens on 7701. -- [ ] Test 3: `--port 7700` fails strictly, no fallback attempted. -- [ ] Test 4: `BRV_WEBUI_PORT=7700` fails strictly, no fallback attempted. -- [ ] Test 5: Persisted preference 9090 fails strictly when 9090 is held, no shift to 9091. -- [ ] Test 6: Daemon-spawn failure produces a "Connection error: ..." message, not a port message.