diff --git a/src/web/server.ts b/src/web/server.ts index 4442e544..2ca28689 100644 --- a/src/web/server.ts +++ b/src/web/server.ts @@ -323,6 +323,14 @@ export class WebServer extends EventEmitter { private store = getStore(); private tabLayouts!: TabLayoutService; private port: number; + + /** + * The port the server is actually listening on once `start()` has resolved — the + * OS-assigned one for `new WebServer(0, …)` — and the constructor's port before that. + */ + get boundPort(): number { + return this.port; + } private host: string; private https: boolean; /** Reverse-proxy sub-path prefix (normalized: '' for root, or '/foo'). */ @@ -746,7 +754,11 @@ export class WebServer extends EventEmitter { saveRespawnConfig: this.saveRespawnConfig.bind(this), // ConfigPort store: this.store, - port: this.port, + // A getter, not a snapshot: this context is built in setupRoutes(), BEFORE + // listen() resolves an ephemeral `port: 0`, and the tunnel start reads it later. + get port() { + return self.port; + }, https: this.https, testMode: this.testMode, serverStartTime: this.serverStartTime, @@ -1154,7 +1166,9 @@ export class WebServer extends EventEmitter { // due times for any persisted jobs, then expose it to its routes. this.cronService = new CronService(ctx); this.cronService.init(); - registerCronRoutes(this.app, { ...ctx, cron: this.cronService }); + // Only what CronPort declares: a spread of ctx would copy `port` by value (the + // pre-listen 0 of an ephemeral bind) into an object nothing keeps in sync. + registerCronRoutes(this.app, { cron: this.cronService }); registerWsRoutes(this.app, ctx, () => this.getHostPolicy()); registerVoiceRoutes(this.app, ctx, () => this.getHostPolicy()); @@ -2909,6 +2923,12 @@ export class WebServer extends EventEmitter { } await this.app.listen({ port: this.port, host: this.host }); + // A `port: 0` bind gets its number from the OS. Everything below and every later + // reader (the banner, CODEMAN_API_URL, the docker bridge listener, the + // unauthenticated-bind warnings, the route context's getter) must see that number, + // not the 0 that was asked for. + const address = this.app.server.address(); + if (address !== null && typeof address === 'object') this.port = address.port; const protocol = this.https ? 'https' : 'http'; const displayHost = this.host === '0.0.0.0' ? 'localhost' : this.host; // The only startup banner: `codeman web` used to print its own copy of this diff --git a/test/webserver-bound-port.test.ts b/test/webserver-bound-port.test.ts new file mode 100644 index 00000000..e81612b9 --- /dev/null +++ b/test/webserver-bound-port.test.ts @@ -0,0 +1,46 @@ +/** + * @fileoverview `new WebServer(0, …)`: the OS picks the port and every reader of + * `this.port` sees that number, not the 0 that was asked for (#440). + * + * Port: ephemeral. + */ +import { afterAll, beforeAll, describe, expect, it } from 'vitest'; +import type { AddressInfo } from 'node:net'; +import { WebServer } from '../src/web/server.js'; + +describe('WebServer on an ephemeral port', () => { + let server: WebServer; + /** Built BEFORE start(), as setupRoutes() builds it, so a by-value snapshot would read 0. */ + let ctx: { port: number }; + const savedApiUrl = process.env.CODEMAN_API_URL; + /** The socket's own answer, so each test stands on its own instead of trusting boundPort. */ + const socketPort = () => + ((server as unknown as { app: { server: { address(): AddressInfo } } }).app.server.address() as AddressInfo).port; + + beforeAll(async () => { + server = new WebServer(0, false, true); + ctx = (server as unknown as { createRouteContext(): { port: number } }).createRouteContext(); + await server.start(); + }); + + afterAll(async () => { + await server.stop(); + if (savedApiUrl === undefined) delete process.env.CODEMAN_API_URL; + else process.env.CODEMAN_API_URL = savedApiUrl; + }); + + it('reports the port the OS assigned', async () => { + expect(socketPort()).toBeGreaterThan(0); + expect(server.boundPort).toBe(socketPort()); + const res = await fetch(`http://127.0.0.1:${server.boundPort}/api/status`); + expect(res.status).toBe(200); + }); + + it('the route context reads it live (the tunnel start gets the real port, not 0)', () => { + expect(ctx.port).toBe(socketPort()); + }); + + it('exports the real port to the panes as CODEMAN_API_URL', () => { + expect(process.env.CODEMAN_API_URL).toBe(`http://127.0.0.1:${socketPort()}`); + }); +});