Skip to content

test(cli): read back the port the e2e serve child actually bound, and fail loudly on a silent drift - #12546

Merged
os-litant merged 1 commit into
mainfrom
claude/issue-12525-runserve-port-readback
Aug 26, 2026
Merged

test(cli): read back the port the e2e serve child actually bound, and fail loudly on a silent drift#12546
os-litant merged 1 commit into
mainfrom
claude/issue-12525-runserve-port-readback

Conversation

@os-litant

Copy link
Copy Markdown
Collaborator

Fixes#12525

runServe() now reads the child's real bound port back out of its ready banner and rejects when it is not the port the harness asked for.

The false green, produced on demand

bin/run-dev.js:28 pins NODE_ENV=development before argv is parsed, so serve.ts:1318's portAutoShiftAllowed is true for every runServe() child: a taken port is not an error, getAvailablePort() walks to the next free one, and the boot succeeds. The harness never learned the child's real port — it was an opaque element of the argv it built — so a test that afterwards talks to the port it asked for reaches whatever else is holding it.

Measured on this branch, twice, with the requested port really held:

harness stateaskedchild boundrunServe()
drift verdict ablated4353743539RESOLVED✓ Server is ready, ➜ API: http://localhost:43539/
drift verdict present4197941980REJECTEDPORT DRIFT …

The ablated row is the defect, reproduced: a healthy banner, every assertion on it green, for a server nobody in the test knew the address of.

What the rejection says

PORT DRIFT on `os serve (bin/run-dev.js, via runServe)`: this harness asked for port 41979
and the child BOUND port 41980 — read back from the child's own ready banner.
`bin/run-dev.js` pins `NODE_ENV=development` before argv is parsed, so `serve.ts`'s
`portAutoShiftAllowed` is true for every child spawned through this helper: …
⛔ This is a HOST race, not a verdict about the code under test. Re-run this file in isolation.
⛔ And do not answer it by relaxing this check. …

os serve's auto-shift rule is unchanged. Auto-shifting in development is correct and deliberate; the defect was that the harness could not tell the difference. No file under packages/cli/src is touched.

⚠️ Correcting a wrong table before it lands anywhere

#12525's body — and the first draft of this helper's docblock — put "bin/run.js with NODE_ENV unset" in the loud column. That is measurably wrong, and the corrected table now lives in boundPortFromBanner's docblock:

serve.ts:1318 is flags.dev || process.env.NODE_ENV === 'development'either half alone opens the auto-shift branch. A spawner is loud only when both are false.

spawn shapetaken port ⇒
no --dev, NODE_ENV not developmenthard exit 1 — loud, portContentionError() names it
--dev, whatever NODE_ENVsilent drift, green boot
bin/run-dev.js (pins NODE_ENV at :28)silent drift, green boot

Re-derived locally: serve-mcp-stdio-answers, serve-mcp-capability-collision and serve-stdio-stdout-purity all spawn serve -p PORT **--dev** (:167, :172, :159), so the bin/run.js entry buys them nothing — they are in the silent column by the --dev route, and are handled by #12526 (⛔ untouched here). serve-node-env-production-default.e2e.test.ts is the only file in this directory that is actually loud.

⚠️ Ruling ③ — a caller that talks to the port, reported and NOT quietly fixed

packages/cli/test/serve-process-child-env.e2e.test.ts is not a runServe() caller, so this change does not reach it — but it is the same defect with live consequences:

  • :160spawn(TSX, [CLI, 'serve', 'objectstack.config.ts', '--port', port]), where CLI is the helper's bin/run-dev.js ⇒ auto-shift branch open.
  • :189 fetches http://localhost:${port}/api/v1/auth/sign-in/email ⇒ it talks to the port it asked for, never the one the child bound.
  • What it asserts there is a security posture (403 INVALID_ORIGIN vs 401), i.e. exactly the "assertion that can never go red for the reason it exists" class that childEnv()'s own docblock warns about. On a lost race the answer can come from a neighbouring agent's server.

packages/cli/test/serve-app-anchored-optional-import.e2e.test.ts:176 has the same blind spawn but only reads the child's output, so its exposure is lower. Both are reported for triage rather than folded in.

Surface

packages/cli/test/helpers/serve-process.ts (the read-back), plus one declared amendment: hold() is hoisted out of serve-port-bind-probe.test.ts into the helper as holdPort(), because a second file needs it and this directory has already paid once for duplicating a port instrument (the three blind draws reservePort() replaced). No behaviour change there — the same 9 tests pass.

Verification — all on c4474afa3, closure built

