objectstack-ai
diff --git a/‎.changeset/cli-serve-exhausted-port-search-notice.md‎
Lines changed: 44 additions & 0 deletions b/‎.changeset/cli-serve-exhausted-port-search-notice.md‎
Lines changed: 44 additions & 0 deletions
diff --git a/‎packages/cli/src/commands/serve-exhausted-port-search-notice.test.ts‎
Lines changed: 304 additions & 0 deletions b/‎packages/cli/src/commands/serve-exhausted-port-search-notice.test.ts‎
Lines changed: 304 additions & 0 deletions
@@ -0,0 +1,44 @@
+---
+"@objectstack/cli": minor
+---
+
+feat(cli): `os serve` says so when its port search runs out of ports, in the words the search threw (#12620)
+
+In development (`os dev`, `--dev`, or `NODE_ENV=development`) `os serve` walks
+forward from the requested port looking for a free one. That walk gives up after
+101 ports, and when it does it throws a message naming the problem exactly —
+which the caller then discarded, fell through, and bound the requested port
+anyway: the one port the search had *just proven* was taken.
+
+That boot died on the kernel's raw `EADDRINUSE` with no explanation anywhere. It
+is the one shape in the whole port policy that reached **neither** half of this
+family's legibility work: the production `Port … is already in use` line lives in
+a branch this boot never enters, and the shifted-port notice (#12543) is printed
+only when the bound port differs from the requested one — on this path they are
+the same, because the search threw before it could assign. The accurate sentence
+existed and was thrown away one line earlier.
+
+The fallthrough is unchanged and still deliberate — a developer whose whole next
+span is busy arguably does want the requested port attempted rather than a hard
+refusal, and whether it should refuse instead is a separate policy question
+(#11113). What changed is that it is no longer silent:
+
+```
+  ⚠ Could not find an available port starting from 32869
+     Development auto-shift probed 101 ports (32869–32969) and every
+     one was busy, so this server is falling back to 32869 — the port the
+     search has just proven is taken. The bind that follows will almost
+     certainly fail with a raw EADDRINUSE from the kernel, and this notice
+     is the only place that says why.
+     Free a port in 32869–32969, or pick another via PORT=<port> (or --port <port>).
+```
+
+The first line is the search's own thrown message, carried rather than
+paraphrased, so there is one spelling of that fact and not two that can drift
+apart. The width it reports is read from the same constant the walk uses, so the
+range it names is always the range it actually probed.
+
+Written to **stderr**, like every other `os serve` diagnostic: `stdout` carries
+JSON-RPC frames whenever the stdio MCP transport is mounted, so nothing but
+protocol may go there. It prints only when the search is exhausted — an ordinary
+auto-shift and a production boot are both unchanged, byte for byte.
@@ -0,0 +1,304 @@
+// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
+
+/**
+ * #12620 — when the dev port search runs out of ports, `os serve` SAYS SO,
+ * carrying the message the search itself threw.
+ *
+ * ## The defect was a discarded sentence, not a wrong behaviour
+ *
+ * `getAvailablePort` throws `Could not find an available port starting from
+ * <n>` — a sentence that names the problem exactly. The caller's `catch` used
+ * to read `// Ignore — fall through and try the requested port` and did exactly
+ * that: it dropped the sentence and bound `requestedPort` anyway, the port the
+ * search had just proven was taken. The boot then died on the kernel's raw
+ * `EADDRINUSE` with nothing anywhere explaining it.
+ *
+ * ⛔ The fallthrough itself is NOT changed and this file does not argue for
+ * changing it. Whether an exhausted search should refuse instead is #11113's
+ * production/development policy split and belongs to that card.
+ *
+ * ## ⛔ Why this file binds no sockets
+ *
+ * The obvious test — hold 101 real ports and boot — is the wrong test: slow,
+ * flaky, and hostile to a shared many-agent container whose ephemeral range is
+ * already crowded. #12441 measured that contention taking a full CLI suite red
+ * (`1 failed | 2101 passed`, clean on an isolated re-run), which is why
+ * `serve-port-bind-probe.test.ts` exists at all. So the SEAM is driven instead:
+ * `getAvailablePort` now takes its port probe as a parameter, defaulting to the
+ * real one, and every runtime assertion below drives that parameter. Zero
+ * sockets, zero spawns.
+ *
+ * ⚠️ That parameter is the change that made this card testable at all. Without
+ * it the exhausted path is reachable only by exhausting a real range, which is
+ * the test this card's ruling forbids.
+ *
+ * ## THE THREE-WAY DISCRIMINATION (the anti-vacuity requirement)
+ *
+ * A test asserting only "the notice appears when the search is exhausted"
+ * passes just as green against code that prints it unconditionally. Three arms,
+ * each pinned at the level it is actually decidable at:
+ *
+ *  1. **exhausted search** → the notice appears, carrying the thrown message.
+ *     RUNTIME, at the seam.
+ *  2. **ordinary auto-shift** (`port !== requestedPort`, search SUCCEEDED) →
+ *     this notice does not appear; #12543's drift notice does. RUNTIME, at the
+ *     seam — and exactly, not by proxy: a successful search RETURNS, so the
+ *     `catch` never runs and the notice is unreachable by construction. The
+ *     drift half of that arm has its own landed runtime pin in
+ *     `test/serve-port-drift-notice.e2e.test.ts`, which spawns a real boot
+ *     against a real HTTP neighbour; it is not re-spawned here.
+ *  3. **production branch** (`portAutoShiftAllowed` false) → neither notice;
+ *     the existing `Port … is already in use` line fires. STRUCTURAL, against
+ *     the live source: this notice's only call site is lexically inside the
+ *     `if (portAutoShiftAllowed)` block and the production line is in the
+ *     `else if`, so a production boot cannot reach it. The production line's
+ *     own runtime pin is landed in
+ *     `test/serve-node-env-production-default.e2e.test.ts`.
+ *
+ * ⚠️ Arm 3 is deliberately not a fourth spawner file. This package's own
+ * `vitest.config.ts` records that its 39 spawner files carry 89.4% of its test
+ * wall at a ~5.5-6.0s floor each, and arms 2 and 3 already hold the landed
+ * runtime pins named above. What was missing from them was the discrimination
+ * against THIS notice, and that is what the source-anchored arm supplies.
+ *
+ * ## The mutual-exclusion arm, and why it is not decoration
+ *
+ * The three notices are also asserted to be pairwise non-matching. One of those
+ * pairs is load-bearing beyond legibility: `PORT_TAKEN_PATTERNS` in
+ * `test/helpers/serve-process.ts` turns `/Port (\d+) is already in use/` and
+ * `/EADDRINUSE[^\n]*?:(\d+)/` into a "port contention" verdict for every
+ * spawner in this package. This notice names EADDRINUSE in prose on purpose —
+ * that is what the operator is about to see — so if it ever grew a `:<digits>`
+ * after that word, an exhausted-search boot would be mis-reported as a lost
+ * port race by an unrelated file.
+ */
+
+import { describe, it, expect } from 'vitest';
+import { readFileSync } from 'node:fs';
+import { resolve } from 'node:path';
+import { fileURLToPath } from 'node:url';
+
+import { formatExhaustedPortSearchNotice, getAvailablePort } 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 when colour happens to be
+ * off is a flake waiting for the first person who runs this attached.
+ */
+const plain = (text: string): string => text.replace(/\u001B\[[0-9;]*m/g, '');
+
+/** An arbitrary start port. Nothing is bound, so it only has to be a number. */
+const START = 34_500;
+
+/** A probe that never finds a free port — the exhausted path, without a socket. */
+function alwaysBusy(): { probe: (port: number) => Promise<boolean>; probed: number[] } {
+  const probed: number[] = [];
+  return {
+    probed,
+    probe: async (port: number) => {
+      probed.push(port);
+      return false;
+    },
+  };
+}
+
+/** #12543's notice, as its landed text. Held against the source below so it cannot go stale. */
+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/;
+
+describe('#12620: an exhausted port search is announced, in the words the search threw', () => {
+  it('ARM 1 — the notice CARRIES the thrown message verbatim, as its headline', async () => {
+    const { probe } = alwaysBusy();
+
+    const thrown = await getAvailablePort(START, probe).then(
+      (port) => {
+        throw new Error(`the search returned ${port} against a probe that never says yes`);
+      },
+      (err: unknown) => err,
+    );
+
+    expect(thrown, 'the exhausted search no longer throws').toBeInstanceOf(Error);
+    const message = (thrown as Error).message;
+
+    // Guard the guard: an empty message would make the assertion below
+    // vacuously true.
+    expect(message, 'the thrown message no longer names the start port').toContain(String(START));
+
+    const notice = plain(formatExhaustedPortSearchNotice(START, thrown));
+
+    // ⭐ The pin. Not "says something similar" — the error's own text, and as
+    // the headline, so a reader meets it first rather than digging for it.
+    expect(
+      notice.split('\n').find((line) => line.includes('⚠')),
+      'the notice paraphrases the thrown message instead of carrying it',
+    ).toBe(`  ⚠ ${message}`);
+  });
+
+  it('ARM 1 — the width it reports is the width it actually probed, endpoints included', async () => {
+    const { probe, probed } = alwaysBusy();
+    const thrown = await getAvailablePort(START, probe).catch((err: unknown) => err);
+
+    // ⚠️ The search is a plain contiguous `port++` walk. A neighbouring card
+    // described it as skipping ports; it does not, and a notice written from
+    // that reading would name a range it never looked at.
+    expect(
+      probed.every((port, i) => port === START + i),
+      `the search is no longer contiguous: ${probed.slice(0, 5).join(',')}…`,
+    ).toBe(true);
+    expect(probed[0], 'the walk no longer starts where it was asked to').toBe(START);
+
+    // ⭐ The anti-off-by-one arm, and the reason it compares against `probed`
+    // rather than against a literal: an inaccurate number inside a diagnostic
+    // that exists to be accurate would be this card's own defect. BOTH terms
+    // are measured here — how many ports the walk touched, and the last one it
+    // reached.
+    const notice = plain(formatExhaustedPortSearchNotice(START, thrown));
+    expect(notice).toContain(`${probed.length} ports (${START}–${probed[probed.length - 1]})`);
+  });
+
+  it('makes NO span claim for a rejection that is not an exhausted walk', async () => {
+    // ⚠️ The `catch` in serve.ts catches every rejection, not only exhaustion.
+    // `isPortAvailable` rejects synchronously with ERR_SOCKET_BAD_PORT for any
+    // port outside 0–65535 — reachable when the walk crosses the ceiling, and
+    // when `--port` text parses to NaN. Measured, not supposed: `net`'s
+    // `listen()` throws for both, inside the probe's promise executor.
+    //
+    // ⭐ On those paths nothing was probed, so a body claiming a range would
+    // print `NaN–NaN` and assert a search that never ran — an inaccurate
+    // diagnostic inside the diagnostic added to stop exactly that.
+    const badPort = new RangeError('options.port should be >= 0 and < 65536. Received NaN.');
+    const notice = plain(formatExhaustedPortSearchNotice(Number.NaN, badPort));
+
+    // The thrown text is still carried — that ruling does not bend by branch.
+    expect(notice).toContain(badPort.message);
+
+    // …but the claim that is false here is simply not made.
+    //
+    // ⚠️ The predicate here is `/probed/`, NOT `/probed \d+ ports/`, and the
+    // difference is the whole test. Removing the branch guard does not produce
+    // a WRONG number — it produces `undefined`, because the span body reads
+    // `startPort`/`lastPort`/`probedCount` off an error that does not carry
+    // them. A `\d+` pattern does not match `probed undefined ports`, so the
+    // narrow spelling passed against precisely the regression this case exists
+    // to catch. Measured: an ablation that neutered the guard left all eight
+    // cases green.
+    expect(notice, 'a span was claimed for a search that never walked one').not.toMatch(/probed/);
+    expect(
+      notice,
+      'the notice rendered a placeholder where a number belongs',
+    ).not.toMatch(/undefined|NaN–/);
+
+    // Anti-vacuity for the two negatives above: the real exhausted notice DOES
+    // make both claims, so their absence here is a discrimination and not a
+    // regex that stopped matching anything.
+    const { probe } = alwaysBusy();
+    const real = await getAvailablePort(START, probe).catch((err: unknown) => err);
+    const exhausted = plain(formatExhaustedPortSearchNotice(START, real));
+    expect(exhausted).toMatch(/probed \d+ ports/);
+    expect(exhausted).toContain(`${START}–`);
+  });
+
+  it('ARM 2 — a search that SUCCEEDS returns, so the notice is unreachable on the drift path', async () => {
+    // Busy at the requested port, free at the next one: the ordinary auto-shift.
+    const probed: number[] = [];
+    const port = await getAvailablePort(START, async (candidate) => {
+      probed.push(candidate);
+      return candidate !== START;
+    });
+
+    // The drift really is a drift — without this the arm proves nothing.
+    expect(port, 'the probe did not produce a shift to discriminate against').not.toBe(START);
+    expect(port).toBe(START + 1);
+    expect(probed).toEqual([START, START + 1]);
+
+    // ⭐ THE DISCRIMINATION. `getAvailablePort` RESOLVED, so the caller's
+    // `catch` — the only place this notice is written — cannot run. The notice
+    // is not merely absent on this path; it is unreachable.
+    //
+    // What fires instead is #12543's drift notice, guarded on
+    // `port !== requestedPort`: true here, and false on the exhausted path,
+    // where the assignment never happens at all. Both guards are read from the
+    // live source rather than described.
+    expect(SERVE_SOURCE, 'the notice moved out of the catch that guards it').toMatch(
+      /catch \(searchExhausted\) \{[\s\S]*?printDiagnostic\(formatExhaustedPortSearchNotice\(requestedPort, searchExhausted\)\);/,
+    );
+    expect(SERVE_SOURCE, "#12543's drift notice is no longer gated on a real shift").toContain(
+      'if (port !== requestedPort) {',
+    );
+    expect(SERVE_SOURCE, "#12543's drift wording moved; this file's discrimination is stale").toMatch(
+      /Port \$\{requestedPort\} is in use — serving on \$\{port\} instead\./,
+    );
+  });
+
+  it('ARM 3 — the notice is inside the auto-shift branch, so production never reaches it', () => {
+    const autoShift = SERVE_SOURCE.indexOf('if (portAutoShiftAllowed) {');
+    const productionBranch = SERVE_SOURCE.indexOf('} else if (!(await isPortAvailable(requestedPort)))');
+    const noticeCallSite = SERVE_SOURCE.indexOf('printDiagnostic(formatExhaustedPortSearchNotice(');
+    const productionLine = SERVE_SOURCE.indexOf('is already in use.');
+
+    // Every anchor has to exist, or the ordering assertions below compare -1s
+    // and pass while measuring nothing.
+    expect(autoShift, 'the `portAutoShiftAllowed` branch head is gone').toBeGreaterThan(-1);
+    expect(productionBranch, 'the production `else if` is gone').toBeGreaterThan(-1);
+    expect(noticeCallSite, 'the exhausted-search notice has no call site').toBeGreaterThan(-1);
+    expect(productionLine, 'the production in-use line is gone').toBeGreaterThan(-1);
+
+    // ⭐ The notice sits between the branch head and the `else if`; the
+    // production line sits after it. A production boot never enters the block
+    // this notice lives in.
+    expect(noticeCallSite).toBeGreaterThan(autoShift);
+    expect(noticeCallSite).toBeLessThan(productionBranch);
+    expect(productionLine).toBeGreaterThan(productionBranch);
+  });
+
+  it('ARM 3 — the sink is `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
+    // diagnostic written anywhere else reds that suite from this file.
+    expect(SERVE_SOURCE).toContain('printDiagnostic(formatExhaustedPortSearchNotice(');
+    expect(SERVE_SOURCE, '`printDiagnostic` no longer writes to stderr').toMatch(
+      /const printDiagnostic = \(text = ''\) => \{\s*\n\s*if \(!bootQuiet\) process\.stderr\.write/,
+    );
+  });
+
+  it('MUTUAL EXCLUSION — the three notices cannot be mistaken for one another', async () => {
+    const { probe } = alwaysBusy();
+    const thrown = await getAvailablePort(START, probe).catch((err: unknown) => err);
+    const notice = plain(formatExhaustedPortSearchNotice(START, thrown));
+
+    expect(notice, "the exhausted notice reads as #12543's drift notice").not.toMatch(DRIFT_NOTICE);
+
+    // ⚠️ Load-bearing beyond legibility: this pattern is the first of
+    // `PORT_TAKEN_PATTERNS` in `test/helpers/serve-process.ts`, which every
+    // spawner in this package uses to decide that a boot lost a port race.
+    expect(notice, 'the exhausted notice reads as the production refusal').not.toMatch(
+      PRODUCTION_REFUSAL,
+    );
+
+    // The other half of that helper's pattern.
+    expect(notice, 'the notice now trips the EADDRINUSE contention pattern').not.toMatch(
+      /EADDRINUSE[^\n]*?:(\d+)/,
+    );
+
+    // …and both patterns are live instruments, not dead regexes: each must
+    // still match the text it was written for, or the two negatives above
+    // prove nothing at all.
+    expect('  ⚠ Port 3000 is in use — serving on 3001 instead.').toMatch(DRIFT_NOTICE);
+    expect('  ✗ Port 3000 is already in use.').toMatch(PRODUCTION_REFUSAL);
+  });
+
+  it('carries a non-Error rejection intact rather than rendering it as [object Object]', () => {
+    // The probe is injectable, so the seam can now reject with anything. The
+    // ruling is to carry the text the seam produced, whatever it is.
+    const notice = plain(formatExhaustedPortSearchNotice(START, 'probe socket exploded'));
+    expect(notice).toContain('probe socket exploded');
+    expect(notice).not.toContain('[object Object]');
+  });
+});