From 8d6f70499f9fbbcfb2f852d7d4796b4e7952d312 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 27 Aug 2026 09:01:33 +0000 Subject: [PATCH] feat(cli): say so when a port is read as something other than what the text says (#12674) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `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. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01UjujZN219uFzBhSYfMykCd --- .changeset/cli-serve-port-text-read-notice.md | 46 +++ .../serve-port-text-read-notice.test.ts | 343 ++++++++++++++++++ packages/cli/src/commands/serve.ts | 192 +++++++++- 3 files changed, 571 insertions(+), 10 deletions(-) create mode 100644 .changeset/cli-serve-port-text-read-notice.md create mode 100644 packages/cli/src/commands/serve-port-text-read-notice.test.ts diff --git a/.changeset/cli-serve-port-text-read-notice.md b/.changeset/cli-serve-port-text-read-notice.md new file mode 100644 index 0000000000..15e72462c4 --- /dev/null +++ b/.changeset/cli-serve-port-text-read-notice.md @@ -0,0 +1,46 @@ +--- +"@objectstack/cli": minor +--- + +feat(cli): `os serve` says so when a port is read as something other than what the text says (#12674) + +`os serve` reads its port with `parseInt`, and `parseInt` is tolerant in a way +that 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 anywhere says so — an +operator who wrote `PORT=3e3` meaning 3000 gets a server on port 3, and on a +non-root host that surfaces (much later, if at all) as an `EACCES` that still +does not name the coercion. + +The value is now announced when it does not read as the port it selected: + +``` + ⚠ PORT="3e3" was read as port 3. + That text is not a plain decimal number, and the reader that accepts it + is tolerant: it honours a leading 0x as hexadecimal and discards + everything from the first character that cannot continue the number. + Nothing downstream reads it again — 3 is the port this server asked + for, whatever the text looks like. + If that is not the port you meant, correct PORT in this process's + environment (for example PORT=3000), or override it with --port 3000. +``` + +**Nothing is refused, and nothing binds differently.** The accept set is exactly +what it was: every spelling that boots today still boots, on the same port, byte +for byte. Whether `os serve` should take only strict decimal text is a contract +question about a published CLI's accepted input, and it is deliberately left +open. This repairs the silence, which is where the harm actually was. + +The notice fires on a *difference*, so what counts as agreement is the whole of +it: leading and trailing whitespace, a leading `+` and leading zeros do not +change what the text says (`" 3000"`, `"+3000"` and `"08080"` are silent — the +first is what production `PORT` values look like, and a notice there would drone +at every ordinary boot). An exponent, a radix prefix, a fraction, a digit +separator or trailing text all do (`"3e3"`, `"1e10"`, `"0x0BB8"`, `"0b111"`, +`"3000.0"`, `"1_000"`, `"3000abc"` all speak). + +It names both the text and the port it selected — never a third number, because +`3e3` looks like 3000 to a reader but `3000abc` has no second reading and a +guess would be wrong the first time it met one. Written to **stderr** like every +other `os serve` diagnostic: `stdout` carries JSON-RPC frames whenever the stdio +MCP transport is mounted. diff --git a/packages/cli/src/commands/serve-port-text-read-notice.test.ts b/packages/cli/src/commands/serve-port-text-read-notice.test.ts new file mode 100644 index 0000000000..cc02fdeb10 --- /dev/null +++ b/packages/cli/src/commands/serve-port-text-read-notice.test.ts @@ -0,0 +1,343 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * #12674 — when `os serve` reads a port as something other than what the text + * says, it SAYS SO. The accept set is untouched. + * + * ## The defect + * + * `parseInt` is the reader, and #12662's ruling keeps it: no value that boots + * today may be refused, because narrowing a published CLI's accepted input is a + * contract decision. But `parseInt`'s tolerance changes the ANSWER, not just + * the spelling. `os serve --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 anywhere says so. + * + * ⭐ The harm is not that a strange value is accepted — it is a server + * listening somewhere nobody asked for. That is what the notice repairs, and it + * repairs only that: behaviour is unchanged, byte for byte. + * + * ## Why this file binds no sockets + * + * The decision is made before any socket exists, so a test that needs one is + * testing the wrong layer — the ruling `serve-port-validation.test.ts` and + * `serve-exhausted-port-search-notice.test.ts` both record, for the measured + * reason (#12441: real-port contention took a full CLI suite red in this shared + * container). Everything below drives exported pure functions and the live + * source text. Zero sockets, zero spawns. + * + * ## THE THREE-WAY DISCRIMINATION (the anti-vacuity requirement) + * + * A pin asserting only "a mismatch prints a notice" is just as green against an + * implementation that prints unconditionally. Three arms, each decidable at + * runtime at the same seam: + * + * 1. **mismatch** (`3e3` → 3, `0x0BB8` → 3000) → a notice naming BOTH the text + * and the port it selected. + * 2. **agreement** (`3000`, `" 3000"`, `"+3000"`, `"08080"`) → `null`. ⭐ This + * is the arm the whole card turns on: `" 3000"` is what production `PORT` + * values look like, and a notice there would drone at every ordinary boot. + * 3. **not a port at all** (`abc`, `99999`) → #12662's REFUSAL owns it, and + * this notice is unreachable — the guard exits the process above the call + * site. Pinned both ways: `parseRequestedPort` returns `null` for those + * values, and the call site is lexically downstream of that exit. + * + * The three must not overlap or swallow one another, so the mutual-exclusion + * arm covers all four port notices this command now carries — one pair of which + * is load-bearing beyond legibility: `PORT_TAKEN_PATTERNS` in + * `test/helpers/serve-process.ts` turns two of them into a "port contention" + * verdict for every spawner in this package. + */ + +import { describe, it, expect } from 'vitest'; +import { readFileSync } from 'node:fs'; +import { resolve } from 'node:path'; +import { fileURLToPath } from 'node:url'; + +import { + parseRequestedPort, + strictPortReading, + portTextReadNotice, + formatInvalidPortNotice, + type PortInputSource, +} from './serve.js'; + +/** Seeded from `import.meta.url`, the spelling `check:cross-package-test-inputs` recognises. */ +const HERE = resolve(fileURLToPath(import.meta.url), '..'); + +/** The live source, for the arms decided lexically rather than at runtime. */ +const SERVE_SOURCE = readFileSync(resolve(HERE, 'serve.ts'), 'utf8'); + +/** + * Strip SGR escapes — `chalk` is inert under a non-TTY runner but not + * guaranteed to be, and an assertion that only passes with colour off is a + * flake waiting for the first person who runs this attached. The ESC byte is + * built with `String.fromCharCode` because this repo refuses raw control bytes + * in source (`check:nul-bytes`). + */ +const SGR = new RegExp(`${String.fromCharCode(27)}\\[[0-9;]*m`, 'g'); +const plain = (text: string): string => text.replace(SGR, ''); + +/** + * Does `notice` name this exact source spelling? + * + * ⚠️ The anchor is WIDENED rather than the assertion loosened, for the trap + * `serve-port-validation.test.ts` measured on its own arm: `OS_PORT="3e3"` + * CONTAINS `PORT="3e3"`, so a plain `toContain`/`not.toContain` pair reports + * the OS_PORT notice as also naming PORT. A preceding `_` disqualifies the + * match; a message that really did name both would still carry a `PORT=` with + * no `_` in front of it and would still be caught. + */ +const names = (notice: string, spelled: string): boolean => + new RegExp(`(? = [ + { raw: '3000', parsed: 3000, saysIt: true }, + { raw: '0', parsed: 0, saysIt: true, note: 'kernel-assigned, and legal' }, + { raw: '65535', parsed: 65535, saysIt: true, note: 'the ceiling itself' }, + { raw: ' 3000', parsed: 3000, saysIt: true, note: 'production env vars carry whitespace' }, + { raw: '3000 ', parsed: 3000, saysIt: true, note: 'and on the other side' }, + { raw: '+3000', parsed: 3000, saysIt: true }, + { raw: '08080', parsed: 8080, saysIt: true, note: 'no leading-zero octal since ES5' }, + { raw: '3e3', parsed: 3, saysIt: false, note: "the card's own repro — binds 3, not 3000" }, + { raw: '1e10', parsed: 1, saysIt: false }, + { raw: '0x0BB8', parsed: 3000, saysIt: false, note: 'hex, and `Number()` agrees with parseInt here' }, + { raw: '3000.0', parsed: 3000, saysIt: false }, + { raw: '3000abc', parsed: 3000, saysIt: false, note: 'trailing text discarded' }, + { raw: '0b111', parsed: 0, saysIt: false, note: 'binds 0 — a kernel-assigned port, from text saying 7' }, + { raw: '0o17', parsed: 0, saysIt: false }, + { raw: '1_000', parsed: 1, saysIt: false, note: 'separator: binds 1' }, +]; + +/** What #12662's refusal owns, and this notice must never reach. */ +const REFUSED = ['abc', '', ' ', '65536', '99999', '-1']; + +describe('#12674: a port read as something other than what the text says is announced', () => { + it('guards its own table first — every row is still what `parseInt` produces, and still ACCEPTED', () => { + // Without this, a drift in `parseInt` (or in the validator) would leave + // every verdict below measuring something else while still passing. + for (const { raw, parsed, note } of ACCEPTED) { + const label = `${JSON.stringify(raw)}${note ? ` (${note})` : ''}`; + expect(parseInt(raw), `the table's parseInt record is stale for ${label}`).toBe(parsed); + expect( + parseRequestedPort(raw), + `${label} is no longer accepted — this row belongs to the refusal, not here`, + ).toBe(parsed); + } + + // Anti-vacuity for every loop over the table: both verdicts must occur in + // it, or a table that drifted to all-mismatch (or all-agree) would prove + // nothing while staying green. + expect(ACCEPTED.filter((row) => row.saysIt).length).toBeGreaterThan(1); + expect(ACCEPTED.filter((row) => !row.saysIt).length).toBeGreaterThan(1); + }); + + it('ARMS 1+2 — the notice fires on a difference and is SILENT on agreement, whole table', () => { + for (const { raw, parsed, saysIt, note } of ACCEPTED) { + const label = `${JSON.stringify(raw)}${note ? ` (${note})` : ''}`; + const notice = portTextReadNotice(raw, '--port', parsed); + + if (saysIt) { + // THE NOISE PIN. `" 3000"` is the shape of an ordinary production + // `PORT`; a notice here fires at every boot of every deployment whose + // env var carries whitespace. + expect(notice, `${label} reads as the port it selected, yet is announced`).toBeNull(); + } else { + expect(notice, `${label} was read as ${parsed} in silence`).not.toBeNull(); + // Both facts, not one. The operator has neither: the bound port alone + // is what the ready banner already prints, and the text alone is what + // they typed. + const shown = plain(notice as string); + expect(shown, `${label}: the notice does not quote the text`).toContain(JSON.stringify(raw)); + expect(shown, `${label}: the notice does not name the port it selected`).toContain( + `port ${parsed}`, + ); + } + } + }); + + it('ARM 1 — names the input that was used, and not the others', () => { + const cases: Array<{ source: PortInputSource; expected: string }> = [ + { source: '--port', expected: '--port "3e3"' }, + { source: 'PORT', expected: 'PORT="3e3"' }, + { source: 'OS_PORT', expected: 'OS_PORT="3e3"' }, + ]; + + for (const { source, expected } of cases) { + const notice = plain(portTextReadNotice('3e3', source, 3) as string); + expect(notice, `the notice does not name ${source}`).toContain(expected); + expect(names(notice, expected), `the ${source} spelling is not matchable`).toBe(true); + + // The discrimination, not merely the presence — a notice listing all + // three would satisfy every assertion above while leaving the operator to + // work out which one is theirs, which is the defect one level up. + for (const other of cases) { + if (other.source === source) continue; + expect( + names(notice, other.expected), + `the notice for ${source} also names ${other.source}`, + ).toBe(false); + } + } + }); + + it('ARM 1 — names the input the same way the refusal does, from one speller', () => { + // Two notices, one input, one spelling. A second hand-written copy is free + // to drift, and an operator who cannot recognise what they typed is the + // defect both notices exist to fix. + for (const source of ['--port', 'PORT', 'OS_PORT'] as PortInputSource[]) { + const refusal = plain(formatInvalidPortNotice('3e3', source)); + const read = plain(portTextReadNotice('3e3', source, 3) as string); + const spelled = source === '--port' ? '--port "3e3"' : `${source}="3e3"`; + expect(refusal).toContain(spelled); + expect(read, `the two notices spell ${source} differently`).toContain(spelled); + } + }); + + it('ARM 1 — states the port SELECTED, never a second reading of the text', () => { + // `3e3` looks like 3000 to a reader; `0b111` looks like 7; `1_000` looks + // like 1000. The notice reports what was selected and refuses to guess what + // was meant — a guess is wrong the first time it meets text that has no + // second reading, and `3000abc` is that text. + const apparent = Number('0b111'); + expect(apparent, 'the arm is measuring the wrong thing').toBe(7); + + const notice = plain(portTextReadNotice('0b111', '--port', 0) as string); + expect(notice).toContain('port 0'); + expect(notice, 'the notice invented a value the operator might have meant').not.toContain( + String(apparent), + ); + }); + + it('ARM 3 — a value that cannot be a port belongs to the REFUSAL, and never reaches this notice', () => { + for (const raw of REFUSED) { + expect( + parseRequestedPort(raw), + `${JSON.stringify(raw)} is accepted, so the two paths now overlap`, + ).toBeNull(); + } + + // …and structurally: the guard exits the process above this call site, so + // no refused value can reach it. Anchors first — missing ones would leave + // the ordering assertions comparing -1s and passing while measuring nothing. + const refusal = SERVE_SOURCE.indexOf('printDiagnostic(formatInvalidPortNotice(flags.port, portSource));'); + const exit = SERVE_SOURCE.indexOf('this.exit(1);', refusal); + const callSite = SERVE_SOURCE.indexOf( + 'const textReadNotice = portTextReadNotice(flags.port, portSource, requestedPort);', + ); + const autoShift = SERVE_SOURCE.indexOf('if (portAutoShiftAllowed) {'); + + expect(refusal, "the refusal's call site is gone").toBeGreaterThan(-1); + expect(exit, 'the refusal no longer exits').toBeGreaterThan(-1); + expect(callSite, 'this notice has no call site').toBeGreaterThan(-1); + expect(autoShift, 'the `portAutoShiftAllowed` branch head is gone').toBeGreaterThan(-1); + + expect(exit, 'the notice can now be reached by a value that was refused').toBeLessThan(callSite); + // And ahead of the port policy, so it states the port that was ASKED FOR + // while #12543's drift notice states the one taken instead. + expect(callSite).toBeLessThan(autoShift); + }); + + it('prints only when there IS something to say — the call site is the `if`', () => { + // ARM 2 returns `null`, and this is what makes that arm mean anything end + // to end: an unconditional `printDiagnostic(portTextReadNotice(...))` would + // write the string `null` at every boot. + expect(SERVE_SOURCE).toContain('if (textReadNotice) printDiagnostic(textReadNotice);'); + }); + + it('writes through `printDiagnostic`, which is stderr (#7915 stdout purity)', () => { + // `stdout` is the JSON-RPC channel whenever the stdio MCP transport is + // mounted, which is what `serve-stdio-stdout-purity.e2e.test.ts` pins. A + // notice written anywhere else reds that suite from this file. + expect(SERVE_SOURCE, '`printDiagnostic` no longer writes to stderr').toMatch( + /const printDiagnostic = \(text = ''\) => \{\s*\n\s*if \(!bootQuiet\) process\.stderr\.write/, + ); + }); + + it('MUTUAL EXCLUSION — cannot be mistaken for the three notices it sits beside', () => { + const notice = plain(portTextReadNotice('3e3', 'PORT', 3) as string); + + /** #12543's drift notice. */ + const DRIFT_NOTICE = /Port (\d+) is in use — serving on (\d+) instead\./; + /** #11113's production refusal — and the first of `PORT_TAKEN_PATTERNS`. */ + const PRODUCTION_REFUSAL = /Port (\d+) is already in use/; + /** #12662's refusal. */ + const INVALID_REFUSAL = /Invalid port:/; + /** This one. */ + const READ_NOTICE = /was read as port (\d+)\./; + + expect(notice, "the read notice reads as #12543's drift notice").not.toMatch(DRIFT_NOTICE); + expect(notice, 'the read notice reads as the production refusal').not.toMatch( + PRODUCTION_REFUSAL, + ); + expect(notice, "the read notice reads as #12662's refusal — it refuses nothing").not.toMatch( + INVALID_REFUSAL, + ); + + // Load-bearing beyond legibility: `PORT_TAKEN_PATTERNS` in + // `test/helpers/serve-process.ts` turns two of these into a "port + // contention" verdict for every spawner in this package. A notice tripping + // one would report a healthy boot as a lost port race. + expect(notice, 'the read notice now trips the EADDRINUSE contention pattern').not.toMatch( + /EADDRINUSE[^\n]*?:(\d+)/, + ); + expect(notice, "the read notice claims a span it never walked (#12620's notice)").not.toMatch( + /probed/, + ); + + // …and the other way: the three siblings must not read as THIS one, or the + // exclusion is only half measured. + expect(plain(formatInvalidPortNotice('abc', 'PORT')), "#12662's refusal reads as this notice") + .not.toMatch(READ_NOTICE); + expect(' Port 3000 is in use — serving on 3001 instead.').not.toMatch(READ_NOTICE); + expect(' Port 3000 is already in use.').not.toMatch(READ_NOTICE); + + // …and every pattern above is a live instrument, not a dead regex: each + // still matches the text it was written for, or the negatives prove nothing. + expect(notice).toMatch(READ_NOTICE); + expect(' Port 3000 is in use — serving on 3001 instead.').toMatch(DRIFT_NOTICE); + expect(' Port 3000 is already in use.').toMatch(PRODUCTION_REFUSAL); + expect(plain(formatInvalidPortNotice('abc', 'PORT'))).toMatch(INVALID_REFUSAL); + }); + + it('draws the boundary on the TRIMMED text, and not with `Number()`', () => { + // The two halves of the line, each stated as the thing that breaks if it + // moves. Whitespace first: this is the shape production `PORT` values have. + expect(strictPortReading(' 3000'), 'whitespace now counts as a difference').toBe(3000); + expect(strictPortReading('3000 ')).toBe(3000); + expect(strictPortReading('+3000')).toBe(3000); + expect(strictPortReading('08080')).toBe(8080); + + // …and the near-miss implementation, named because it is the one a reader + // would reach for: `Number()` AGREES with `parseInt` on a hex literal, so a + // boundary built on it would be blind to `0x0BB8` — one of the two + // coercions this card exists to see. + expect(Number('0x0BB8'), 'the near-miss is no longer a near-miss').toBe(parseInt('0x0BB8')); + expect(strictPortReading('0x0BB8'), '`0x0BB8` now reads as a plain decimal').toBeNull(); + expect(portTextReadNotice('0x0BB8', '--port', 3000), 'the hex case went silent').not.toBeNull(); + + // …and the other direction, where `Number()` disagrees with `parseInt`: + // the text a reader would call 3000, selecting port 3. + expect(Number('3e3')).toBe(3000); + expect(parseInt('3e3')).toBe(3); + expect(strictPortReading('3e3')).toBeNull(); + }); +}); diff --git a/packages/cli/src/commands/serve.ts b/packages/cli/src/commands/serve.ts index fcd9489562..3aef836359 100644 --- a/packages/cli/src/commands/serve.ts +++ b/packages/cli/src/commands/serve.ts @@ -463,6 +463,32 @@ export function parseRequestedPort(raw: string): number | null { return parsed; } +/** + * Spell a port input the way the operator set it: `--port "3e3"`, `PORT="3e3"`, + * `OS_PORT="3e3"`, or `the built-in default ("3000")`. + * + * ⭐ ONE spelling of one fact. Both notices that name the input read it from + * here — {@link formatInvalidPortNotice} (#12662) and + * {@link portTextReadNotice} (#12674) — which is the rule + * {@link PORT_SEARCH_SPAN}'s docblock established for numbers, applied to + * prose. Two hand-written copies of "how this input is written back" are two + * things free to drift, and an operator who cannot recognise what they typed is + * the defect both notices exist to fix. + * + * ⚠️ `JSON.stringify` is not decoration. It makes `" 3000"` distinguishable + * from `"3000"` on the screen — whitespace is the likeliest thing an operator + * is staring at without seeing — and it escapes control bytes instead of + * writing them to a terminal. + */ +function spellPortInput(raw: string, source: PortInputSource): string { + const shown = JSON.stringify(raw); + return source === '--port' + ? `--port ${shown}` + : source === 'the built-in default' + ? `the built-in default (${shown})` + : `${source}=${shown}`; +} + /** * The refusal for a port value that cannot be one (#12662). * @@ -475,10 +501,11 @@ export function parseRequestedPort(raw: string): number | null { * a second, hand-written copy of those numbers: this sentence exists to be * accurate about the bounds the code enforces, so it interpolates them. * - * ⚠️ The raw text is rendered with `JSON.stringify`, which is not decoration. - * It makes `" 3000"` distinguishable from `"3000"` on the screen — the - * whitespace case is the most likely thing an operator is staring at without - * seeing — and it escapes control bytes rather than writing them to a terminal. + * ⚠️ The source spelling is {@link spellPortInput}'s, shared with #12674's + * read notice so that one input is named one way — `JSON.stringify` rendering + * included, which is not decoration: it makes `" 3000"` distinguishable from + * `"3000"` on the screen and escapes control bytes rather than writing them to + * a terminal. * * ⚠️ KNOWN LIMIT, stated rather than papered over: `os dev` forwards its own * `--port` (and `$PORT`, promoted to a flag) to the `serve` child on argv, @@ -493,12 +520,7 @@ export function parseRequestedPort(raw: string): number | null { * client as a transport error. */ export function formatInvalidPortNotice(raw: string, source: PortInputSource): string { - const shown = JSON.stringify(raw); - const spelled = source === '--port' - ? `--port ${shown}` - : source === 'the built-in default' - ? `the built-in default (${shown})` - : `${source}=${shown}`; + const spelled = spellPortInput(raw, source); const fix = source === '--port' || source === 'the built-in default' ? ' Pass a whole number instead, for example --port 3000.' : ` Correct ${source} in this process's environment (for example ${source}=3000),\n` @@ -514,6 +536,137 @@ export function formatInvalidPortNotice(raw: string, source: PortInputSource): s ); } +/** + * The port the TEXT says, on a strict reading — or `null` when the text does + * not say a port at all. + * + * ## This boundary IS the card (#12674) + * + * {@link parseRequestedPort} keeps `parseInt`, deliberately: #12662's ruling is + * that no value which boots today may be refused, and a tightening would narrow + * a published CLI's accepted input. But `parseInt`'s tolerance changes the + * ANSWER, not merely the spelling — `--port 3e3` binds port **3**, `--port + * 0x0BB8` binds 3000, `--port 3000abc` binds 3000. The server comes up on a + * port the operator never named and nothing says so. ⭐ That silence, not the + * tolerance, is what is repaired here. + * + * So the notice fires on a DIFFERENCE, which makes the definition of "the same" + * the whole precision of this card. MEASURED on this checkout (Node v22.22.2): + * + * ``` + * " 3000" → 3000 "+3000" → 3000 "08080" → 8080 ← says what it selected + * "3000 " → 3000 "3000" → 3000 + * "3e3" → 3 "0x0BB8" → 3000 "3000.0" → 3000 ← does NOT + * "1e10" → 1 "0b111" → 0 "3000abc" → 3000 + * ``` + * + * - **Whitespace is not a difference.** `" 3000"` reads as 3000 to a human and + * to `parseInt` alike, and production `PORT` values carry whitespace. A + * boundary that counted it would drone a notice on every boot of the most + * ordinary deployment there is — noise on the one input shape that is both + * common and harmless. ⭐ This half of the line is why it is drawn on the + * TRIMMED text. + * - **A leading `+` is not a difference**, and **leading zeros are not**: + * `"+3000"` says 3000, `"08080"` says 8080 (`parseInt` has read no leading + * zero as octal since ES5 — measured above, not assumed). + * - **Everything else IS**, because it means the port was not read off the + * digits: an exponent, a radix prefix, a fraction, a separator, or trailing + * text. + * + * ⛔ Nothing here refuses anything. The accept set stays EXACTLY + * {@link parseRequestedPort}'s. Whether the CLI should take only strict decimal + * text is a contract question, left open on purpose (#12674), and #12673 is + * blocked on the same one. + * + * ⛔ Not `Number()`, which is the near-miss worth naming: it AGREES with + * `parseInt` on `"0x0BB8"` (both 3000, measured), so a boundary built on it + * would be blind to a hex literal — one of the two coercions this exists to + * see. It disagrees on `"3e3"` (3000 vs 3), which is the other one. + */ +export function strictPortReading(raw: string): number | null { + const trimmed = raw.trim(); + if (!/^[+-]?\d+$/.test(trimmed)) return null; + const value = Number(trimmed); + return Number.isInteger(value) ? value : null; +} + +/** + * The notice for a port that was read as something other than what it says + * (#12674) — or `null` when the text and the port agree. + * + * ## ⭐ The DECISION lives in here, not at the call site + * + * A notice printed unconditionally satisfies every assertion that only checks + * what a mismatch prints. Returning `null` for the agreeing case puts that arm + * where a test can drive it directly, at the same seam, with no source at all: + * `portTextReadNotice(' 3000', 'PORT', 3000)` is `null` or this is broken. The + * call site is then one `if`, and the three-way split it completes is: + * mismatch → this notice; agreement → nothing; a value that cannot be a port + * at all → {@link formatInvalidPortNotice}'s refusal, which exits before this + * function is ever reached. + * + * ## What it states — and the second reading it must NOT invent + * + * Two facts, both of which the operator lacks: the text they set, and the port + * it selected. ⛔ Never a third — the number they MEANT. `3e3` looks like 3000 + * to a reader, but `3000abc` and `0b111` have no such second reading, and a + * diagnostic that guessed would be wrong the first time it met one. It reports + * that the text does not say the selected port, and stops there. + * + * ⚠️ It says "asked for", not "bound": in development the auto-shift below + * may still move off this port, and #12543's drift notice — which prints just + * after this one — owns that fact. Two notices, two facts, neither restating + * the other. + * + * ## ⚠️ KNOWN LIMIT — the source it names on a spawn (#12673) + * + * MEASURED, and stated rather than papered over. `os dev` reads + * `flags.port ?? readEnvWithDeprecation('OS_PORT', 'PORT')` and forwards the + * result to the `serve` child as `--port ` on argv (`commands/dev.ts`). + * So `PORT=3e3 os dev` reaches this function as `--port "3e3"`, and the notice + * names the channel the value ARRIVED on rather than the one the operator set. + * The text and the port — the two facts above — are still exactly right; only + * the source label is the child's rather than the parent's. ⛔ It is NOT fixed + * here: `os dev`'s own port handling is #12673, which is blocked on the same + * accept-set question this card was scoped away from. + * + * `os start` is measured too and does not have the defect on this path: its + * `--port` is a `Flags.integer` (parser `/^-?\d+$/` in `@oclif/core` 4.13.3), + * so a spelling this notice would fire on is refused by the parent before any + * spawn, and an env-supplied value reaches the child through the inherited + * environment under its own name. + * + * CHANNEL — the same `printDiagnostic` (stderr) as its three siblings, for the + * reason #7915 measured: `stdout` carries JSON-RPC frames whenever the stdio + * MCP transport is mounted, where one non-frame line reaches a conforming + * client as a transport error. `serve-stdio-stdout-purity.e2e.test.ts` pins it. + */ +export function portTextReadNotice( + raw: string, + source: PortInputSource, + port: number, +): string | null { + if (strictPortReading(raw) === port) return null; + + const spelled = spellPortInput(raw, source); + const fix = source === '--port' || source === 'the built-in default' + ? ' If that is not the port you meant, write it as a plain decimal number\n' + + ' (for example --port 3000).' + : ` If that is not the port you meant, correct ${source} in this process's\n` + + ` environment (for example ${source}=3000), or override it with --port 3000.`; + + return ( + '\n' + + chalk.yellow(` ⚠ ${spelled} was read as port ${port}.\n`) + + chalk.dim(' That text is not a plain decimal number, and the reader that accepts it\n') + + chalk.dim(' is tolerant: it honours a leading 0x as hexadecimal and discards\n') + + chalk.dim(' everything from the first character that cannot continue the number.\n') + + chalk.dim(` Nothing downstream reads it again — ${port} is the port this server asked\n`) + + chalk.dim(' for, whatever the text looks like.\n') + + chalk.dim(fix) + ); +} + /** * The IDENTITIES a capability provider registers under: full `plugin.name` ids * (`com.objectstack.mcp`) and/or exported class names (`MCPServerPlugin`). @@ -1702,6 +1855,25 @@ export default class Serve extends Command { this.exit(1); } const requestedPort = parsedPort; + + // ── …and it has to BE the port the text SAYS, or say otherwise (#12674) ── + // `parseInt` is kept as the reader (#12662's ruling: nothing that boots + // today may be refused), and its tolerance changes the answer rather than + // the spelling — `--port 3e3` selects port 3, `--port 0x0BB8` selects 3000. + // The boot then succeeds on a port nobody named, which is the harm: not a + // strange value being accepted, but a server listening somewhere the + // operator never asked for and nothing saying so. + // + // ⭐ Placement completes the three-way split rather than adding a fourth + // path. A value that cannot be a port has already exited above, so the + // refusal and this notice can never both fire; and this sits AHEAD of + // `portAutoShiftAllowed`, so it states the port that was ASKED FOR while + // #12543's drift notice, a few lines down, states the one that was taken + // instead. It also runs before the boot-quiet window opens, so — like its + // siblings — it survives a boot that dies later. + const textReadNotice = portTextReadNotice(flags.port, portSource, requestedPort); + if (textReadNotice) printDiagnostic(textReadNotice); + let port = requestedPort; // Port-conflict policy differs by mode: //