Skip to content

Commit c76e849

Browse files
test(cli): runServe() takes a kernel-assigned port and returns the bound one; the released-port pin widens to every CLI spawn (#24386)
Part of #24342 Clause-②: no Basis for the Clause-② line: every changed file is under `packages/cli/test/**`, the CLI's test harness. `@objectstack/cli` publishes `dist`, `README.md` and `CHANGELOG.md` (its `files[]`). Measured after `pnpm --filter @objectstack/cli build`: the symbols this PR adds or spreads (`RELEASED_DRAW_LEDGER`, `cliSpawners`, `SLICE_2`, `KERNEL_ASSIGNED_PORT`, `readKernelAssignedPort`) occur in 0 of those files. The positive control `childPortEnv`, which comes from `src/`, occurs in 2. Nothing published moves, so this PR carries the `skip-changeset` label and no changeset. This is slice 1 of #24342, the `runServe()` family. The card stays open for slice 2: the 25 files listed under "What slice 2 converts" below. ## Why the card is split Triage `6251398054`: "If the change is over 1,000 lines, split it by file family, as the card says." - This slice measures 499 changed lines (+306 / -193, 12 files). - A conversion of 31 of the 35 development-column files was measured at 788 changed lines before the pin and the helper docblocks were touched. The widened pin is 269 changed lines here, about 25 of them the slice-2 ledger. Even without that ledger the whole card is past 1,000, and that is before the last three `bin/run.js --dev` files. - So this slice is the files that call `runServe()`. The bespoke spawners and the `bin/run.js --dev` files are slice 2. ## What changed - **`runServe()`** (`test/helpers/serve-process.ts`): - A caller that passes no `--port` now gets `--port KERNEL_ASSIGNED_PORT` (`0`). The child's own `listen()` picks the port, so no released number crosses a gap. - `ServeRun` gains `port`: the port the child bound, read back from its own banner. It is `undefined` when the run ended without a complete banner (a boot that died, or a `waitFor` that matched an earlier line). - A caller that passes its own `--port` keeps both old checks, `portContentionError()` and `portDriftError()`. The held-port arm of `serve-port-readback` depends on that. - **The drift check for the converted family (A4).** With `--port 0` there is no number to drift from, so the check becomes the read-back. `runServe()` rejects a complete banner that names no non-zero `http://localhost` port, through `readKernelAssignedPort()`. That covers an unreadable `API:` row (an `OS_AUTH_URL` / `BETTER_AUTH_URL` / `OS_BASE_URL` origin) and an announced `localhost:0`. A boot with no complete banner gets no verdict, as before. - **10 `runServe()` caller files**, every draw removed (21 draws): - `serve-app-runtime-hooks.e2e` (1), `serve-boot-diagnostics-noise-budget.e2e` (1), `serve-boot-diagnostics.e2e` (2), `serve-cluster-driver-diagnosis.e2e` (1), `serve-no-artifact.e2e` (5), `serve-organizations-host-resolution.e2e` (3), `serve-organizations-mount-failure.e2e` (4), `serve-package-declared-capabilities.test` (1), `serve-port-readback.e2e` (1). - `serve-package-registry-always-on.e2e` (2): its `runServe()` call, and also its own `os serve --dev` boot, so the whole file leaves the ledger. The boot reads its port back and each of its two boots builds its own base URL. - **`serve-port-readback.e2e`.** - The positive control is now a kernel-assigned boot: `runServe()` resolves with a `port` greater than 0 that equals the banner's read-back. - Three cheap cases pin `readKernelAssignedPort()`'s answers: a complete banner, `localhost:0`, and an unreadable or missing banner. - The held-port arm is unchanged. - **The pin** (`released-port-production-spawn.pin.test.ts`). Its population widens from "names `bin/run.js`" to "spawns the CLI". A file is in it when its code does any of these: - spells either entry, `bin/run.js` or `bin/run-dev.js`, as one string or as segments in one call; - imports `CLI` or `runServe` from the harness, by name, under an alias, or as a namespace member. - `TSX` alone does not count, because it runs whatever script it is handed. - The ledger is renamed `RELEASED_DRAW_LEDGER`, since an entry is no longer "outside the production column". It holds every unconverted file with its exact draw count and the reason `slice 2 of #24342`: 25 files, 28 draws. The four old `OUTSIDE_THE_PRODUCTION_COLUMN` entries are among them. The pin is green. - New fixtures cover a `runServe` caller, an aliased `CLI`, a namespace `CLI`, `TSX` alone, and a local non-harness `CLI`. The source entry is now in the population. - The call shape `runServe` + `(` is spelled in pieces inside the pin. `vitest-tiers.ts` reads that shape in code position, string literals included, and would otherwise move the pin to the `integration` tier. Measured: every changed test file keeps its tier from the base commit. - **Docblocks.** `reservePort()`, `randomPort()`, `boundPortFromBanner()` and `portDriftError()` now say what may still draw: instruments, and the slice-2 files while they stay in the ledger. ## Measured before writing it (A1 to A3) - **A1.** `git grep -l "reservePort\|randomPort"` names 39 files at `6a49efb9c`, and `git grep -l "runServe("` names 21. A TypeScript AST census, counting calls rather than lines, splits the 37 drawing test files like this: - 10 call `runServe()`: 21 draws, all converted here. - 21 spawn `CLI` themselves (bespoke tsx spawners): 24 draws, slice 2. - 4 spawn `bin/run.js --dev` or `os dev`: 4 draws, slice 2. - 2 are instruments that spawn no CLI (`serve-port-bind-probe` 7, `serve-probe-child-attribution` 1). They stay outside the population. - Of the 21 `runServe(` hits: 10 are the callers above. 4 draw but name `runServe` only in prose (`package-install-local-handlers`, `serve-migrate-exit-schema-parity`, `serve-port-drift-notice`, `serve-process-child-env`), so they are bespoke spawners and slice 2. 2 define a local function of that name and draw nothing. 3 are production files #24295 already converted. The other 2 are the helper and the pin. - **A2.** Re-verified for the `runServe()` shape (tsx `serve --port 0`): every converted boot reached its banner with a non-zero `http://localhost` port. One reading: port 44449. - **A3.** No `runServe()` caller needs the number before the spawn. `runServe()` stops the child once `waitFor` matches, and no caller addresses the port afterwards. The one caller that passes a number, the held-port arm, holds it on purpose. ## Reverse verification (one-time, not kept) - **The pin turns red on a reverted caller.** - Mutation through `scripts/ablation-replace.mjs` (WRAP mode): `serve-boot-diagnostics.e2e`'s `runServe(dir, ['--log-level', 'debug'], …)` became `['--port', randomPort(), '--log-level', 'debug']`. - Result: `Tests 1 failed | 3 passed (4)`, with `serve-boot-diagnostics.e2e.test.ts: 1 released-port draw(s) (line 112), ledger allows 0`. - That file names no entry and imports no `CLI`, so only the new `runServe` leg of the population could catch it. - Restore: blob == HEAD (`e97518ea9587`), `git diff HEAD` empty. - **The A4 read-back can fail on a real boot.** A scratch test, deleted afterwards (`git status` clean): - A plain `runServe(dir, [], …)` resolved with port 44449. - The same boot with `OS_BASE_URL=https://app.example.com` rejected with `CANNOT READ BACK THE BOUND PORT of \`os serve (bin/run-dev.js, via runServe)\`: it was asked for port 0 …`. - **The lost race, forced.** `serve-port-readback`'s held-port arm hands an explicit held port to `runServe()`, and the boot is rejected with `PORT DRIFT`. That is the red this card describes. The kernel-assigned positive control in the same run resolves. Both pass (16/16). ## Tests All on head `98af344ee`, after merging `origin/main` `5da2e8881`; closure rebuilt with `turbo run build --filter='@objectstack/cli...'`. - Queue tier (`vitest run --maxWorkers=2`): - `released-port-production-spawn.pin` 4/4 (project `unit`) - `serve-package-declared-capabilities` 5/5 - Nightly tier (`OS_TEST_TIERS=nightly`): - `serve-port-readback` 16/16 - `serve-boot-diagnostics` 2/2 - `serve-boot-diagnostics-noise-budget` 3/3 - `serve-app-runtime-hooks` 1/1 - `serve-cluster-driver-diagnosis` 8/8 - `serve-no-artifact` 5/5 - `serve-organizations-host-resolution` 3/3 - `serve-organizations-mount-failure` 4/4 - `serve-package-registry-always-on` 5/5 - 56/56 in 11 files. The same 56 also passed on the pre-merge head `188625089`. - `pnpm --filter @objectstack/cli typecheck`: exit 0. The test layer holds its 3-file / 28-error ledger. `tsc --listFiles -p tsconfig.test.json` puts all 12 changed files in the program, with 0 errors in them. - `pnpm exec eslint --no-inline-config` over the 12 changed files: exit 0. ## Gates (local subset, head `98af344ee`) - `node scripts/pm/dispatch-gates.mjs --commands` derives 52 commands. Each ran with its exit code captured before any pipe, and all 52 exited 0. - `--ran` reconciliation: `Run reconciliation — 52 derived, 52 run, 0 NOT-MEASURED, 0 UNRUN.` - The 11 families of the "declared wide population" block (`check:init-service-contract`, `check:live-db-isolation`, `check:meta-type-normalized`, `check:optional-error-sink`, `check:resume-authority-declared`, `check:route-envelope`, `check:runner-env-posture`, `check:settings-bind-window`, `check:startup-registry-verdict`, `check:verify-stand-in`, `check:wildcard-fallthrough`): all 11 exited 0. - `check:cli-test-child-env` (red once on #24295) printed `155 spawner source(s) among 305 under packages/cli/test/**; no new bulk process.env copy reaches a spawned child, all 189 spawn call(s) declare their child's env`, exit 0. - The repo-wide `pnpm lint` and the CI farm are CI's, read from this PR's check-runs. ## What slice 2 converts (25 files, 28 draws, each ledgered as `slice 2 of #24342`) - **Bespoke boot helpers that settle on `portDriftError()` and name `portContentionError()` on exit** (15 files). Spawn on `KERNEL_ASSIGNED_PORT`, settle on `readKernelAssignedPort(out, what)`, carry the port on the handle, and build each boot's base URL from it. Several of these reuse one drawn port across 2 to 8 boots. - `artifact-flag-precedence.integration` - `package-install-local-{boot-steps,handlers,hooks,jobs,jobs-pull,jobs-shared-name,uninstall-cleanups}.integration` - `package-restart-acceptance.integration` - `serve-config-boot-flat-docs.integration` - `serve-config-boot-residual.integration` (2: plus `bootRefused`) - `serve-host-config-metadata-door.integration` (2) - `serve-organization-record-page.e2e` - `serve-view-container-read.integration` - **Their own banner read-back, no drift check** (2): `serve-runtime-assets-branding-warning.e2e`, `start-signal-forwarding.e2e`. - **Copies of the `runServe()` shape** (3): - `serve-app-anchored-optional-import.e2e` (`runServeFrom`) - `serve-process-child-env.e2e` (`probeOrigin`'s default draw; its `readyVerdict` already handles a requested 0) - `serve-migrate-exit-schema-parity.integration` (migrate-and-exit) - **A negative arm** (1): `serve-port-drift-notice.e2e`'s NEGATIVE boot. `--port 0` never shifts, because `getAvailablePort(0)` answers 0. - **Deliberate draws, to re-ledger with their real reason rather than convert** (1): `serve-publishes-bound-port.e2e` (2). - One draw is a free, unbound port for the connect instrument's negative arm. - The other is the non-zero request that is the arm's subject, and that arm already tolerates the race through the child's own drift notice. - **`bin/run.js --dev` / `os dev`** (4): - `dev-boot-output-order.integration`: a pty command line that reads no port. - `serve-mcp-capability-collision.e2e`, `serve-mcp-stdio-answers.e2e`, `serve-stdio-stdout-purity.e2e`: a module-level drawn `port`, a local `boundPort()` and a local `portDriftError()`. Convert by reading the first boot's port back and dropping the local drift function. ## Acceptance notes - **A misleading warning on every kernel-assigned boot.** `os serve --port 0` prints `WARN Port 0 is in use, using port N instead` in the ready banner's *Boot diagnostics* block, at the default log level. - Measured on this head with tsx `serve --port 0` against `export default {};`: one such line at the default level and one at `info`, beside a banner that names the real port. - The source is `plugin-hono-server`'s `kernel:listening` hook, which compares the requested port (0) with the bound one. Port 0 is a legal request (`utils/port-contract.ts`, `MIN_PORT = 0`), not a busy port. - It does not change any verdict here: the noise budget counts it as informational and stays green. It is reported to the seat for filing, not fixed here (out of scope). - **`runServe()`'s new default is a default.** A test of the port doors themselves (`OS_PORT`, `PORT`, the built-in default) has to pass its port explicitly, and the docblock says so. No caller does that today: all 21 call sites passed `--port`. - **The nightly-tier readings are local.** Nine of the eleven files run nightly on `main`, not in this PR's `Test Core`, so their readings above are this machine's. --- _Generated by [Claude Code](https://claude.ai/code/session_01SYEJqDL3NXxrJRBVqaMu9T)_ --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent 741aa85 commit c76e849

12 files changed

Lines changed: 306 additions & 193 deletions

‎packages/cli/test/helpers/serve-process.ts‎

Lines changed: 66 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -212,26 +212,31 @@ function probeBind(want: number): number | null {
212212
* nobody re-examined the draw until a real run in this fleet went red on
213213
* `✗ Port 49402 is already in use`. **Do not write another one here.**
214214
*
215-
* ## Where the race is closed, and where this draw is still allowed (#24295)
215+
* ## Where the race is closed, and where this draw is still allowed (#24295, #24342)
216216
*
217217
* The residual race reached `main` once: an hourly run went red on
218218
* `start-mcp-stdio-early-initialize.integration.test.ts`, whose `os start`
219219
* child refused a port taken inside this gap. A production-posture child
220220
* refuses to auto-shift by design, so for that column the gap is a red, and it
221221
* is now CLOSED rather than narrowed: such a spawn passes
222222
* {@link KERNEL_ASSIGNED_PORT} and reads the port back with
223-
* {@link readKernelAssignedPort}, so no number crosses a gap at all.
224-
* `released-port-production-spawn.pin.test.ts` holds it: a file that names the
225-
* production-capable entry (`bin/run.js`) may call this or `randomPort()` only
226-
* as often as its ledger entry there allows — zero without one, and an entry is
227-
* a file whose every spawn of that entry is development posture.
223+
* {@link readKernelAssignedPort}, so no number crosses a gap at all. A
224+
* development-posture child hops instead, and `portDriftError()` reds the hop
225+
* just as surely, so that column closes the same way: `runServe()` hands its
226+
* child a kernel-assigned port whenever the caller passes no `--port`, and
227+
* returns the port the child bound (#24342).
228+
* `released-port-production-spawn.pin.test.ts` holds both: a file that spawns
229+
* the CLI may call this or `randomPort()` only as often as its ledger entry
230+
* there allows — zero without one.
228231
*
229232
* What may still call it, each with the race still real and unclosed there:
230233
*
231-
* • the DEVELOPMENT column — `runServe()` and every other child of
232-
* `bin/run-dev.js`, which pins `NODE_ENV=development` before argv is
233-
* parsed. A lost race there is a hop to the next port, not an exit, and
234-
* `portDriftError()` below is what turns that hop into a red;
234+
* • the development-column spawners slice 2 of #24342 has yet to convert —
235+
* the files that spawn `CLI` themselves (the `package-install-local-*`
236+
* `os start` family among them) and the four `bin/run.js --dev` / `os dev`
237+
* files, each ledgered in the pin with its exact count. A lost race there
238+
* is a hop to the next port, and `portDriftError()` below turns it into a
239+
* red on a correct tree;
235240
* • instruments that need a number NOTHING is listening on (a closed-port
236241
* probe), or that measure this draw itself (`serve-port-bind-probe`).
237242
*
@@ -312,10 +317,11 @@ export function holdPort(): Promise<{ port: number; release: () => Promise<void>
312317
*
313318
* ⚠️ The name is kept — and is now a slight misnomer, which is cheaper than the
314319
* alternative. It is `String(reservePort())` and nothing else; the draw, the
315-
* guarantees, the residual race and the columns it may still serve (the
316-
* development column and instruments — never a production-capable spawn) are
317-
* all documented on `reservePort()` above, and there is no second mechanism
318-
* hiding behind this name.
320+
* guarantees, the residual race and the callers it may still serve
321+
* (instruments, and the development-column files the pin's ledger names until
322+
* slice 2 of #24342 converts them — never another CLI spawn) are all
323+
* documented on `reservePort()` above, and there is no second mechanism hiding
324+
* behind this name.
319325
*/
320326
export function randomPort(): string {
321327
return String(reservePort());
@@ -551,10 +557,11 @@ export type BannerPortReadback =
551557
* the sentence that made this population look half its real size. Which files
552558
* are in the loud row is ⛔ not counted here either — an earlier count here
553559
* ("the ONLY file") went stale as `bin/run.js` spawners were added. The loud row
554-
* can only be reached through `bin/run.js`, so
555-
* `released-port-production-spawn.pin.test.ts` enumerates every file that names
556-
* it, and holds each of them to {@link KERNEL_ASSIGNED_PORT} unless its ledger
557-
* entry there says why not (#24295).
560+
* can only be reached through `bin/run.js`, but a lost race reds a correct tree
561+
* in the silent rows too, so `released-port-production-spawn.pin.test.ts`
562+
* enumerates every file that spawns the CLI — either entry, either posture —
563+
* and holds each of them to {@link KERNEL_ASSIGNED_PORT} unless its ledger entry
564+
* there says why not (#24295, #24342).
558565
*
559566
* And the port `runServe()` hands over is an opaque element of an argv list, so
560567
* until this read-back the harness never learned the child's real one. A test
@@ -592,7 +599,8 @@ export function boundPortFromBanner(output: string): BannerPortReadback {
592599
* the port between this harness's bind probe and the child's `listen()`. The
593600
* production column would die loudly, which is why it no longer draws (it takes
594601
* {@link KERNEL_ASSIGNED_PORT}, #24295); the development column drifts and gets
595-
* this — the race is still real there, and this is what makes it a red.
602+
* this. `runServe()` without a `--port` no longer draws either (#24342); the
603+
* spawners still ledgered in the pin do, and this is what makes their race a red.
596604
*
597605
* ## Why `unreadable` is an ERROR and not a skip
598606
*
@@ -609,7 +617,8 @@ export function boundPortFromBanner(output: string): BannerPortReadback {
609617
* `/Nothing to serve/` case does. Such a child announced no bound port, so
610618
* there is nothing to compare and this returns `null`. Every `runServe()`
611619
* caller that boots successfully today waits for `Press Ctrl+C to stop` — the
612-
* banner tail — so all of them are covered.
620+
* banner tail — so all of them are covered: by this check when the caller
621+
* passed its own `--port`, by {@link readKernelAssignedPort} when it passed none.
613622
*/
614623
export function portDriftError(
615624
output: string,
@@ -873,6 +882,13 @@ export function childEnv(
873882
export interface ServeRun {
874883
stdout: string;
875884
stderr: string;
885+
/**
886+
* The port the child BOUND, read back out of its own ready banner — the only
887+
* number a caller may address the child on. `undefined` when the run ended
888+
* without a complete banner (a boot that died, or a `waitFor` that matched
889+
* an earlier line, as `serve-no-artifact`'s refusals do).
890+
*/
891+
port: number | undefined;
876892
}
877893

878894
/**
@@ -922,9 +938,18 @@ function portOf(args: string[]): string | undefined {
922938
* free port and boots clean. Read the docblock there for why a green is the
923939
* more expensive outcome of the two.
924940
*
925-
* Only the SECOND needs the caller to have passed `--port`; both read the
941+
* Both apply only to a caller that passed `--port` itself; both read the
926942
* child's own output for the port they name, never the harness's expectation.
927943
*
944+
* ⭐ A caller that passes NO `--port` gets {@link KERNEL_ASSIGNED_PORT} (#24342):
945+
* the child's own `listen()` picks the port, so no released number crosses a
946+
* gap and there is nothing to contend for or drift from. The drift check then
947+
* becomes the read-back: a complete banner must name the non-zero port the
948+
* child bound ({@link readKernelAssignedPort} — an unreadable `API:` row or an
949+
* announced `localhost:0` rejects), and that port is the returned `port`.
950+
* ⛔ So a test of the port doors themselves (`OS_PORT`, `PORT`, the built-in
951+
* default) cannot ride this default; it passes the port it means explicitly.
952+
*
928953
* `waitFor` is matched against **stdout and stderr together** (#7915). `serve`
929954
* writes every human line — banner, boot progress, kernel logs — to stderr now,
930955
* because its stdout belongs to the MCP stdio transport when one is mounted;
@@ -941,8 +966,10 @@ export function runServe(
941966
// exercises use `??` and an empty string is not nullish.
942967
opts: { waitFor: RegExp; timeoutMs?: number; config?: string; env?: Record<string, string | undefined> },
943968
): Promise<ServeRun> {
969+
const requested = portOf(args);
970+
const argv = requested === undefined ? [...args, '--port', KERNEL_ASSIGNED_PORT] : args;
944971
return new Promise((resolveRun, rejectRun) => {
945-
const child = spawn(TSX, [CLI, 'serve', opts.config ?? 'objectstack.config.ts', ...args], {
972+
const child = spawn(TSX, [CLI, 'serve', opts.config ?? 'objectstack.config.ts', ...argv], {
946973
cwd,
947974
// `childEnv`, never a bare `...process.env` — see its header for the
948975
// measured reason (commit 1ddda1d00).
@@ -980,7 +1007,20 @@ export function runServe(
9801007
return;
9811008
}
9821009
const what = 'os serve (bin/run-dev.js, via runServe)';
983-
const contended = portContentionError(stdout + stderr, what, portOf(args));
1010+
const output = stdout + stderr;
1011+
if (requested === undefined) {
1012+
// Kernel-assigned (#24342): no `portContentionError()` — this child was
1013+
// handed no number to lose, so a port it could not bind is not that race.
1014+
if (boundPortFromBanner(output).state === 'no-banner') {
1015+
resolveRun({ stdout, stderr, port: undefined });
1016+
return;
1017+
}
1018+
const bound = readKernelAssignedPort(output, what);
1019+
if (bound instanceof Error) rejectRun(bound);
1020+
else resolveRun({ stdout, stderr, port: bound });
1021+
return;
1022+
}
1023+
const contended = portContentionError(output, what, requested);
9841024
if (contended) {
9851025
rejectRun(contended);
9861026
return;
@@ -989,12 +1029,13 @@ export function runServe(
9891029
// succeeded. `bin/run-dev.js` pins `NODE_ENV=development`, so the branch
9901030
// above (a loud refusal to bind) is not the one these children take; this
9911031
// is. See `portDriftError`.
992-
const drifted = portDriftError(stdout + stderr, what, portOf(args));
1032+
const drifted = portDriftError(output, what, requested);
9931033
if (drifted) {
9941034
rejectRun(drifted);
9951035
return;
9961036
}
997-
resolveRun({ stdout, stderr });
1037+
const readback = boundPortFromBanner(output);
1038+
resolveRun({ stdout, stderr, port: readback.state === 'bound' ? readback.port : undefined });
9981039
};
9991040

10001041
const timer = setTimeout(

0 commit comments

Comments
 (0)