Skip to content

Commit 4f9df8f

Browse files
os-litantclaude
andauthored
fix(cli): os serve publishes the port it BOUND, on all three announcement channels (#13120)
* fix(cli): serve publishes the port it BOUND, on all three channels (#13062) `os serve` announced the port it was ASKED for on the `objectstack:listening` IPC message, the ready banner's `API:` row and `runtime.<environment>.json`. Requested and bound coincide for every port but one, so this stayed invisible; `--port 0` is the value where they cannot coincide (`MIN_PORT = 0` is legal on purpose — `listen(0)` binds a kernel-assigned port), and all three announced an address nothing was listening on, with nothing erroring. The three now read one number, resolved once off the transport's own `IHttpServer.getPort()` — the contract member that already promises the real bound port, in particular for `listen(0)`. The comment above the publish block asserted this invariant while the code did not hold it; it now says what the code does. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UjujZN219uFzBhSYfMykCd * test(cli): name the kernel by package, not by repo-relative path `check:cross-package-test-inputs` is a source scan and cannot tell a path in prose from one a test really opens, so a docblock spelling `packages/core/src/kernel.ts` read as an undeclared cross-package input. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UjujZN219uFzBhSYfMykCd --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
1 parent f2dc3ac commit 4f9df8f

6 files changed

Lines changed: 821 additions & 22 deletions

File tree

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
1+
---
2+
"@objectstack/cli": patch
3+
---
4+
5+
fix(cli): `os serve` publishes the port it BOUND, not the one it was asked for (#13062)
6+
7+
The three channels `serve` announces an address on — the `objectstack:listening`
8+
IPC message, the ready banner's `API:` row and `runtime.<environment>.json` —
9+
were three outputs of one number, and that number was the port the operator
10+
requested. For every port but one the requested and the bound value coincide, so
11+
this stayed invisible; `0` is the value where they cannot coincide.
12+
`MIN_PORT = 0` is legal on purpose (`utils/port-contract.ts`: 0 is "a REQUEST,
13+
not an error" — `listen(0)` binds a kernel-assigned port).
14+
15+
FROM (`os serve --port 0`): IPC `{ port: 0, url: 'http://localhost:0' }`, banner
16+
`API: http://localhost:0/`, `runtime.env_local.json` `"port": 0` — three
17+
channels naming an address nothing was listening on, with nothing erroring.
18+
19+
TO: all three name the port the HTTP server actually bound, read off the
20+
transport's own `IHttpServer.getPort()` (the contract member that already
21+
promises "the real bound port — in particular when `listen(0)` requested an
22+
ephemeral port"). The same repair covers a bind that walked past a port taken
23+
between `serve`'s probe and the transport's `listen()`.
24+
25+
Unchanged for every other port: when the requested port is the bound one — which
26+
is every ordinary boot, including one that dev-auto-shifted off a busy port —
27+
all three channels publish exactly what they published before. A boot with no
28+
HTTP server, or a transport that does not implement the optional member, also
29+
falls back to the previous value.
Lines changed: 244 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,244 @@
1+
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
2+
3+
/**
4+
* #13062 — `os serve` publishes the port it BOUND, on all three of the channels
5+
* that announce one.
6+
*
7+
* ## The defect, and why it hid
8+
*
9+
* The IPC message (`objectstack:listening`), the ready banner's `API:` row and
10+
* `runtime.<environment>.json` were three outputs of ONE number, and that
11+
* number was the port the operator ASKED for. For every value but one the
12+
* requested and the bound port coincide, so the three agreed with each other
13+
* AND with the socket, and nothing ever disagreed. For `0` they cannot
14+
* coincide: `utils/port-contract.ts` declares `MIN_PORT = 0` from its own
15+
* measurement and states that 0 is "a REQUEST, not an error" — `listen(0)`
16+
* binds a kernel-assigned port — so `os serve --port 0` announced
17+
* `{ port: 0 }`, printed `API: http://localhost:0/` and wrote `"port": 0`.
18+
* Three channels naming an address nothing listens on, with nothing erroring.
19+
*
20+
* ## Two halves, and the second is the one that rots
21+
*
22+
* The BEHAVIOUR half is {@link resolveBoundPort}, driven below against a fake
23+
* kernel — a unit, so the asymmetry that matters (`0` in, a real port out) is
24+
* exercised without a boot.
25+
*
26+
* The WIRING half cannot be reached that way at all: `run()` is one ~3000-line
27+
* method that needs a whole kernel to enter, so nothing in-process can observe
28+
* which variable its three publish sites read. That is exactly the half the
29+
* card is about — ⛔ "fix one channel and two go on lying, harder to find than
30+
* before" — so it is pinned by reading the source, with comments MASKED so a
31+
* sentence about the bound port can never answer for code that publishes the
32+
* requested one. (`test/serve-publishes-bound-port.e2e.test.ts` drives all
33+
* three through a real boot; this is the cheap half that fails in 40ms and
34+
* names which channel regressed.)
35+
*/
36+
37+
import { describe, it, expect } from 'vitest';
38+
import { readFileSync } from 'node:fs';
39+
import { resolve } from 'node:path';
40+
import { fileURLToPath } from 'node:url';
41+
42+
// The repo's ONE code/prose separator, typed by the hand-written `.d.mts`
43+
// beside it — the same import `utils/port-contract-single-source.test.ts` uses,
44+
// and for the same reason: this file asks "does the CODE publish the bound
45+
// port", and a comment claiming it does is precisely what was there before.
46+
import { maskComments } from '../../../../scripts/js-comment-mask.mjs';
47+
48+
import { resolveBoundPort } from './serve.js';
49+
import { MAX_PORT } from '../utils/port-contract.js';
50+
51+
/** …/packages/cli/src/commands — seeded from `import.meta.url`. */
52+
const HERE = resolve(fileURLToPath(import.meta.url), '..');
53+
54+
/** `serve.ts`'s CODE, with every comment span blanked. */
55+
const SERVE = maskComments(readFileSync(resolve(HERE, 'serve.ts'), 'utf8'));
56+
57+
/** `serve.ts` verbatim — only for asserting that the mask actually masked. */
58+
const SERVE_RAW = readFileSync(resolve(HERE, 'serve.ts'), 'utf8');
59+
60+
/**
61+
* A kernel whose transport reports `reported` from `getPort()`.
62+
*
63+
* ⚠️ The miss path THROWS rather than returning `undefined`, because that is
64+
* what `ObjectKernel.getService` really does — a miss is a composition fault
65+
* that `@objectstack/core` refuses to answer silently. A fake that returned
66+
* `undefined` would leave the production `try` untested and green.
67+
*
68+
* ⛔ That kernel is named by PACKAGE, never as a repo-relative path: this file
69+
* does not read it, and `check:cross-package-test-inputs` is a source scan that
70+
* cannot tell a path in prose from one this test really opens (measured — the
71+
* first draft of this comment failed that gate).
72+
*/
73+
function kernelReporting(
74+
reported: unknown,
75+
opts: { under?: string; getPort?: unknown } = {},
76+
): { kernel: { getService: (name: string) => unknown }; asked: string[] } {
77+
const under = opts.under ?? 'http.server';
78+
const asked: string[] = [];
79+
return {
80+
asked,
81+
kernel: {
82+
getService(name: string) {
83+
asked.push(name);
84+
if (name !== under) throw new Error(`Service '${name}' not found`);
85+
return 'getPort' in opts ? { getPort: opts.getPort } : { getPort: () => reported };
86+
},
87+
},
88+
};
89+
}
90+
91+
describe('#13062 resolveBoundPort — the transport answers, not the request', () => {
92+
it('answers with the BOUND port when the request was 0', () => {
93+
// ⭐ The whole card in one line: `--port 0` is the one request that can
94+
// never equal its answer, and it is the case every channel got wrong.
95+
const { kernel } = kernelReporting(44321);
96+
expect(resolveBoundPort(kernel, 0)).toBe(44321);
97+
});
98+
99+
it('answers with the BOUND port when a non-zero request drifted', () => {
100+
// The second way the two part company on this command, and it needs no
101+
// `--port 0`: `HonoHttpServer.listen()` walks past EADDRINUSE on its own,
102+
// so a port taken between this command's probe and the transport's
103+
// `listen()` is bound one higher than the number serve resolved.
104+
const { kernel } = kernelReporting(41235);
105+
expect(resolveBoundPort(kernel, 41234)).toBe(41235);
106+
});
107+
108+
it('is a no-op for the case that was always right — request === bound', () => {
109+
// ⛔ The half most easily broken on the way past: every ordinary boot must
110+
// publish exactly what it published before.
111+
const { kernel } = kernelReporting(41234);
112+
expect(resolveBoundPort(kernel, 41234)).toBe(41234);
113+
});
114+
115+
it('asks for the CANONICAL service name, never the deprecated alias', () => {
116+
const { kernel, asked } = kernelReporting(44321);
117+
resolveBoundPort(kernel, 0);
118+
expect(asked).toEqual(['http.server']);
119+
// `http-server` is the same instance under a deprecated second name
120+
// (#4251). Reading it here would be new code taking a retiring dependency.
121+
expect(asked).not.toContain('http-server');
122+
});
123+
124+
describe('the fallback is the OLD behaviour, and it may not narrow what boots', () => {
125+
it('falls back when nothing registered a transport (`--server=false`)', () => {
126+
const { kernel } = kernelReporting(0, { under: 'nothing-registers-this' });
127+
expect(resolveBoundPort(kernel, 3000)).toBe(3000);
128+
});
129+
130+
it('falls back when the transport does not implement the optional member', () => {
131+
const { kernel } = kernelReporting(0, { getPort: undefined });
132+
expect(resolveBoundPort(kernel, 3000)).toBe(3000);
133+
});
134+
135+
it('falls back when `getPort()` itself throws', () => {
136+
const { kernel } = kernelReporting(0, {
137+
getPort: () => { throw new Error('transport is mid-restart'); },
138+
});
139+
expect(resolveBoundPort(kernel, 3000)).toBe(3000);
140+
});
141+
142+
it('falls back for a kernel that has no `getService` at all', () => {
143+
expect(resolveBoundPort(undefined, 3000)).toBe(3000);
144+
expect(resolveBoundPort({}, 3000)).toBe(3000);
145+
});
146+
});
147+
148+
describe('what may not be published, whatever the transport says', () => {
149+
it('⛔ refuses 0 as an ANSWER, though 0 is a legal REQUEST', () => {
150+
// No socket is bound to port 0. A transport reporting it has not listened
151+
// yet — `HonoHttpServer.getPort()` returns its constructor argument until
152+
// the listening callback fires — and republishing it IS the defect.
153+
const { kernel } = kernelReporting(0);
154+
expect(resolveBoundPort(kernel, 41234)).toBe(41234);
155+
});
156+
157+
it('refuses anything that cannot be a bound port', () => {
158+
for (const bad of [-1, 1.5, Number.NaN, Number.POSITIVE_INFINITY, MAX_PORT + 1]) {
159+
const { kernel } = kernelReporting(bad);
160+
expect(resolveBoundPort(kernel, 41234), `accepted ${String(bad)}`).toBe(41234);
161+
}
162+
for (const bad of ['44321', null, undefined, {}]) {
163+
const { kernel } = kernelReporting(bad);
164+
expect(resolveBoundPort(kernel, 41234), `accepted ${JSON.stringify(bad)}`).toBe(41234);
165+
}
166+
});
167+
168+
it('accepts the ceiling itself, read from the ONE port contract', () => {
169+
// ⛔ Never written as a literal here — `utils/port-contract.ts` is the one
170+
// place either bound is declared, and `port-contract-single-source.test.ts`
171+
// fails on a second copy.
172+
const { kernel } = kernelReporting(MAX_PORT);
173+
expect(resolveBoundPort(kernel, 41234)).toBe(MAX_PORT);
174+
});
175+
});
176+
});
177+
178+
describe('#13062 all THREE channels publish that one number — read off the code', () => {
179+
it('the `objectstack:listening` IPC message', () => {
180+
expect(
181+
SERVE,
182+
'the IPC message no longer publishes `boundPort` — `os dev` reads this channel to learn '
183+
+ 'where its child ended up, and #13061 records that `os start` could read it too',
184+
).toContain("process.send({ type: 'objectstack:listening', port: boundPort, url: runtimeUrl });");
185+
});
186+
187+
it('`runtime.<environment>.json`', () => {
188+
expect(
189+
SERVE,
190+
'the runtime state file no longer publishes `boundPort` — external supervisors and health '
191+
+ 'checks read it for the address to poll',
192+
).toMatch(/pid: process\.pid,\s*\n\s*port: boundPort,/);
193+
});
194+
195+
it('the ready banner, through the runtime\'s own base-URL chain', () => {
196+
expect(
197+
SERVE,
198+
'the banner no longer resolves its origin from `boundPort`',
199+
).toContain('externalBaseOrigin: resolveAuthBaseUrl(boundPort).baseOrigin');
200+
});
201+
202+
it('the URL those channels carry is composed from the same number', () => {
203+
expect(SERVE).toContain('const runtimeUrl = `http://localhost:${boundPort}`;');
204+
});
205+
206+
it('⛔ and NONE of the three has drifted back onto the requested port', () => {
207+
// The card's own instruction: three outputs of one defect, and repairing
208+
// one leaves two lying in a place nobody thinks to look next time.
209+
expect(SERVE).not.toContain('port: Number(port)');
210+
expect(SERVE).not.toContain('externalBaseOrigin: resolveAuthBaseUrl(port)');
211+
expect(SERVE).not.toContain('const runtimeUrl = `http://localhost:${port}`');
212+
});
213+
214+
it('resolves it ONCE, from the transport, after the boot', () => {
215+
expect(
216+
SERVE,
217+
'the bound port is no longer read off the transport — three call sites resolving it '
218+
+ 'separately is how they earn the right to disagree',
219+
).toContain('const boundPort = resolveBoundPort(kernel, port);');
220+
expect(SERVE.match(/const boundPort =/g) ?? []).toHaveLength(1);
221+
});
222+
223+
describe('ANTI-VACUITY: the scan reads CODE, and it read something', () => {
224+
it('the mask blanked comments rather than returning the file unchanged', () => {
225+
// Without this, every `not.toContain` above passes on an empty string.
226+
expect(SERVE.length).toBe(SERVE_RAW.length);
227+
expect(SERVE).not.toBe(SERVE_RAW);
228+
// A sentence that exists ONLY in a comment must be invisible to the scan…
229+
const proseOnly = 'republishing it is the defect itself';
230+
expect(SERVE_RAW, 'the control sentence was reworded — pick another').toContain(proseOnly);
231+
expect(SERVE).not.toContain(proseOnly);
232+
// …while the code around it is still there.
233+
expect(SERVE).toContain('export function resolveBoundPort(');
234+
});
235+
236+
it('the requested port is still what the transport is CONSTRUCTED with', () => {
237+
// The positive control for the negatives above: `port` has not been
238+
// globally renamed, so `not.toContain('port: Number(port)')` is a
239+
// measurement rather than a consequence of the variable disappearing.
240+
expect(SERVE).toContain('new HonoServerPlugin({ port })');
241+
expect(SERVE).toContain('port = await getAvailablePort(requestedPort)');
242+
});
243+
});
244+
});

0 commit comments

Comments
 (0)