mirror of
https://github.com/Ark0N/Codeman.git
synced 2026-10-10 01:09:43 +02:00
fix(server): report the port actually bound, so new WebServer(0) is usable
`port: 0` already bound an ephemeral port at the socket level, but `this.port` stayed 0: the banner printed `:0`, CODEMAN_API_URL pointed panes at `:0`, the docker bridge listener and the unauthenticated-bind warnings read 0, and the route context's `port` was a by-value snapshot taken in setupRoutes(), before listen() runs, so `tunnelManager.start(ctx.port, …)` would have been handed 0. - After `app.listen()`, `this.port` takes the number from `this.app.server.address()` (string/null addresses are left alone). - The route context exposes `port` as a getter, and the cron routes get only the `cron` their CronPort declares instead of a spread copy of the context. - `get boundPort()`: the readonly accessor tests use instead of a private field. test/webserver-bound-port.test.ts compares each reader against the socket's own `address().port`; all three tests fail with only the write-back removed. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 5.5
parent
ac94f339ac
commit
03629c966e
+22
-2
@@ -323,6 +323,14 @@ export class WebServer extends EventEmitter {
|
|||||||
private store = getStore();
|
private store = getStore();
|
||||||
private tabLayouts!: TabLayoutService;
|
private tabLayouts!: TabLayoutService;
|
||||||
private port: number;
|
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 host: string;
|
||||||
private https: boolean;
|
private https: boolean;
|
||||||
/** Reverse-proxy sub-path prefix (normalized: '' for root, or '/foo'). */
|
/** Reverse-proxy sub-path prefix (normalized: '' for root, or '/foo'). */
|
||||||
@@ -746,7 +754,11 @@ export class WebServer extends EventEmitter {
|
|||||||
saveRespawnConfig: this.saveRespawnConfig.bind(this),
|
saveRespawnConfig: this.saveRespawnConfig.bind(this),
|
||||||
// ConfigPort
|
// ConfigPort
|
||||||
store: this.store,
|
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,
|
https: this.https,
|
||||||
testMode: this.testMode,
|
testMode: this.testMode,
|
||||||
serverStartTime: this.serverStartTime,
|
serverStartTime: this.serverStartTime,
|
||||||
@@ -1154,7 +1166,9 @@ export class WebServer extends EventEmitter {
|
|||||||
// due times for any persisted jobs, then expose it to its routes.
|
// due times for any persisted jobs, then expose it to its routes.
|
||||||
this.cronService = new CronService(ctx);
|
this.cronService = new CronService(ctx);
|
||||||
this.cronService.init();
|
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());
|
registerWsRoutes(this.app, ctx, () => this.getHostPolicy());
|
||||||
registerVoiceRoutes(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 });
|
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 protocol = this.https ? 'https' : 'http';
|
||||||
const displayHost = this.host === '0.0.0.0' ? 'localhost' : this.host;
|
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
|
// The only startup banner: `codeman web` used to print its own copy of this
|
||||||
|
|||||||
@@ -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()}`);
|
||||||
|
});
|
||||||
|
});
|
||||||
Reference in New Issue
Block a user