Skip to content

Commit 9185ff0

Browse files
Elon Muskclaude
andauthored
test(spec): let the publish-smoke THIEF own the port it calls held (#10684)
The "held from outside the registry" case staged its holder by picking a port, deleting the claim, then binding it. Between the `rm -f` and the stub's `listen` the port was claimed by nobody, so any concurrent `smoke_pick_free_port` caller scanning from 3210 could legitimately take it — and losing there fails as `STEAL_HELD != THIEF`, which reads as an accusation of the picker on a PR that never touched it. Port the shape PR #10456 landed for the sdui sibling: the stub binds `:0` and reports back the port the kernel gave it, so the port it holds is one it provably owns, and the pick-then-`rm` dance goes away entirely. The registry property gets plainer rather than weaker — a port bound straight from the ephemeral range was never claimed in any registry, so there is no claim to remove. The stub takes NO host argument, unlike the sdui sibling's `127.0.0.1`: `smoke_pick_free_port`'s probe binds the wildcard address because that is the spelling serve.ts's own isPortAvailable() uses. That asymmetry is load-bearing and recorded on #10261; it is preserved here, not converged. Claude-Session: https://claude.ai/code/session_01B4h3medzvhB9rpfoja9jcw Co-authored-by: Claude <noreply@anthropic.com>
1 parent 8f04d9a commit 9185ff0

1 file changed

Lines changed: 72 additions & 7 deletions

File tree

‎packages/spec/scripts/publish-smoke-port-collision.test.ts‎

Lines changed: 72 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -72,6 +72,24 @@
7272
// the cheap direction: before the fix, two picks in one run with nothing bound in
7373
// between both returned the base.
7474
//
75+
// ## An occupier must OWN the port it calls held — a reservation is not a bind
76+
//
77+
// The "outside the registry" case below used to stage its holder by picking a
78+
// port and deleting the claim, then binding it. Between the `rm -f` and the
79+
// stub's `listen` the port was claimed by nobody, so any concurrent
80+
// `smoke_pick_free_port` caller scanning from 3210 could legitimately take it —
81+
// and losing there does not fail as "the precondition evaporated", it fails as
82+
// `STEAL_HELD != THIEF`, which reads as an accusation of the picker on a PR
83+
// that never touched it. The sibling `gen-sdui-manifest-collision.test.ts` had
84+
// the same gap and measured it: an in-file racer won that bind 39 times in 80,
85+
// reddening three unrelated PRs in one afternoon (PR #10456).
86+
//
87+
// So the stub binds `:0` and reports back the port the kernel gave it. It is
88+
// already holding that port when it names it, which is exactly what STEAL_HELD
89+
// asserts, and no gap is left to race through. The registry property gets
90+
// PLAINER rather than weaker: a port bound straight from the ephemeral range
91+
// was never claimed in any registry, so there is no claim to remove.
92+
//
7593
// No `objectstack dev` boot and no scaffold: the contract under test belongs to
7694
// the shell script, and the ports are picked at run time by the script's own
7795
// helper so this test cannot collide with a concurrent agent — which would be a
@@ -109,6 +127,37 @@ const HTTP_STUB = [
109127
'}).listen(Number(process.env.STUB_PORT));',
110128
].join('');
111129

130+
/**
131+
* The same announcing stub, except it BINDS AN EPHEMERAL PORT and prints the
132+
* one the kernel gave it on stdout.
133+
*
134+
* Bind first, name second. Taking a port from the picker and binding it
135+
* afterwards leaves a window in which anything scanning the same base can
136+
* legitimately take it, and losing there evaporates the precondition the
137+
* STEAL assertions rest on — see the header. Port 0 closes that window by
138+
* construction: the number is written from inside the `listening` callback, so
139+
* by the time the harness can read it the socket is already held.
140+
*
141+
* ⛔ No host argument — deliberately, and NOT the `127.0.0.1` the sdui sibling
142+
* uses. `smoke_pick_free_port`'s probe binds the WILDCARD address because that
143+
* is the spelling `serve.ts`'s own `isPortAvailable()` uses, so an occupier
144+
* here has to hold the wildcard address for that probe to see it at all. The
145+
* asymmetry between the two files is load-bearing and is recorded on #10261:
146+
* do not converge them.
147+
*/
148+
const OWNED_HTTP_STUB = [
149+
'const http = require("node:http");',
150+
'const s = http.createServer((_q, r) => {',
151+
' r.writeHead(200, { "content-type": "application/json" });',
152+
' r.end(JSON.stringify({ iam: process.env.STUB_NAME }));',
153+
'});',
154+
// Losing the bind must EXIT, not throw: an unhandled 'error' event would kill
155+
// the harness with a stack trace at the point a vacuity guard is supposed to
156+
// name the problem. Near-unreachable on `:0`, and kept for that reason.
157+
's.once("error", () => process.exit(1));',
158+
's.listen(0, () => console.log(s.address().port));',
159+
].join('');
160+
112161
/**
113162
* Run a bash harness that SOURCES the real script (so the real functions run)
114163
* and prints `KEY=value` lines. Returns them parsed.
@@ -124,6 +173,7 @@ function runHarness(body: string[]): Record<string, string> {
124173
`export SMOKE_ROOT=${JSON.stringify(dir)}`,
125174
'export SMOKE_KEEP=1',
126175
`export STUB=${JSON.stringify(HTTP_STUB)}`,
176+
`export OWNED_STUB=${JSON.stringify(OWNED_HTTP_STUB)}`,
127177
// Sourcing defines the helpers and runs nothing.
128178
`source ${JSON.stringify(SCRIPT)}`,
129179
// The script's own `set -euo pipefail` came with it. Several steps below
@@ -305,21 +355,36 @@ describe.skipIf(!RUNNABLE)('[#9647] publish-smoke.sh smoke-tests its OWN dev ser
305355
it('skips a port held from outside the registry and hands the claim back', () => {
306356
const r = runHarness([
307357
'export SMOKE_PORT_RESERVATION_DIR="$SMOKE_ROOT/reg"',
308-
// Take a port, then forget the claim — now the port is held by something
309-
// the registry knows nothing about, which is the one case a claim cannot
310-
// cover and the probe still has to.
311-
'STEAL="$(smoke_pick_free_port 3210)"',
312-
'rm -f "$SMOKE_PORT_RESERVATION_DIR/$STEAL"',
313-
'STUB_PORT="$STEAL" STUB_NAME=THIEF node -e "$STUB" >/dev/null 2>&1 & THIEF=$!',
314-
'sleep 1',
358+
// A port held by something the registry knows nothing about — the one
359+
// case a claim cannot cover and the probe still has to. The thief takes
360+
// it straight from the ephemeral range and reports what it got, so being
361+
// outside the registry is a property of HOW IT GOT THE PORT rather than
362+
// something staged by picking a port and then deleting its claim. That
363+
// staging is what left the port unclaimed across the bind; see the
364+
// header. Nothing to remove now, so nothing to lose the port during.
365+
'THIEF_FILE="$SMOKE_ROOT/thief.port"',
366+
'STUB_NAME=THIEF node -e "$OWNED_STUB" > "$THIEF_FILE" 2>/dev/null & THIEF=$!',
367+
// The port appears only once the socket is bound, so waiting for the
368+
// number IS waiting for the hold — there is no second thing to wait for,
369+
// and no fixed sleep to be wrong about on a loaded container.
370+
'STEAL=""',
371+
'for _ in $(seq 1 40); do STEAL="$(tr -d "[:space:]" < "$THIEF_FILE" 2>/dev/null || true)"; [ -n "$STEAL" ] && break; sleep 0.25; done',
315372
'echo "STEAL_BASE=$STEAL"',
316373
'echo "STEAL_HELD=$(curl -sS "http://localhost:$STEAL/" | jq -r .iam)"',
374+
// An ephemeral base cannot scan off the end of the port space: the first
375+
// candidate is the port the thief holds, the second is free, and the scan
376+
// returns there rather than walking its whole 200-port span upward.
317377
'PICK="$(smoke_pick_free_port $STEAL)"',
318378
'echo "STEAL_PICK=$PICK"',
319379
'echo "STEAL_CLAIM_RELEASED=$([ -e "$SMOKE_PORT_RESERVATION_DIR/$STEAL" ] && echo no || echo yes)"',
320380
'echo "STEAL_CLAIM_ON_PICK=$([ -e "$SMOKE_PORT_RESERVATION_DIR/$PICK" ] && echo yes || echo no)"',
321381
'kill "$THIEF" 2>/dev/null',
322382
]);
383+
// The base is now reported BY THE THIEF rather than returned by the picker,
384+
// so "is it a number at all" is a question this case did not used to have.
385+
// Not ceremony: with STEAL empty, `smoke_pick_free_port` falls back to its
386+
// own 3210 default and STEAL_PICK then differs from STEAL_BASE for free.
387+
expect(r.STEAL_BASE).toMatch(/^\d+$/);
323388
// Vacuity guard: the thief really held the port.
324389
expect(r.STEAL_HELD).toBe('THIEF');
325390
expect(r.STEAL_PICK).not.toBe(r.STEAL_BASE);

0 commit comments

Comments
 (0)