whatresult
vitest run the new file + serve-port-bind-probe2 files / 22 tests passed
vitest run all fiverunServe() callers5 files / 15 tests passed — none relied on the drift
pnpm lint (repo-wide, eslint . --no-inline-config)exit 0, 57s
check:nul-bytes · cli-test-child-env · engine-double-contract · where-matcher · query-options-erasure · type-check-coverage · cross-package-test-inputs · test-source-aliasall exit 0 on c4474afa3
hidden-test-tree tsc (the layer check:type-check-debt ratchets)146 errors — exactly the ledger's recorded count for @objectstack/cli, same code composition (TS7006×60, TS2835×57, TS2339×24, TS2307×3, TS18046×2), 0 of them naming any file in this diff

The last row is a declared narrowing: --re-measure re-runs all 34 ledger entries and needs the whole workspace built, so the one entry this diff can move was reconstructed instead — and reproducing the recorded number exactly is what makes it a measurement rather than a guess. CI runs the full farm.

The ablation restored under a trap and proved it: on-disk blob back to 936ea7c5… = the HEAD blob, injected marker count 0, git diff HEAD empty.

Test-only (packages/cli/package.json publishes dist, README.md, CHANGELOG.md — no test file ships), so skip-changeset.


Generated by Claude Code

@github-actions

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

10 anchor(s) derived from 1 changed package(s); no hand-written page names any of them, so this run has nothing to listnot a clean bill of health. This check sees only pages that NAME a derived anchor: one that documents this change in prose, or enumerates it in an authoring dialect, names none and stays invisible to it on every run.

What this run could not see
  • the SDK route bridge reached 47 of 219 client-bound route-ledger rows — the other 172 have no registrar path: tail to select them, so pages documenting THEIR client methods cannot appear above, on this or any run. Of those 172: 14 are remediable by widening that discovery convention (an in-repo file declares the path; the convention did not scan it); 56 are structural — on a ledger where NOT ONE row is declared in-repo, so no discovery change reaches them at any price; 102 are undecided (no in-repo declaration, on a ledger that has other in-repo registrars — absence and an unreadable spelling are not distinguishable here). The rows themselves: node scripts/docs-audit/affected-docs.mjs --bridge-coverage
  • a page that states a rule by its inputs shares no identifier with the emitter that implements the rule, so an emitter-only diff cannot list it — not on this run and not on any run. Measured on fix(driver-sql): emit varchar(maxLength) for a text field a declared index keys on #11430: content/docs/protocol/objectql/types.mdx documents the text-family column mapping by the ObjectQL type names it maps FROM (text / textarea / html) while the diff changed createColumn; it went unlisted, and it was the page that diff falsified, in four places. No shared token exists to detect this on, so a rule your change carries has to be re-read by hand in the pages that restate it.

Coarse fallback — 23 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): node scripts/docs-audit/affected-docs.mjs --json c0f5e8f21539f5299e3c5dec0e4039ed0387989cpackageMentionDocs.

Which tree this was computed on

This run read content/docs from 2548dd583c5c0620bfc4bcd34c88779204f44a0c — the merge of head c4474afa315dc2026836c861978e9cf2fe28827a into base c0f5e8f21539f5299e3c5dec0e4039ed0387989c, which is what actions/checkout gives a pull_request run. Not the PR head.

A worktree cut from an older main holds a different content/docs, so re-deriving there can legitimately return a different list — that is a different tree, not a wrong row. To answer on the same tree:

# while this PR is open — GitHub drops the merge commit once it closes
git fetch origin 2548dd583c5c0620bfc4bcd34c88779204f44a0c && git checkout 2548dd583c5c0620bfc4bcd34c88779204f44a0c
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin c0f5e8f21539f5299e3c5dec0e4039ed0387989c c4474afa315dc2026836c861978e9cf2fe28827a && git checkout -B drift-repro c0f5e8f21539f5299e3c5dec0e4039ed0387989c && git merge --no-ff c4474afa315dc2026836c861978e9cf2fe28827a
node scripts/docs-audit/affected-docs.mjs --json c0f5e8f21539f5299e3c5dec0e4039ed0387989c

⚠️ That checkout carried uncommitted changes, so the commit above does not fully identify what was read.

@os-litantos-litant added the skip-changeset PR has no user-facing published change; bypasses the changeset gate label Aug 26, 2026 — with Claude
@os-litant
os-litant marked this pull request as ready for review August 26, 2026 10:07
@os-litant
os-litant added this pull request to the merge queueAug 26, 2026
Merged via the queue into main with commit 52a9823Aug 26, 2026
37 checks passed
@os-litant
os-litant deleted the claude/issue-12525-runserve-port-readback branch August 26, 2026 10:25
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size/lskip-changesetPR has no user-facing published change; bypasses the changeset gatetests

Projects

None yet

1 participant

@os-litant