Uh oh!
There was an error while loading. Please reload this page.
feat(cli): os serve announces an exhausted port search, carrying the message the search threw (#12620) - #12660
Conversation
…message the search threw (#12620) In development `os serve` walks forward from the requested port looking for a free one. The walk gives up after 101 ports and throws a message that names the problem exactly — and the caller's `catch { /* Ignore */ }` discarded it, fell through, and bound the requested port anyway: the one port the search had just proven was taken. The boot then died on the kernel's raw EADDRINUSE with the accurate explanation thrown away one line earlier. This is the one shape in the whole port policy that reaches NEITHER half of the family's legibility work. The production `Port ... is already in use` line lives in the `else if` this boot never enters, and the shifted-port notice is gated on `port !== requestedPort`, which is false here because the assignment threw before it could happen. The fallthrough itself is unchanged and stays deliberate; whether an exhausted search should refuse instead is #11113's production/development policy split and is not decided here. Only the silence is repaired. The notice CARRIES the thrown message as its headline rather than paraphrasing it, so one fact keeps one spelling. The width it reports is derived from the same constant the walk uses, so the range it names is always the range it probed. Channel is `printDiagnostic` — stderr, the same helper, stream and boot position as the two sibling notices; stdout carries JSON-RPC frames whenever the stdio MCP transport is mounted. `getAvailablePort` now takes its port probe as a parameter, defaulting to the real one. That is what makes the exhausted path testable without holding 101 real ports — a test that would be slow, flaky and hostile to a shared container whose ephemeral range is already crowded. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UjujZN219uFzBhSYfMykCd
…hausted-port-search-notice
📓 Docs Drift CheckThis PR changes 1 package(s): 17 hand-written doc(s) name something this change touched — list omitted above 15 rows. Re-derive on the tree named below: ⛔ 4 release-owned page(s) also affected — read-only, see AGENTS.md Documentation Guardrails. What this run could not see
Coarse fallback — 23 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): Which tree this was computed onThis run read A worktree cut from an older # while this PR is open — GitHub drops the merge commit once it closes
git fetch origin 1859cdce4f3b6ab36f7c4f21078af21c85d54d4e && git checkout 1859cdce4f3b6ab36f7c4f21078af21c85d54d4e
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 3519f8d11fa537e1d9dce2f0f1dabd3555f571cf 0ea410a0c5512285aed39f6cb2d7df2e56a1ba46 && git checkout -B drift-repro 3519f8d11fa537e1d9dce2f0f1dabd3555f571cf && git merge --no-ff 0ea410a0c5512285aed39f6cb2d7df2e56a1ba46
node scripts/docs-audit/affected-docs.mjs --json 3519f8d11fa537e1d9dce2f0f1dabd3555f571cf
|
) The `catch` this notice feeds catches EVERY rejection from the port search, not only an exhausted walk — and the notice asserted a probed range unconditionally. `isPortAvailable` rejects synchronously with ERR_SOCKET_BAD_PORT for any port outside 0-65535, which is reachable two ways: a walk that starts high enough to cross the ceiling, and `--port` text that `parseInt` turns into NaN. Measured against `net` rather than supposed — `listen()` throws for both, inside the probe's promise executor. On those paths nothing was exhausted, so the notice would have printed `probed 101 ports (NaN-NaN)`: an inaccurate diagnostic inside the diagnostic added to stop exactly that. The walk now throws a `PortSearchExhaustedError` carrying its own startPort, lastPort and probedCount, and the span sentence is reachable only through it. Its message is unchanged, so the sentence the notice carries verbatim is untouched and the exhausted-path output is byte-identical to before; the type says WHICH failure this is, not the same thing differently. The numbers are read off the error the walk recorded them on rather than re-derived from `requestedPort`, so there is no second source that could disagree. Any other rejection still carries its own message as the headline — that ruling does not bend by branch — with a body that claims no range. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UjujZN219uFzBhSYfMykCd
…s to catch (#12620) An ablation that neutered the `instanceof PortSearchExhaustedError` guard — so the span body runs for EVERY rejection, the exact regression the guard was added to prevent — left all eight cases GREEN. The predicate was `/probed \d+ ports/`. Removing the guard does not produce a wrong NUMBER; it produces `undefined`, because the span body reads startPort, lastPort and probedCount off an error that does not carry them. `\d+` does not match `probed undefined ports`, so the assertion passed against the one shape it was written for. The `NaN–` check missed it for the same reason. Widened to `/probed/` plus an explicit placeholder check, and the reasoning is recorded at the assertion so the narrow spelling does not come back. The positive control on the real exhausted notice is unchanged, so the negatives still discriminate rather than matching nothing. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UjujZN219uFzBhSYfMykCd
Uh oh!
There was an error while loading. Please reload this page.
Fixes#12620
getAvailablePortgives up after a bounded walk and throws a message that namesthe problem exactly. The caller's
catch { /* Ignore — fall through */ }threwthat message away, fell through, and bound
requestedPortanyway — the one portthe search had just proven was taken. The boot then died on the kernel's raw
EADDRINUSEwith the accurate explanation discarded one line earlier.This is the one shape in the whole port policy that reaches neither half of
the family's legibility work: the production
Port … is already in uselinelives in the
else ifthis boot never enters, and #12543's drift notice is gatedon
port !== requestedPort, which is false here because the assignment threwbefore it could happen. Both halves are correct; the gap is exactly between them.
What this does and does not change
⛔ The fallthrough stays. Whether an exhausted search should refuse instead is
#11113's production/development policy split, and answering it here would be
answering a different card. Only the silence is repaired. Nothing about what
os servedoes is changed — an ordinary auto-shift and a production boot arebyte-identical to before.
The notice carries the thrown error's own message as its headline rather than
paraphrasing it, so one fact keeps one spelling instead of two that can drift.
Everything after the headline is a different fact — how wide the search was,
what happens next, what to do:
portplaceholder, matching the production refusal it sits beside. GitHub's body
sanitizer eats short angle-bracket fragments — including inside a fenced block —
so it is spelled
PORTNUMabove. The source and the changeset carry the realbytes; this substitution exists only so the quoted line is not silently truncated
here.
Channel is
printDiagnostic→ stderr, re-derived from the merged code ratherthan from any card's description of it: the same helper, stream and boot position
as both sibling notices.
stdoutcarries JSON-RPC frames whenever the stdio MCPtransport is mounted, which is what
serve-stdio-stdout-purity.e2e.test.tspins;that suite was run on this branch and passes.
The span claim is made only for the failure it is true of
The
catchcatches every rejection from the search, not only an exhaustedwalk — so a body that always claimed a probed range would sometimes assert a
search that never ran. That is reachable, and measured against
netrather thansupposed:
isPortAvailablerejects synchronously withERR_SOCKET_BAD_PORTforany port outside 0–65535, which happens when the walk crosses the ceiling and
when
--porttext turns intoNaNunderparseInt. Unguarded, the notice wouldhave printed
probed 101 ports (NaN–NaN)— an inaccurate diagnostic inside thediagnostic added to stop exactly that.
So the walk throws a
PortSearchExhaustedErrorcarrying its ownstartPort,lastPortandprobedCount, and the span sentence is reachable only through it.Its message is unchanged, so the sentence the notice carries verbatim is
untouched and the exhausted-path output is byte-identical either way; the type
says which failure this is, not the same thing differently. Any other rejection
still carries its own message as the headline — that ruling does not bend by
branch — with a body that claims no range.
The two numbers, measured here rather than inherited
101 ports, contiguous, no skip. The walk probes
startPortthroughstartPort + 100inclusive — the guard fires only afterporthas alreadybeen incremented past the last port checked. Counted by instrumenting the loop
and reading the probe sequence back, not by reading the source and reasoning:
ports probed: 101 · first/last: 3000/3100 · contiguous: true.It is a plain
port++walk with no skip mechanism. (A neighbouringchangeset's example shows
32869 → 32871; that is reachable on a contiguouswalk — it just means 32870 was busy too — so there is nothing to correct there.
But a diagnostic written from a "skip" reading would name a range the search
never looked at.)
The numbers in the notice come off the error the walk recorded them on, never
re-derived from
requestedPort— one source, so there is no second one todisagree. An off-by-one inside a diagnostic that exists to be accurate would be
this card's own defect, so it is pinned against the measured probe sequence
rather than against a literal.
Testing — the seam, not 101 sockets
⛔ No test here binds a port.
getAvailablePortnow takes its probe as aparameter (defaulting to the real one); that injection is what made the exhausted
path reachable at all. The alternative — holding 101 real ports — is slow, flaky
and hostile to a shared container whose ephemeral range is already crowded;
#12441 measured exactly that contention taking this suite red at
1 failed | 2101 passed, clean on an isolated re-run.packages/cli/src/commands/serve-exhausted-port-search-notice.test.tspins thethree-way discrimination, each arm at the level it is actually decidable at:
catchnever runs and the notice is unreachable by constructionif (portAutoShiftAllowed), the production line is in theelse ifArm 3 is deliberately not a fourth spawner file: this package's own
vitest.config.tsrecords its 39 spawner files carrying 89.4% of its test wall ata ~5.5–6.0s floor each, and arms 2 and 3 already hold landed runtime pins —
test/serve-port-drift-notice.e2e.test.tsandtest/serve-node-env-production-default.e2e.test.ts. What those lacked was thediscrimination against this notice, which the source-anchored arm supplies. The
first of those was run on this branch and passes.
A mutual-exclusion arm asserts the three notices are pairwise non-matching. One
pair is load-bearing beyond legibility:
PORT_TAKEN_PATTERNSintest/helpers/serve-process.tsturns/Port (\d+) is already in use/and/EADDRINUSE[^\n]*?:(\d+)/into a port-contention verdict for every spawner inthis package. This notice names
EADDRINUSEon purpose, so a colon followed bydigits after that word would make an exhausted boot mis-report as a lost port
race in an unrelated file. That is asserted, not merely avoided.
Ablation — the tests are shown able to go red
Four mutations, each proven on disk by hash before its run and proven restored
byte-identical to the
HEADblob after it (restore viagit checkout HEAD --,never a bare
git checkout --, which restores from the index). No build step isinvolved and that is measured, not assumed: the test imports
./serve.jsrelatively, so vitest transforms the source — and these mutations turning the
suite red is what establishes it.
1 failed | 7 passed1 failed | 7 passed3 failed | 5 passedinstanceofguard, so the span is claimed for every rejection1 failed | 7 passed8 passed (8), tree byte-clean vsHEADthis PR's own test. The negative assertion read
/probed \d+ ports/. Removingthe guard does not produce a wrong number — it produces
undefined, becausethe span body reads its fields off an error that does not carry them, and
\d+does not match
probed undefined ports. The narrow spelling passed againstprecisely the regression it existed to catch. Widened to
/probed/plus anexplicit placeholder check, with the reasoning recorded at the assertion; the
positive control on the real notice is unchanged, so the negatives still
discriminate rather than matching nothing. Re-run after the fix: red, as it
should be.
Verification
All heavy runs went through
scripts/pm/os-verify-lock.sh; every exit code wascaptured before any pipe, and each verdict below is the gate's own, not a
$?read through
tail. Re-run in full at0ea410a0c, which isHEAD.pnpm lint(eslint . --no-inline-config) exit 0. Thewhole sweep, so there is no narrowing to declare.
node scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack(24path-derived, plus the convention-triggered "adds a test file" set) exit 0,
including
check:slot-lookup,check:cross-package-test-inputs,check:engine-double-contract,check:where-matcher,check:type-check-coverage,check:type-check-debt,check:i18n,check:published-filesandcheck:nul-bytes. One non-result:scripts/pm/check-half-states.mjsexits 3 =PREREQUISITE NOT MET— thiscontainer holds no valid GitHub credential, so it swept nothing. That is not a
measurement in either direction; CI runs it with a real token.
pnpm --filter @objectstack/cli typecheckexit 0, and the newtest file is actually in the program:
tsc --noEmit --listFileslists bothserve-exhausted-port-search-notice.test.tsandcommands/serve.ts(a cleantypecheck says nothing about a file no tsc program includes).
serve.ts:5 files / 48 tests passed. Plus the two e2e suites this change couldplausibly red —
serve-port-drift-notice.e2e([finding]getAvailablePortreturns a DIFFERENT port than requested and nothing says so — the drift is announced nowhere, and three test files have been silently talking past it #12543's arm) andserve-stdio-stdout-purity.e2e(the channel ruling) —2 files / 3 tests passed.70 successful, 70 total, before the gatesthat require it.
Changeset
minoron@objectstack/cli, reasoned from AGENTS.md rather than reached for byhabit: this is published CLI source and the change is user-visible new output on
a path that previously printed nothing, which is a functional improvement rather
than a pure bug fix.
skip-changesetwould be wrong here. #12543 — the sameshape, the same file, landing in the same release — shipped
minor, and thesetwo notices will read as one story in the release notes.
Generated by Claude Code