Uh oh!
There was an error while loading. Please reload this page.
test(cli): read the bound port back in the two direct-spawn serve e2e files - #12565
Merged
Merged
Conversation
Contributor
📓 Docs Drift CheckNothing in this diff resolved to a documentable surface (no symbol, route or SDK anchor derived from 0 changed package(s)), so this run has no opinion about the docs. What this run could not see
Coarse fallback — 0 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): |
os-litant
marked this pull request as ready for review
August 26, 2026 11:14
This was referenced Aug 26, 2026
os-litant
enabled auto-merge
August 26, 2026 11:28
This was referenced Aug 26, 2026
Open
Uh oh!
There was an error while loading. Please reload this page.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes#12548
Two e2e files in
packages/cli/testspawnos servedirectly — so #12525's read-back,which lives inside
runServe(), never reaches them. Both now read the port the childactually bound out of its own ready banner and refuse a mismatch, using the helpers
PR #12546 landed. ⛔ No second implementation, ⛔ no change to
os serve's auto-shift rule(#11113 pins the production half; dev auto-shift is correct product behaviour), ⛔ neither
file routed through
runServe().1.
serve-process-child-env.e2e.test.ts— the priority fileIt is the only file in this directory where a drifted port lets a foreign process answer a
SECURITY assertion.
probeOrigin()POSTs to/api/v1/auth/sign-in/emailwith an untrustedOriginand asserts the origin-check posture —403 INVALID_ORIGINagainst401 INVALID_EMAIL_OR_PASSWORD. Until this PR that request went to the port the harnessasked for. Its children spawn through
bin/run-dev.js, which pinsNODE_ENV=developmentbefore argv is parsed, so
serve.ts'sportAutoShiftAllowedis TRUE: a taken port is not anerror, it is a hop to the next free one, and the boot then SUCCEEDS.
Reproduced end to end on this container, against the real
os serveAn HTTP server was made to hold the requested port and answer every request
401 {"code":"INVALID_EMAIL_OR_PASSWORD", …}:⭐ Note which leg goes green: the pre-#11267 control leg, whose whole job is to prove the
TESTleak accepts an untrusted origin, is satisfied by a stranger. That is a false greenon the assertion this file exists to make.
The gate settles on having SEEN the port, not on a ready marker
The old gate was
/Press Ctrl\+C to stop|Server is ready/.Server is readyis the banner'sHEAD, printed one
console.errorahead of theAPI:row that carries the port, whileboundPortFromBanner()keys on the TAIL — so a head-keyed gate can resolve with the portstill unknown,
portDriftError()then answersnull, and the comparison compares nothing.Whether it did would depend on how the pipe happened to chunk. So the gate is now
readyVerdict(), which returns{ settled: false }forno-bannerand only ever hands backa port the child itself named; the fetch addresses that port, not the requested one (they
are provably the same number on that line — a mismatch was refused before reaching it).
A cheap unit pin holds the gate itself:
readyVerdict('\n ✓ Server is ready\n', …)must be{ settled: false }.Proof it can say no — and that the proof is load-bearing
A third real boot was bought deliberately:
holdPort()holds the requested port, the childdrifts, and the harness is asserted to reject with
PORT DRIFT. ⛔ Nothing is simulated.Ablation, to show that arm is not decoration — the refusal was disabled in place
(
portDriftError(output, WHAT, requestedPort)→… , undefined), which the helper documentsas "no expectation stated, no verdict to give"):
Predicted direction before running: RED, on exactly one test. Observed: exactly that.
⚠️ No rebuild leg is reported because none exists here: vitest loads the mutated file from
source and
bin/run-dev.jsruns the CLI fromsrc/through tsx, so nothing in the mutation'sresolution path goes via a
dist/. The mutation is evidenced by blob hash instead, in bothdirections. (A first attempt at this ablation exited 127 — a bad runner path, not a red gate;
it was re-run through
pnpm exec vitestand only that second run is reported.)2.
serve-app-anchored-optional-import.e2e.test.ts— the sibling, and a measured correctionSame blind direct spawn; the read-back is wired in beside the existing
⚠️ But the card's "lower exposure" is, measured on
portContentionError().52a982388, nil — and the PR says sorather than letting a silent call read as an instrument:
console.errors its marker and registers nothing, soservewalkson to
✗ Cluster driver "redis" is not registeredand the child exits 1 at ~5.6s, havingnever called
listen(). The undeclared leg dies earlier still.boundPortFromBanner()answersno-banneron all three runs, andportDriftError()returnsnull. The same measurementretires the existing
portContentionError()to insurance: these children spawn throughbin/run-dev.js, so auto-shift is open and a taken port never produces the bind failure thathelper reads for.
so a drift cannot corrupt them.
⛔ It is deliberately not given the priority file's gate: no child here prints a banner, so
gating resolution on one would hang out the timeout. Both refusals stay as insurance for the
day this fixture registers a real driver and boots to a banner. The file's header now carries
that measurement so the next drift sweep does not re-file it.
inert port apparatus should stay, or whether the fixture should be made to reach a listening
server, is a separate call and a separate card.
Verification — everything below ran at
177c0d1db, which is this branch's headserve-port-readback.e2e.test.tsis included as the control on the helpers being consumed.Every heavy run went through
scripts/pm/os-verify-lock.sh; the verdicts above are the lock'sown VERDICT lines, not a bare
$?.Gates, each read from the gate's own verdict line (all exit 0):
check:nul-bytesOK (scanned 6925 text file(s) … no raw ASCII control bytes)check:cli-test-child-env35 spawner source(s) among 97 … all 41 spawn call(s) declare their child's envcheck:engine-double-contractOK — 416 pinned, 133 in the DEBT ledger, 2 exemptcheck:where-matcher303 matcher(s) discovered, 303 answer … none newcheck:query-options-erasureratchet holds: 67 unswept non-test site(s) … none newcheck:cross-package-test-inputsOK: 20 package(s) read outside themselves, all declaredcheck:test-source-aliasOK — 72 packages with tests scannedThe family list was re-derived from the real change set, not from the dispatch:
node scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack→ exit 0, 18 matchedfamilies over the 2 changed paths.
Type reading.⚠️
packages/cli'stypecheckistsc --noEmitover a tsconfig whoseincludeis["src"], so it covers none ofpackages/cli/testand is not claimed ascoverage here. The two changed files were compiled directly against that same config with
includewidened to them: exit 0, and membership proved with--listFiles(240 files inthe program; both changed files and
test/helpers/serve-process.tspresent by absolute path).Lint — a declared narrowing, with its evidence.
pnpm lint(eslint . --no-inline-config)is a repo-wide scan CI owns; it was narrowed here to the changed files, and the narrowing is a
measurement rather than a skip on three counts:
git diff --name-only 52a982388— 2 paths — nota guess about what matters.
--format jsonoutput: 2 files linted, 0 errors,0 warnings, exit 0. Neither file was ignored by the config.
eslint.config.mjsnever enablestype-aware linting for any file (no
parserOptions.project, no typed@typescript-eslintrules) — its own header states and measures this — so no file's verdictcan depend on another file's contents.
Test-only diff (2 files under
packages/cli/test/, no package source, no user-visible change)⇒
skip-changeset.Generated by Claude Code