Skip to content

Commit 2805e52

Browse files
os-litantclaude
andauthored
feat(cli): say so when a port is read as something other than what the text says (#12674) (#12692)
`os serve` reads its port with `parseInt`, whose tolerance changes the answer rather than the spelling: `--port 3e3` binds port 3, `--port 0x0BB8` binds 3000, `--port 3000abc` binds 3000. The boot succeeds on a port the operator never named and nothing says so. The accept set is untouched — narrowing a published CLI's accepted input is a contract decision, left open on purpose. Only the silence is repaired: a notice naming the text that was set and the port it selected, printed when a strict reading of the trimmed text does not name that port. Whitespace, a leading `+` and leading zeros are not differences (`" 3000"` is what production PORT values look like); an exponent, a radix prefix, a fraction, a separator and trailing text are. The source spelling is shared with #12662's refusal so one input is named one way, and the notice goes to stderr like every other `os serve` diagnostic. Claude-Session: https://claude.ai/code/session_01UjujZN219uFzBhSYfMykCd Co-authored-by: Claude <noreply@anthropic.com>
1 parent b853cf3 commit 2805e52

3 files changed

Lines changed: 571 additions & 10 deletions

File tree

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,46 @@
1+
---
2+
"@objectstack/cli": minor
3+
---
4+
5+
feat(cli): `os serve` says so when a port is read as something other than what the text says (#12674)
6+
7+
`os serve` reads its port with `parseInt`, and `parseInt` is tolerant in a way
8+
that changes the *answer* rather than the spelling. `--port 3e3` binds port
9+
**3**. `--port 0x0BB8` binds 3000. `--port 3000abc` binds 3000. The boot
10+
succeeds, on a port the operator never named, and nothing anywhere says so — an
11+
operator who wrote `PORT=3e3` meaning 3000 gets a server on port 3, and on a
12+
non-root host that surfaces (much later, if at all) as an `EACCES` that still
13+
does not name the coercion.
14+
15+
The value is now announced when it does not read as the port it selected:
16+
17+
```
18+
⚠ PORT="3e3" was read as port 3.
19+
That text is not a plain decimal number, and the reader that accepts it
20+
is tolerant: it honours a leading 0x as hexadecimal and discards
21+
everything from the first character that cannot continue the number.
22+
Nothing downstream reads it again — 3 is the port this server asked
23+
for, whatever the text looks like.
24+
If that is not the port you meant, correct PORT in this process's
25+
environment (for example PORT=3000), or override it with --port 3000.
26+
```
27+
28+
**Nothing is refused, and nothing binds differently.** The accept set is exactly
29+
what it was: every spelling that boots today still boots, on the same port, byte
30+
for byte. Whether `os serve` should take only strict decimal text is a contract
31+
question about a published CLI's accepted input, and it is deliberately left
32+
open. This repairs the silence, which is where the harm actually was.
33+
34+
The notice fires on a *difference*, so what counts as agreement is the whole of
35+
it: leading and trailing whitespace, a leading `+` and leading zeros do not
36+
change what the text says (`" 3000"`, `"+3000"` and `"08080"` are silent — the
37+
first is what production `PORT` values look like, and a notice there would drone
38+
at every ordinary boot). An exponent, a radix prefix, a fraction, a digit
39+
separator or trailing text all do (`"3e3"`, `"1e10"`, `"0x0BB8"`, `"0b111"`,
40+
`"3000.0"`, `"1_000"`, `"3000abc"` all speak).
41+
42+
It names both the text and the port it selected — never a third number, because
43+
`3e3` looks like 3000 to a reader but `3000abc` has no second reading and a
44+
guess would be wrong the first time it met one. Written to **stderr** like every
45+
other `os serve` diagnostic: `stdout` carries JSON-RPC frames whenever the stdio
46+
MCP transport is mounted.
Lines changed: 343 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,343 @@
1+
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
2+
3+
/**
4+
* #12674 — when `os serve` reads a port as something other than what the text
5+
* says, it SAYS SO. The accept set is untouched.
6+
*
7+
* ## The defect
8+
*
9+
* `parseInt` is the reader, and #12662's ruling keeps it: no value that boots
10+
* today may be refused, because narrowing a published CLI's accepted input is a
11+
* contract decision. But `parseInt`'s tolerance changes the ANSWER, not just
12+
* the spelling. `os serve --port 3e3` binds port **3**. `--port 0x0BB8` binds
13+
* 3000. `--port 3000abc` binds 3000. The boot SUCCEEDS, on a port the operator
14+
* never named, and nothing anywhere says so.
15+
*
16+
* ⭐ The harm is not that a strange value is accepted — it is a server
17+
* listening somewhere nobody asked for. That is what the notice repairs, and it
18+
* repairs only that: behaviour is unchanged, byte for byte.
19+
*
20+
* ## Why this file binds no sockets
21+
*
22+
* The decision is made before any socket exists, so a test that needs one is
23+
* testing the wrong layer — the ruling `serve-port-validation.test.ts` and
24+
* `serve-exhausted-port-search-notice.test.ts` both record, for the measured
25+
* reason (#12441: real-port contention took a full CLI suite red in this shared
26+
* container). Everything below drives exported pure functions and the live
27+
* source text. Zero sockets, zero spawns.
28+
*
29+
* ## THE THREE-WAY DISCRIMINATION (the anti-vacuity requirement)
30+
*
31+
* A pin asserting only "a mismatch prints a notice" is just as green against an
32+
* implementation that prints unconditionally. Three arms, each decidable at
33+
* runtime at the same seam:
34+
*
35+
* 1. **mismatch** (`3e3` → 3, `0x0BB8` → 3000) → a notice naming BOTH the text
36+
* and the port it selected.
37+
* 2. **agreement** (`3000`, `" 3000"`, `"+3000"`, `"08080"`) → `null`. ⭐ This
38+
* is the arm the whole card turns on: `" 3000"` is what production `PORT`
39+
* values look like, and a notice there would drone at every ordinary boot.
40+
* 3. **not a port at all** (`abc`, `99999`) → #12662's REFUSAL owns it, and
41+
* this notice is unreachable — the guard exits the process above the call
42+
* site. Pinned both ways: `parseRequestedPort` returns `null` for those
43+
* values, and the call site is lexically downstream of that exit.
44+
*
45+
* The three must not overlap or swallow one another, so the mutual-exclusion
46+
* arm covers all four port notices this command now carries — one pair of which
47+
* is load-bearing beyond legibility: `PORT_TAKEN_PATTERNS` in
48+
* `test/helpers/serve-process.ts` turns two of them into a "port contention"
49+
* verdict for every spawner in this package.
50+
*/
51+
52+
import { describe, it, expect } from 'vitest';
53+
import { readFileSync } from 'node:fs';
54+
import { resolve } from 'node:path';
55+
import { fileURLToPath } from 'node:url';
56+
57+
import {
58+
parseRequestedPort,
59+
strictPortReading,
60+
portTextReadNotice,
61+
formatInvalidPortNotice,
62+
type PortInputSource,
63+
} from './serve.js';
64+
65+
/** Seeded from `import.meta.url`, the spelling `check:cross-package-test-inputs` recognises. */
66+
const HERE = resolve(fileURLToPath(import.meta.url), '..');
67+
68+
/** The live source, for the arms decided lexically rather than at runtime. */
69+
const SERVE_SOURCE = readFileSync(resolve(HERE, 'serve.ts'), 'utf8');
70+
71+
/**
72+
* Strip SGR escapes — `chalk` is inert under a non-TTY runner but not
73+
* guaranteed to be, and an assertion that only passes with colour off is a
74+
* flake waiting for the first person who runs this attached. The ESC byte is
75+
* built with `String.fromCharCode` because this repo refuses raw control bytes
76+
* in source (`check:nul-bytes`).
77+
*/
78+
const SGR = new RegExp(`${String.fromCharCode(27)}\\[[0-9;]*m`, 'g');
79+
const plain = (text: string): string => text.replace(SGR, '');
80+
81+
/**
82+
* Does `notice` name this exact source spelling?
83+
*
84+
* ⚠️ The anchor is WIDENED rather than the assertion loosened, for the trap
85+
* `serve-port-validation.test.ts` measured on its own arm: `OS_PORT="3e3"`
86+
* CONTAINS `PORT="3e3"`, so a plain `toContain`/`not.toContain` pair reports
87+
* the OS_PORT notice as also naming PORT. A preceding `_` disqualifies the
88+
* match; a message that really did name both would still carry a `PORT=` with
89+
* no `_` in front of it and would still be caught.
90+
*/
91+
const names = (notice: string, spelled: string): boolean =>
92+
new RegExp(`(?<!_)${spelled.replace(/[.*+?^${}()|[\]\\]/g, '\\$&')}`).test(notice);
93+
94+
/**
95+
* Every spelling `parseRequestedPort` ACCEPTS, and whether the text says the
96+
* port that came out of it.
97+
*
98+
* MEASURED on this checkout with `node -e`, Node v22.22.2 — `parsed` is
99+
* `parseInt(raw)`, unchanged by this card:
100+
*
101+
* ```
102+
* " 3000" → 3000 "3000 " → 3000 "+3000" → 3000 "08080" → 8080
103+
* "3e3" → 3 "1e10" → 1 "0x0BB8" → 3000 "3000.0" → 3000
104+
* "0b111" → 0 "0o17" → 0 "1_000" → 1 "3000abc" → 3000
105+
* ```
106+
*
107+
* `saysIt` is this card's whole precision: whether a reader looking at the text
108+
* would name the port that was selected. Whitespace, a leading `+` and leading
109+
* zeros do not change what the text says; an exponent, a radix prefix, a
110+
* fraction, a separator and trailing text all do.
111+
*/
112+
const ACCEPTED: Array<{ raw: string; parsed: number; saysIt: boolean; note?: string }> = [
113+
{ raw: '3000', parsed: 3000, saysIt: true },
114+
{ raw: '0', parsed: 0, saysIt: true, note: 'kernel-assigned, and legal' },
115+
{ raw: '65535', parsed: 65535, saysIt: true, note: 'the ceiling itself' },
116+
{ raw: ' 3000', parsed: 3000, saysIt: true, note: 'production env vars carry whitespace' },
117+
{ raw: '3000 ', parsed: 3000, saysIt: true, note: 'and on the other side' },
118+
{ raw: '+3000', parsed: 3000, saysIt: true },
119+
{ raw: '08080', parsed: 8080, saysIt: true, note: 'no leading-zero octal since ES5' },
120+
{ raw: '3e3', parsed: 3, saysIt: false, note: "the card's own repro — binds 3, not 3000" },
121+
{ raw: '1e10', parsed: 1, saysIt: false },
122+
{ raw: '0x0BB8', parsed: 3000, saysIt: false, note: 'hex, and `Number()` agrees with parseInt here' },
123+
{ raw: '3000.0', parsed: 3000, saysIt: false },
124+
{ raw: '3000abc', parsed: 3000, saysIt: false, note: 'trailing text discarded' },
125+
{ raw: '0b111', parsed: 0, saysIt: false, note: 'binds 0 — a kernel-assigned port, from text saying 7' },
126+
{ raw: '0o17', parsed: 0, saysIt: false },
127+
{ raw: '1_000', parsed: 1, saysIt: false, note: 'separator: binds 1' },
128+
];
129+
130+
/** What #12662's refusal owns, and this notice must never reach. */
131+
const REFUSED = ['abc', '', ' ', '65536', '99999', '-1'];
132+
133+
describe('#12674: a port read as something other than what the text says is announced', () => {
134+
it('guards its own table first — every row is still what `parseInt` produces, and still ACCEPTED', () => {
135+
// Without this, a drift in `parseInt` (or in the validator) would leave
136+
// every verdict below measuring something else while still passing.
137+
for (const { raw, parsed, note } of ACCEPTED) {
138+
const label = `${JSON.stringify(raw)}${note ? ` (${note})` : ''}`;
139+
expect(parseInt(raw), `the table's parseInt record is stale for ${label}`).toBe(parsed);
140+
expect(
141+
parseRequestedPort(raw),
142+
`${label} is no longer accepted — this row belongs to the refusal, not here`,
143+
).toBe(parsed);
144+
}
145+
146+
// Anti-vacuity for every loop over the table: both verdicts must occur in
147+
// it, or a table that drifted to all-mismatch (or all-agree) would prove
148+
// nothing while staying green.
149+
expect(ACCEPTED.filter((row) => row.saysIt).length).toBeGreaterThan(1);
150+
expect(ACCEPTED.filter((row) => !row.saysIt).length).toBeGreaterThan(1);
151+
});
152+
153+
it('ARMS 1+2 — the notice fires on a difference and is SILENT on agreement, whole table', () => {
154+
for (const { raw, parsed, saysIt, note } of ACCEPTED) {
155+
const label = `${JSON.stringify(raw)}${note ? ` (${note})` : ''}`;
156+
const notice = portTextReadNotice(raw, '--port', parsed);
157+
158+
if (saysIt) {
159+
// THE NOISE PIN. `" 3000"` is the shape of an ordinary production
160+
// `PORT`; a notice here fires at every boot of every deployment whose
161+
// env var carries whitespace.
162+
expect(notice, `${label} reads as the port it selected, yet is announced`).toBeNull();
163+
} else {
164+
expect(notice, `${label} was read as ${parsed} in silence`).not.toBeNull();
165+
// Both facts, not one. The operator has neither: the bound port alone
166+
// is what the ready banner already prints, and the text alone is what
167+
// they typed.
168+
const shown = plain(notice as string);
169+
expect(shown, `${label}: the notice does not quote the text`).toContain(JSON.stringify(raw));
170+
expect(shown, `${label}: the notice does not name the port it selected`).toContain(
171+
`port ${parsed}`,
172+
);
173+
}
174+
}
175+
});
176+
177+
it('ARM 1 — names the input that was used, and not the others', () => {
178+
const cases: Array<{ source: PortInputSource; expected: string }> = [
179+
{ source: '--port', expected: '--port "3e3"' },
180+
{ source: 'PORT', expected: 'PORT="3e3"' },
181+
{ source: 'OS_PORT', expected: 'OS_PORT="3e3"' },
182+
];
183+
184+
for (const { source, expected } of cases) {
185+
const notice = plain(portTextReadNotice('3e3', source, 3) as string);
186+
expect(notice, `the notice does not name ${source}`).toContain(expected);
187+
expect(names(notice, expected), `the ${source} spelling is not matchable`).toBe(true);
188+
189+
// The discrimination, not merely the presence — a notice listing all
190+
// three would satisfy every assertion above while leaving the operator to
191+
// work out which one is theirs, which is the defect one level up.
192+
for (const other of cases) {
193+
if (other.source === source) continue;
194+
expect(
195+
names(notice, other.expected),
196+
`the notice for ${source} also names ${other.source}`,
197+
).toBe(false);
198+
}
199+
}
200+
});
201+
202+
it('ARM 1 — names the input the same way the refusal does, from one speller', () => {
203+
// Two notices, one input, one spelling. A second hand-written copy is free
204+
// to drift, and an operator who cannot recognise what they typed is the
205+
// defect both notices exist to fix.
206+
for (const source of ['--port', 'PORT', 'OS_PORT'] as PortInputSource[]) {
207+
const refusal = plain(formatInvalidPortNotice('3e3', source));
208+
const read = plain(portTextReadNotice('3e3', source, 3) as string);
209+
const spelled = source === '--port' ? '--port "3e3"' : `${source}="3e3"`;
210+
expect(refusal).toContain(spelled);
211+
expect(read, `the two notices spell ${source} differently`).toContain(spelled);
212+
}
213+
});
214+
215+
it('ARM 1 — states the port SELECTED, never a second reading of the text', () => {
216+
// `3e3` looks like 3000 to a reader; `0b111` looks like 7; `1_000` looks
217+
// like 1000. The notice reports what was selected and refuses to guess what
218+
// was meant — a guess is wrong the first time it meets text that has no
219+
// second reading, and `3000abc` is that text.
220+
const apparent = Number('0b111');
221+
expect(apparent, 'the arm is measuring the wrong thing').toBe(7);
222+
223+
const notice = plain(portTextReadNotice('0b111', '--port', 0) as string);
224+
expect(notice).toContain('port 0');
225+
expect(notice, 'the notice invented a value the operator might have meant').not.toContain(
226+
String(apparent),
227+
);
228+
});
229+
230+
it('ARM 3 — a value that cannot be a port belongs to the REFUSAL, and never reaches this notice', () => {
231+
for (const raw of REFUSED) {
232+
expect(
233+
parseRequestedPort(raw),
234+
`${JSON.stringify(raw)} is accepted, so the two paths now overlap`,
235+
).toBeNull();
236+
}
237+
238+
// …and structurally: the guard exits the process above this call site, so
239+
// no refused value can reach it. Anchors first — missing ones would leave
240+
// the ordering assertions comparing -1s and passing while measuring nothing.
241+
const refusal = SERVE_SOURCE.indexOf('printDiagnostic(formatInvalidPortNotice(flags.port, portSource));');
242+
const exit = SERVE_SOURCE.indexOf('this.exit(1);', refusal);
243+
const callSite = SERVE_SOURCE.indexOf(
244+
'const textReadNotice = portTextReadNotice(flags.port, portSource, requestedPort);',
245+
);
246+
const autoShift = SERVE_SOURCE.indexOf('if (portAutoShiftAllowed) {');
247+
248+
expect(refusal, "the refusal's call site is gone").toBeGreaterThan(-1);
249+
expect(exit, 'the refusal no longer exits').toBeGreaterThan(-1);
250+
expect(callSite, 'this notice has no call site').toBeGreaterThan(-1);
251+
expect(autoShift, 'the `portAutoShiftAllowed` branch head is gone').toBeGreaterThan(-1);
252+
253+
expect(exit, 'the notice can now be reached by a value that was refused').toBeLessThan(callSite);
254+
// And ahead of the port policy, so it states the port that was ASKED FOR
255+
// while #12543's drift notice states the one taken instead.
256+
expect(callSite).toBeLessThan(autoShift);
257+
});
258+
259+
it('prints only when there IS something to say — the call site is the `if`', () => {
260+
// ARM 2 returns `null`, and this is what makes that arm mean anything end
261+
// to end: an unconditional `printDiagnostic(portTextReadNotice(...))` would
262+
// write the string `null` at every boot.
263+
expect(SERVE_SOURCE).toContain('if (textReadNotice) printDiagnostic(textReadNotice);');
264+
});
265+
266+
it('writes through `printDiagnostic`, which is stderr (#7915 stdout purity)', () => {
267+
// `stdout` is the JSON-RPC channel whenever the stdio MCP transport is
268+
// mounted, which is what `serve-stdio-stdout-purity.e2e.test.ts` pins. A
269+
// notice written anywhere else reds that suite from this file.
270+
expect(SERVE_SOURCE, '`printDiagnostic` no longer writes to stderr').toMatch(
271+
/const printDiagnostic = \(text = ''\) => \{\s*\n\s*if \(!bootQuiet\) process\.stderr\.write/,
272+
);
273+
});
274+
275+
it('MUTUAL EXCLUSION — cannot be mistaken for the three notices it sits beside', () => {
276+
const notice = plain(portTextReadNotice('3e3', 'PORT', 3) as string);
277+
278+
/** #12543's drift notice. */
279+
const DRIFT_NOTICE = /Port (\d+) is in use serving on (\d+) instead\./;
280+
/** #11113's production refusal — and the first of `PORT_TAKEN_PATTERNS`. */
281+
const PRODUCTION_REFUSAL = /Port (\d+) is already in use/;
282+
/** #12662's refusal. */
283+
const INVALID_REFUSAL = /Invalid port:/;
284+
/** This one. */
285+
const READ_NOTICE = /was read as port (\d+)\./;
286+
287+
expect(notice, "the read notice reads as #12543's drift notice").not.toMatch(DRIFT_NOTICE);
288+
expect(notice, 'the read notice reads as the production refusal').not.toMatch(
289+
PRODUCTION_REFUSAL,
290+
);
291+
expect(notice, "the read notice reads as #12662's refusal — it refuses nothing").not.toMatch(
292+
INVALID_REFUSAL,
293+
);
294+
295+
// Load-bearing beyond legibility: `PORT_TAKEN_PATTERNS` in
296+
// `test/helpers/serve-process.ts` turns two of these into a "port
297+
// contention" verdict for every spawner in this package. A notice tripping
298+
// one would report a healthy boot as a lost port race.
299+
expect(notice, 'the read notice now trips the EADDRINUSE contention pattern').not.toMatch(
300+
/EADDRINUSE[^\n]*?:(\d+)/,
301+
);
302+
expect(notice, "the read notice claims a span it never walked (#12620's notice)").not.toMatch(
303+
/probed/,
304+
);
305+
306+
// …and the other way: the three siblings must not read as THIS one, or the
307+
// exclusion is only half measured.
308+
expect(plain(formatInvalidPortNotice('abc', 'PORT')), "#12662's refusal reads as this notice")
309+
.not.toMatch(READ_NOTICE);
310+
expect(' Port 3000 is in use — serving on 3001 instead.').not.toMatch(READ_NOTICE);
311+
expect(' Port 3000 is already in use.').not.toMatch(READ_NOTICE);
312+
313+
// …and every pattern above is a live instrument, not a dead regex: each
314+
// still matches the text it was written for, or the negatives prove nothing.
315+
expect(notice).toMatch(READ_NOTICE);
316+
expect(' Port 3000 is in use — serving on 3001 instead.').toMatch(DRIFT_NOTICE);
317+
expect(' Port 3000 is already in use.').toMatch(PRODUCTION_REFUSAL);
318+
expect(plain(formatInvalidPortNotice('abc', 'PORT'))).toMatch(INVALID_REFUSAL);
319+
});
320+
321+
it('draws the boundary on the TRIMMED text, and not with `Number()`', () => {
322+
// The two halves of the line, each stated as the thing that breaks if it
323+
// moves. Whitespace first: this is the shape production `PORT` values have.
324+
expect(strictPortReading(' 3000'), 'whitespace now counts as a difference').toBe(3000);
325+
expect(strictPortReading('3000 ')).toBe(3000);
326+
expect(strictPortReading('+3000')).toBe(3000);
327+
expect(strictPortReading('08080')).toBe(8080);
328+
329+
// …and the near-miss implementation, named because it is the one a reader
330+
// would reach for: `Number()` AGREES with `parseInt` on a hex literal, so a
331+
// boundary built on it would be blind to `0x0BB8` — one of the two
332+
// coercions this card exists to see.
333+
expect(Number('0x0BB8'), 'the near-miss is no longer a near-miss').toBe(parseInt('0x0BB8'));
334+
expect(strictPortReading('0x0BB8'), '`0x0BB8` now reads as a plain decimal').toBeNull();
335+
expect(portTextReadNotice('0x0BB8', '--port', 3000), 'the hex case went silent').not.toBeNull();
336+
337+
// …and the other direction, where `Number()` disagrees with `parseInt`:
338+
// the text a reader would call 3000, selecting port 3.
339+
expect(Number('3e3')).toBe(3000);
340+
expect(parseInt('3e3')).toBe(3);
341+
expect(strictPortReading('3e3')).toBeNull();
342+
});
343+
});

0 commit comments

Comments
 (0)