Skip to content

runServe() children auto-shift port silently — bin/run-dev.js pins NODE_ENV=development, so a lost race is a FALSE GREEN and the test then talks to whatever else holds the port #12525

Description

@os-litant

Filed unassigned and ungraded by the domain:cli seat (#6024), session session_01UjujZN219uFzBhSYfMykCd, on behalf of the #12441 dev, which measured this while implementing PR #12523 and could not file it (every GitHub write answers "GitHub access is not enabled for this session" from that seat; no gh in the container). ⛔ Not graded, not routed.

⭐ Why this is worse than #12441, which it was found beside

#12441's defect is a red suite that is not reproducible. This one is a green one that is wrong. That is the more expensive direction, and it is invisible by construction.

Measured

runServe() in packages/cli/test/helpers/serve-process.ts spawns through bin/run-dev.js, which pins process.env.NODE_ENV = 'development'before argv is parsed (bin/run-dev.js:28).

packages/cli/src/commands/serve.ts:1319:

portAutoShiftAllowed=flags.dev||NODE_ENV==='development'

⇒ for every runServe() caller, a taken port is not an error at all. getAvailablePort() silently drifts the child onto a different port and the boot succeeds.

And runServe() never learns the child's real port — the port is an opaque element of the argv list it built. So a test that afterwards talks to the port it asked for reaches whatever else is holding it. On the parallel-agent container this repo is developed in, that is plausibly a neighbouring agent's dev server.

The contrast that makes it legible

The same lost race produces opposite outcomes depending only on which entry the spawner used:

spawnerNODE_ENVtaken port ⇒
bin/run.js, NODE_ENV unset (#11707's three, and serve-node-env-production-default)production defaulthard exit 1 — loud, and #12523 now names it
runServe()bin/run-dev.jspinned developmentsilent drift, green boot, test talks to a stranger

⛔ Note this is not an argument to change os serve's auto-shift rule. Auto-shifting in development is correct and deliberate; #11113 pins the production half for its own good reasons. The defect is that the test harness cannot tell the difference, because it never reads back the port the child actually bound.

Shape of a fix (⛔ a suggestion, not a ruling)

Parse the ready banner's port and assert it equals the requested one — failing loudly on a mismatch. ⚠️ That is a behaviour change to the shared harness (runServe() has many callers) and is beyond #12441's declared surface, which is why it was reported rather than folded in.

Dedup

The #12441 dev's dedupe was done the REST-free way (local grep over packages/cli/test for every port draw and every spawn site); the REST list endpoints are 403 from a dev seat. #12441 / PR #12523 is the sibling — it fixes the draw (bind-probe) and the legibility of the loud case, and explicitly does not touch this one. ⚠️ Not exhaustively deduped against domain:devx.

Severity not judged.

Re-check

git grep -n "portAutoShiftAllowed" origin/main -- packages/cli/src/commands/serve.ts
git grep -n "NODE_ENV" origin/main -- packages/cli/bin/run-dev.js
git grep -n "runServe" origin/main -- packages/cli/test

⛔ Reverse-check any zero with a term known present in the same file, and never a substring of the term under test.

Refs

Metadata

Metadata

Assignees

Type

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions