diff --git a/packages/cli/test/helpers/serve-process.ts b/packages/cli/test/helpers/serve-process.ts index 765605d110..35e0e2aab3 100644 --- a/packages/cli/test/helpers/serve-process.ts +++ b/packages/cli/test/helpers/serve-process.ts @@ -10,7 +10,7 @@ * rather than re-implements it. */ -import { spawn } from 'node:child_process'; +import { execFileSync, spawn } from 'node:child_process'; import { resolve } from 'node:path'; import { fileURLToPath } from 'node:url'; @@ -20,9 +20,208 @@ const HERE = resolve(fileURLToPath(import.meta.url), '..'); export const CLI = resolve(HERE, '../../bin/run-dev.js'); export const TSX = resolve(HERE, '../../../../node_modules/.bin/tsx'); -/** A random high port, so a run never contends with a dev server on this host. */ +/** + * The bind probe, run in a throwaway Node process: bind `0.0.0.0:`, print + * the port the kernel actually assigned, close. `want = 0` asks the kernel to + * choose. Returns `null` when the bind failed — which for a specific `want` + * means "that port is taken", and for `0` means the probe itself malfunctioned. + * + * ## Why a SUBPROCESS rather than an in-process `net.createServer()` + * + * `net.Server#listen()` reports its assigned port ASYNCHRONOUSLY. Measured on + * this container (node 22.22.2): `server.address()` is `null` on the very next + * line after `listen(0, '0.0.0.0')`, so an in-process probe can only be + * `async`. `randomPort()` below is called from ~14 sites across 8 other files + * in this directory, every one of them passing it straight into a `spawn()` + * argument list; turning it async would edit all of them for no behavioural + * gain. A `node -e` child does the same bind and is synchronous from this + * process's point of view. + * + * The price, measured on the container this suite runs in: ~38 ms per draw + * (5-draw mean, empty child env — an inherited environment measured 74.7 ms, + * so the strip below roughly halves it too). Against this package's own + * measured per-spawn floor — 2.9 s for `node bin/run.js --version`, 6.5 s for + * the tsx source entry — one draw is ~1.3% of the cheapest thing it precedes, + * and ~14 draws are ~0.5 s against a suite whose wall was 495.8 s when + * `vitest.config.ts` last measured it (~0.1%). + * + * ⛔ The probe child gets an environment built from NOTHING — not + * `{ ...process.env, … }`, not even `childEnv()`. It is a bare `net` bind that + * reads no variable at all, so the whole runner environment is surplus, and a + * bulk copy into a spawn is the class `pnpm check:cli-test-child-env` closes in + * this directory (a spread here is a real finding under that gate, not a false + * positive — measured: it takes this file from 0 to 1 over its ceiling). Not + * inheriting `NODE_OPTIONS` is the concrete win: an `--import` hook meant for + * the suite would otherwise load inside every port draw. + */ +function probeBind(want: number): number | null { + const src = [ + "const net = require('node:net');", + 'const want = Number(process.argv[1] || 0);', + 'const s = net.createServer();', + "s.on('error', () => process.exit(3));", + "s.listen(want, '0.0.0.0', () => {", + ' const p = s.address().port;', + ' s.close(() => process.stdout.write(String(p)));', + '});', + ].join('\n'); + try { + const out = execFileSync(process.execPath, ['-e', src, String(want)], { + encoding: 'utf8', + stdio: ['ignore', 'pipe', 'pipe'], + timeout: 30_000, + env: {}, + }); + const port = Number(out.trim()); + return Number.isInteger(port) && port > 0 ? port : null; + } catch { + return null; + } +} + +/** + * The ONE port draw for every e2e spawn in this directory — a real bind probe, + * not a blind `Math.random()` (#12441). + * + * Listen on `0.0.0.0:0`, read the port the kernel assigned, close the listener, + * return the port for the caller to hand to `os serve`. + * + * ## ⚠️ What this guarantees, and what it does NOT — both halves, deliberately + * + * It is **still TOCTOU**. The listener is closed before `serve` binds, so the + * port is unheld across the close-to-spawn gap and this can still lose a race. + * What it actually buys: + * + * • It never draws a port that is **already held**. A blind draw picks a + * number out of a range without asking anyone, so a neighbouring agent's + * dev server — bound for that process's entire lifetime, minutes or hours — + * is a live target on every single draw. The kernel does not assign a port + * that is currently bound, so that whole population is off the table. + * • It narrows the window from "the whole run" to "one close-to-spawn gap" + * (milliseconds). Only something that binds *inside* that gap can take it. + * • It removes this directory's second collision source. There used to be + * three independent draws over two overlapping ranges (41000-60000 here, + * 40000-60000 in `serve-app-anchored-optional-import.e2e.test.ts`), which + * could collide with EACH OTHER under `--maxWorkers > 1`, not only with a + * neighbour. There is one draw now and it asks the kernel. + * + * ⛔ The comment this replaced claimed "a run never contends with another + * agent's dev server on this host". That unqualified negative is the reason + * nobody re-examined the draw until a real run in this fleet went red on + * `✗ Port 49402 is already in use`. **Do not write another one here.** The + * residual race is real and unclosed; what pays for it is + * `portContentionError()` below, which makes the residual failure SAY it is a + * port race instead of `serve exited 1 before "Server is ready"`. + * + * ⚠️ One property that is worse than the old range and is stated rather than + * hidden: the kernel assigns from its ephemeral range + * (`/proc/sys/net/ipv4/ip_local_port_range`, 32768-60999 on this container), + * which is also where it draws source ports for OUTBOUND connections. The old + * 40000-60000 range overlapped that anyway, and the probe's win — never + * handing out a port some listener already holds — is the larger term. Nothing + * here makes the port immune once it is handed over. + */ +export function reservePort(): number { + // Retried, because a `null` for `want = 0` is a malfunctioning probe (the + // kernel cannot answer "busy" to a request for any free port), and turning a + // transient subprocess hiccup into a hard failure would replace one flake + // class with another. + for (let attempt = 0; attempt < 3; attempt++) { + const port = probeBind(0); + if (port !== null) return port; + } + throw new Error( + 'bind probe failed: could not obtain a free TCP port from the kernel after 3 attempts ' + + '(listen on 0.0.0.0:0 in a `node -e` child). This is a host problem, not a verdict ' + + 'about the code under test.', + ); +} + +/** + * Is `port` bindable RIGHT NOW? The **negative arm** of the same probe. + * + * Exported so `serve-port-bind-probe.test.ts` can prove the instrument is able + * to answer NO: a probe that reports "free" for a port the test is holding open + * is not an instrument, and every claim `reservePort()` makes rests on this + * being a real bind rather than a shape that always succeeds. + */ +export function portIsFree(port: number | string): boolean { + return probeBind(Number(port)) !== null; +} + +/** + * `reservePort()` as a string, for the call sites that pass a port straight + * into a `spawn()` argument list. + * + * ⚠️ The name is kept — and is now a slight misnomer, which is cheaper than the + * alternative. It is `String(reservePort())` and nothing else; the draw, the + * guarantees and the residual race are all documented on `reservePort()` above + * and there is no second mechanism hiding behind this name. Renaming it would + * be a rename-only edit across the 8 other files in this directory that call + * it, which is churn this change deliberately does not spend. + */ export function randomPort(): string { - return String(40000 + Math.floor(Math.random() * 20000)); + return String(reservePort()); +} + +/** How a boot says the port was taken — `serve.ts`'s own diagnostic, then the raw kernel error. */ +const PORT_TAKEN_PATTERNS = [ + /Port (\d+) is already in use/, + /EADDRINUSE[^\n]*?:(\d+)/, +]; + +/** + * ⭐ Turn a boot that died on a taken port into a failure that SAYS SO (#12441 + * ruling ④). + * + * Returns an `Error` when `output` shows the child could not bind, else `null`. + * + * ## Why this exists, and why it is the half that pays + * + * The measured cost of a lost port race here is not the lost run. It is *"a red + * suite that is not reproducible, on a test file the reader has no reason to + * connect to a port"* — the failure surfaced as `serve exited 1 before "Server + * is ready"` inside a file about `NODE_ENV` defaulting, and it cost an agent a + * round to decide whether the failure belonged to the change under test. A fix + * that only lowers the probability leaves that cost exactly where it was, just + * rarer and therefore even more surprising when it lands. + * + * So the port number is read out of the CHILD's own diagnostic rather than + * passed in: whatever the harness thought it reserved, the number the child + * printed is the one that was contended. `probedPort` is threaded in only to + * say, in the message, that the port WAS probed free moments earlier — which is + * what tells the reader this is the residual TOCTOU gap and not a harness that + * never looked. + */ +export function portContentionError( + output: string, + what: string, + probedPort?: number | string, +): Error | null { + let port: string | undefined; + for (const pattern of PORT_TAKEN_PATTERNS) { + const match = pattern.exec(output); + if (match) { + port = match[1]; + break; + } + } + if (port === undefined) return null; + const probed = probedPort === undefined + ? '' + : `This harness bind-probed ${probedPort} and the kernel reported it FREE moments earlier ` + + '(`reservePort()` in `test/helpers/serve-process.ts`), so this is the residual ' + + 'close-to-spawn gap that probe narrows but does not close.\n'; + return new Error( + `PORT CONTENTION on port ${port}: \`${what}\` could not bind it.\n` + + probed + + 'Several agents share one container in this fleet, so another process took the port ' + + 'between the probe and the spawn.\n' + + '⛔ This is a HOST race, not a verdict about the code under test. Do not spend a round ' + + 'deciding whether your change caused it — re-run this file in isolation. If it ' + + 'reproduces there, the port is genuinely held and the message above names it.\n' + + `--- child output ---\n${output}`, + ); } /** @@ -214,6 +413,23 @@ export interface ServeRun { stderr: string; } +/** + * The port a caller asked for, read back out of its own `args`. + * + * `runServe` takes the port as an opaque argv element rather than a parameter, + * so this is how it learns which port it probed — for the message only, never + * for the verdict (`portContentionError` reads the contended port out of the + * child's own diagnostic). `undefined` when the caller passed no `--port`, + * which just drops one sentence from the message. + */ +function portOf(args: string[]): string | undefined { + for (const flag of ['--port', '-p']) { + const at = args.indexOf(flag); + if (at !== -1 && at + 1 < args.length) return args[at + 1]; + } + return undefined; +} + /** * Boot `os serve` in `cwd`, collect its output until `waitFor` matches (or the * process exits), then stop it. Never leaves the child running. @@ -221,6 +437,14 @@ export interface ServeRun { * A boot that DIES still has to have said why, so an early exit resolves rather * than rejects — the caller's assertions read what it printed on the way down. * + * ⭐ ONE narrow exception to that, and it is deliberate (#12441): a boot that + * died because it could not BIND rejects, with `portContentionError()`'s + * message. That death says nothing about the code under test, and letting it + * resolve hands the caller an output buffer whose assertions then fail on + * whatever marker is missing — which is the illegible shape the card measured. + * No test in this directory drives a deliberately-busy port, so nothing is + * asserting on the resolved form of it. + * * `waitFor` is matched against **stdout and stderr together** (#7915). `serve` * writes every human line — banner, boot progress, kernel logs — to stderr now, * because its stdout belongs to the MCP stdio transport when one is mounted; @@ -271,8 +495,20 @@ export function runServe( } catch { /* already gone */ } - if (err) rejectRun(err); - else resolveRun({ stdout, stderr }); + if (err) { + rejectRun(err); + return; + } + const contended = portContentionError( + stdout + stderr, + 'os serve (bin/run-dev.js, via runServe)', + portOf(args), + ); + if (contended) { + rejectRun(contended); + return; + } + resolveRun({ stdout, stderr }); }; const timer = setTimeout( diff --git a/packages/cli/test/serve-app-anchored-optional-import.e2e.test.ts b/packages/cli/test/serve-app-anchored-optional-import.e2e.test.ts index 703afee852..958befb06a 100644 --- a/packages/cli/test/serve-app-anchored-optional-import.e2e.test.ts +++ b/packages/cli/test/serve-app-anchored-optional-import.e2e.test.ts @@ -35,10 +35,12 @@ * * The spawn is written out here rather than taken from `test/helpers/ * serve-process.ts` on purpose: that helper always runs the child WITH `cwd` set - * to the app, which is the one shape this file must not use. Only its - * `childEnv()` choke point is borrowed (#11267) — what the child INHERITS is - * orthogonal to which directory it is started in, and this file boots the real - * stack, better-auth included, which reads `TEST` directly. + * to the app, which is the one shape this file must not use. What IS borrowed + * from it is everything orthogonal to the directory the child starts in: + * `childEnv()` (#11267) — this file boots the real stack, better-auth included, + * which reads `TEST` directly — plus `randomPort()` and `portContentionError()` + * (#12441), because a port draw is not a property of the CWD either and this + * file used to carry its own second, overlapping one. * * ── The anti-vacuity floor ─────────────────────────────────────────────── * @@ -57,7 +59,7 @@ import { mkdirSync, mkdtempSync, readFileSync, rmSync, writeFileSync } from 'nod import { tmpdir } from 'node:os'; import { dirname, join, resolve } from 'node:path'; import { fileURLToPath } from 'node:url'; -import { childEnv } from './helpers/serve-process.js'; +import { childEnv, portContentionError, randomPort } from './helpers/serve-process.js'; const HERE = dirname(fileURLToPath(import.meta.url)); @@ -149,6 +151,13 @@ interface Run { stdout: string; stderr: string; both: string } * * An early exit resolves rather than rejects: a boot that DIES still has to have * said why, and the refusal case below reads exactly that. + * + * ⭐ ONE exception (#12441): a boot that died because it could not BIND rejects, + * naming the port. Resolving it would hand the caller an output buffer with no + * marker in it, and the assertions would then fail with "the cluster gate was + * not loaded from the app" — a sentence about resolution bases, for a failure + * that is entirely about a port. That mis-signalling is the whole cost the card + * measured, and it is more expensive than the lost run. */ function runServeFrom( cwd: string, @@ -156,8 +165,15 @@ function runServeFrom( waitFor: RegExp, timeoutMs = 240_000, ): Promise { - return new Promise((resolveRun) => { - const port = String(40000 + Math.floor(Math.random() * 20000)); + return new Promise((resolveRun, rejectRun) => { + // ⛔ Was an inline `String(40000 + Math.random() * 20000)`, a SECOND blind + // draw whose range overlapped the one in + // `serve-node-env-production-default.e2e.test.ts` (41000-60000) — so under + // `--maxWorkers > 1` the two files could collide with EACH OTHER, not only + // with a neighbouring agent's dev server. One bind-probed draw now, in the + // helper; its docblock is the authority on what that does and does not + // guarantee. + const port = randomPort(); const child = spawn(TSX, [CLI, 'serve', configArg, '--port', port], { cwd, env: childEnv({ @@ -179,6 +195,15 @@ function runServeFrom( settled = true; clearTimeout(timer); try { child.kill('SIGTERM'); } catch { /* already gone */ } + const contended = portContentionError( + stdout + stderr, + 'os serve (bin/run-dev.js ⇒ NODE_ENV=development)', + port, + ); + if (contended) { + rejectRun(contended); + return; + } resolveRun({ stdout, stderr, both: stdout + stderr }); }; const timer = setTimeout(finish, timeoutMs); diff --git a/packages/cli/test/serve-node-env-production-default.e2e.test.ts b/packages/cli/test/serve-node-env-production-default.e2e.test.ts index 14ea4d52ca..1f0d3957e3 100644 --- a/packages/cli/test/serve-node-env-production-default.e2e.test.ts +++ b/packages/cli/test/serve-node-env-production-default.e2e.test.ts @@ -144,7 +144,7 @@ import { mkdtempSync, rmSync, writeFileSync } from 'node:fs'; import { join, resolve } from 'node:path'; import type { Readable } from 'node:stream'; import { fileURLToPath } from 'node:url'; -import { childEnv, E2E_SECRET_KEY } from './helpers/serve-process.js'; +import { childEnv, E2E_SECRET_KEY, portContentionError, reservePort } from './helpers/serve-process.js'; /** What `spawn(..., { stdio: ['ignore', 'pipe', 'pipe'] })` actually returns — no `stdin`. */ type ProbeChild = ChildProcessByStdio; @@ -197,11 +197,6 @@ export default { let dir: string; const children: ProbeChild[] = []; -/** A random high port, so a run never contends with another agent's dev server on this host. */ -function randomPort(): number { - return 41000 + Math.floor(Math.random() * 19000); -} - interface OriginCheckResult { status: number; body: any; @@ -218,7 +213,19 @@ interface OriginCheckResult { * stringifying them, so this is not the same as `NODE_ENV=''`. */ async function probeOriginCheck(env: Record): Promise { - const port = randomPort(); + // A BIND-PROBED port, from the one draw this directory has (#12441). It used + // to be a blind `41000 + Math.random() * 19000` under a docblock claiming "a + // run never contends with another agent's dev server on this host" — and this + // very file is where that claim was falsified, with `✗ Port 49402 is already + // in use`. ⚠️ `reservePort()` narrows that race, it does not close it; the + // `exit` handler below is what makes the residual loss legible. Read + // `reservePort()`'s own docblock before trusting either half of that. + // + // ⛔ This must stay a DRAWN port, not a fixed one: `serve` is driven here with + // `NODE_ENV` unset, i.e. into its production posture, where it refuses to + // auto-select a different port on purpose (#11113) — so a pinned port would + // convert an unlikely collision into a certain one. + const port = reservePort(); const untrustedOriginPort = port + 1; writeFileSync(join(dir, 'objectstack.config.ts'), configFor(port), 'utf8'); @@ -307,7 +314,21 @@ async function probeOriginCheck(env: Record): Promis child.stderr.on('data', (d) => { err += String(d); onData(); }); child.on('exit', (code) => { clearTimeout(timer); - readyReject(new Error(`serve exited ${code} before "Server is ready"\n--- stdout ---\n${out}\n--- stderr ---\n${err}`)); + // ⭐ Port contention gets its OWN failure, before the generic one (#12441). + // The generic message — `serve exited 1 before "Server is ready"` — is + // literally what a lost port race produced in this file, and it costs a + // reader a round to work out that a file about NODE_ENV defaulting just + // failed for a reason that has nothing to do with NODE_ENV. `serve` in + // production posture prints `✗ Port is already in use.` and exits 1, + // so the evidence is already in `err` — it just was not being read. + readyReject( + portContentionError( + out + err, + 'os serve (bin/run.js, NODE_ENV unset ⇒ production ⇒ no auto-select)', + port, + ) + ?? new Error(`serve exited ${code} before "Server is ready"\n--- stdout ---\n${out}\n--- stderr ---\n${err}`), + ); }); }); diff --git a/packages/cli/test/serve-port-bind-probe.test.ts b/packages/cli/test/serve-port-bind-probe.test.ts new file mode 100644 index 0000000000..0e3653b6c6 --- /dev/null +++ b/packages/cli/test/serve-port-bind-probe.test.ts @@ -0,0 +1,237 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * #12441 — the pin for the e2e port draw itself. + * + * ## What went wrong, and why the fix needed a pin of its own + * + * Every `os serve` e2e in this directory hands the child a port. That port used + * to be drawn BLIND — `40000 + Math.random() * 20000` in the shared helper, + * `41000 + Math.random() * 19000` in `serve-node-env-production-default`, and a + * third inline copy in `serve-app-anchored-optional-import` — under a docblock + * asserting that "a run never contends with another agent's dev server on this + * host". Several agents share one container in this fleet, and a measured run + * of the full CLI suite went `1 failed | 2101 passed` on: + * + * ✗ Port 49402 is already in use. + * ObjectStack does not auto-select a different port in production mode + * + * clean on an isolated re-run. The unqualified negative in that comment is what + * kept anyone from re-examining the draw; the replacement (`reservePort()`) is + * careful to state its residual race instead. + * + * ## ⚠️ Why this file exists rather than trusting the mechanism + * + * A bind probe is an INSTRUMENT, and an instrument that cannot answer NO is not + * one. `probeBind()` would still look healthy — draws succeed, ports come back, + * every e2e stays green — if its bind silently never failed: it would report + * "free" for a port something else is holding, and the whole guarantee + * `reservePort()`'s docblock makes would be decoration. So the load-bearing + * test here is the NEGATIVE arm, not the positive one. + * + * The second thing pinned is legibility, which is the half of #12441 that pays. + * The residual TOCTOU window is real and `reservePort()` says so; what makes it + * affordable is that losing it now fails saying "PORT CONTENTION on port N" + * rather than `serve exited 1 before "Server is ready"` inside a file about + * `NODE_ENV` defaulting. + * + * ## Why this file is not an `.e2e.` one + * + * It spawns nothing but `node -e` (the probe's own child, ~75 ms) and binds + * loopback sockets in-process. No CLI, no fixture app, no `dist/` dependency. + */ + +import { describe, it, expect } from 'vitest'; +import { readFileSync } from 'node:fs'; +import { createServer, type Server } from 'node:net'; +import { resolve } from 'node:path'; +import { fileURLToPath } from 'node:url'; + +import { portContentionError, portIsFree, randomPort, reservePort } from './helpers/serve-process.js'; + +/** Seeded from `import.meta.url`, the spelling `check:cross-package-test-inputs` recognises. */ +const HERE = resolve(fileURLToPath(import.meta.url), '..'); + +/** Bind `0.0.0.0:0` and KEEP it bound. Resolves with the port and its closer. */ +function hold(): Promise<{ port: number; release: () => Promise }> { + return new Promise((resolveHold, rejectHold) => { + const server: Server = createServer(); + server.on('error', rejectHold); + server.listen(0, '0.0.0.0', () => { + const address = server.address(); + if (address === null || typeof address === 'string') { + rejectHold(new Error(`listen(0) produced no numeric address: ${String(address)}`)); + return; + } + resolveHold({ + port: address.port, + release: () => new Promise((done) => server.close(() => done())), + }); + }); + }); +} + +describe('#12441: the e2e serve port is bind-probed, and the probe can say NO', () => { + it( + 'THE NEGATIVE ARM: portIsFree() answers NO for a port this test is holding, and YES once released', + async () => { + const held = await hold(); + + // The load-bearing assertion of this whole file. If this ever goes green + // the other way round, `reservePort()`'s guarantee is decoration: a probe + // that cannot fail to bind reports every port free. + expect( + portIsFree(held.port), + `the bind probe called held port ${held.port} FREE — it is not an instrument`, + ).toBe(false); + + await held.release(); + // The positive control for the same call, so a probe that answers NO to + // everything (a broken subprocess, a bad argv, a crash on startup) cannot + // pass the assertion above by accident. + expect( + portIsFree(held.port), + `the bind probe called released port ${held.port} BUSY — it answers NO unconditionally`, + ).toBe(true); + }, + 60_000, + ); + + it( + 'reservePort() hands back a real, currently-bindable TCP port', + () => { + const port = reservePort(); + expect(Number.isInteger(port)).toBe(true); + expect(port).toBeGreaterThan(1023); + expect(port).toBeLessThan(65536); + // Free at this instant — which is the ONLY thing the probe claims. ⛔ Not + // "free when serve binds it": that gap is the residual race, stated in + // `reservePort()`'s docblock and made legible by `portContentionError()`. + // + // ⚠️ Three independent draw→verify pairs, and the plurality IS the point + // rather than a workaround: this assertion sits ON the residual window, + // so one pair can legitimately lose it to a neighbour on this shared + // container. Three in a row is not a race, it is a broken probe. + expect( + [port, reservePort(), reservePort()].some((p) => portIsFree(p)), + 'three consecutive bind-probed ports were all busy on verification', + ).toBe(true); + }, + 60_000, + ); + + it( + 'reservePort() never draws a port that is already HELD — the shape the blind draw could not avoid', + async () => { + const holders = await Promise.all(Array.from({ length: 12 }, () => hold())); + const heldPorts = new Set(holders.map((h) => h.port)); + try { + const drawn = Array.from({ length: 12 }, () => reservePort()); + + // The positive control FIRST, on its own input: prove this comparison + // can detect an overlap at all. A `Set` membership test that silently + // matches nothing — number-vs-string being the classic way — would make + // the real assertion below pass on any input whatsoever. + expect( + [[...heldPorts][0]].filter((p) => heldPorts.has(p)), + 'the overlap check cannot detect a port that IS held — it proves nothing', + ).toHaveLength(1); + + expect( + drawn.filter((p) => heldPorts.has(p)), + `a probed draw returned a port held by this test: drawn=${drawn.join(',')}`, + ).toEqual([]); + } finally { + for (const holder of holders) await holder.release(); + } + }, + 120_000, + ); + + it( + 'randomPort() is reservePort() as a string — one draw, no second mechanism', + () => { + const port = randomPort(); + expect(port).toMatch(/^\d+$/); + // Same three-pair shape and the same reason as above — this sits on the + // residual window too. + expect( + [port, randomPort(), randomPort()].some((p) => portIsFree(p)), + 'three consecutive randomPort() draws were all busy on verification', + ).toBe(true); + }, + 60_000, + ); +}); + +/** + * The measured stderr from the run that filed #12441, verbatim (the harness + * spawns with `NO_COLOR=1`, so `serve.ts`'s `chalk.red` wrapper is inert and + * this is byte-for-byte what a contended boot writes). + */ +const MEASURED_CONTENTION = [ + '', + ' ✗ Port 49402 is already in use.', + ' ObjectStack does not auto-select a different port in production mode:', + ' a drifted port silently breaks reverse-proxy, OAuth callback, and CORS config.', + ' Free the port, or pick another via PORT= (or --port ).', +].join('\n'); + +describe('#12441: a lost race fails LEGIBLY — naming port contention and the port', () => { + it('names the contended port, read out of the CHILD\'s own diagnostic', () => { + const error = portContentionError(MEASURED_CONTENTION, 'os serve', 49402); + expect(error).not.toBeNull(); + expect(error?.message).toContain('PORT CONTENTION'); + // ⭐ The port. Without it the reader is back to guessing which of the + // suite's parallel spawns lost, which is the round the card is paying for. + expect(error?.message).toContain('49402'); + }); + + it('reads the port from the diagnostic even when the harness passes none', () => { + expect(portContentionError(MEASURED_CONTENTION, 'os serve')?.message).toContain('49402'); + }); + + it('catches the raw kernel error too, not only serve.ts own wording', () => { + const raw = 'Error: listen EADDRINUSE: address already in use 0.0.0.0:53119'; + expect(portContentionError(raw, 'os serve')?.message).toContain('53119'); + }); + + it( + 'ANTI-VACUITY: the detector is held against the LIVE wording in serve.ts, not a copy of it', + () => { + // ⚠️ `MEASURED_CONTENTION` above is a transcript. A transcript cannot + // notice that the source it was copied from has been reworded — and the + // day `serve.ts` rewrites its diagnostic, every assertion in this + // describe block stays green while the real failure goes back to being + // illegible. That is a phantom check, so the live template is read here. + const serveSource = readFileSync(resolve(HERE, '../src/commands/serve.ts'), 'utf8'); + const template = /Port \$\{requestedPort\} is already in use/.exec(serveSource); + expect( + template, + 'serve.ts no longer prints `Port ${requestedPort} is already in use` — ' + + 'portContentionError()\'s pattern in test/helpers/serve-process.ts must be ' + + 'updated to whatever it prints now, or a lost port race goes illegible again', + ).not.toBeNull(); + + // …and the pattern must actually fire on that template once rendered. + const rendered = String(template?.[0]).replace('${requestedPort}', '51001'); + expect(portContentionError(rendered, 'os serve')?.message).toContain('51001'); + }, + ); + + it( + 'ABLATION-SHAPED CONTROL: stays null for a boot that died of something else', + () => { + // If this returned an Error, every unrelated boot failure in this + // directory would be re-labelled a port race — a detector that always + // fires is exactly as useless as one that never does, and it would bury + // the real diagnostic under a message about ports. + const unrelated = [ + 'Cannot find package \'@objectstack/service-cluster\': the host app does not declare it.', + ' host app: /tmp/os-anchored-neutral-cwd-xyz', + ].join('\n'); + expect(portContentionError(unrelated, 'os serve')).toBeNull(); + expect(portContentionError('', 'os serve')).toBeNull(); + }, + ); +});