Skip to content

Commit 10b3453

Browse files
claude[bot]claude
andauthored
fix(scripts): gen-sdui-manifest picks a per-run port and proves the server it dumps is its own (#9578) (#9670)
The script named a fixed port (5180) and a fixed log path for per-run state. Agent dispatch containers run several agents against one filesystem and one network namespace, so both were shared between concurrent runs. Measured on the pinned vite (8.2.1) with the requested port already held: vite prints "Port N is in use, trying another one..." and binds N+1. Run B's server came up on the neighbour port while run B's wait loop and BASE_URL still named N, so run B curled run A's server, got 200, and dumped A's manifest as its own with exit 0 — output that then feeds the ADR-0082 declaration-parity ratchet. --strictPort alone does not fix it (measured): run B's vite exits into run B's own log while run A keeps answering, and a probe that only asks "does the port answer?" still accepts it. So both halves ship — the flag, and a probe that requires the session this run spawned to still be alive before it accepts an answer on that port. Claude-Session: https://claude.ai/code/session_01XqDQYVU5smx29ts9pAErja Co-authored-by: Claude <noreply@anthropic.com>
1 parent 3b3f67d commit 10b3453

2 files changed

Lines changed: 370 additions & 8 deletions

File tree

Lines changed: 212 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,212 @@
1+
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
2+
//
3+
// Pins the CONCURRENT-RUN contract of `scripts/gen-sdui-manifest.sh` — the half
4+
// that decides whether the manifest this script writes describes THIS tree.
5+
//
6+
// ## What was measured, and why the obvious fix is only half of one
7+
//
8+
// Agent dispatch containers run several agents against one filesystem and one
9+
// network namespace. The script used to name a fixed port (5180) and a fixed log
10+
// path, so two overlapping runs shared both. Measured against the pinned vite
11+
// (8.2.1) with the requested port already held:
12+
//
13+
// $ vite dev --port 5390
14+
// Port 5390 is in use, trying another one...
15+
// ➜ Local: http://localhost:5391/
16+
//
17+
// vite AUTO-INCREMENTS. So run B's server came up on 5391 while run B's wait
18+
// loop and BASE_URL still named 5390 — run B curled run A's server, got 200,
19+
// and dumped A's manifest as its own with exit 0. The output is a real manifest
20+
// and nothing in the run says which tree it describes, which is what makes it
21+
// worse than no manifest: it feeds the ADR-0082 declaration-parity ratchet.
22+
//
23+
// `--strictPort` alone does NOT fix that, and this was measured too rather than
24+
// reasoned: with it, run B's vite exits (`Error: Port 5390 is already in use`)
25+
// into run B's OWN log, while run A keeps answering on the port. A probe that
26+
// asks only "does the port answer?" still gets its 200 and still dumps A's tree.
27+
// So the flag is necessary — it makes "the port we asked for" and "the port we
28+
// got" the same port or no port — but the probe has to ALSO require that the
29+
// server answering is the one this run spawned. Both halves, or neither works.
30+
//
31+
// ## Why these are executed assertions and not greps
32+
//
33+
// A grep for `--strictPort` passes against a file that only mentions the flag in
34+
// a comment, and a grep for "liveness" passes against a check that runs in the
35+
// wrong order. So the flag is asserted through `sdui_dev_server_cmd`, the
36+
// function the script itself builds its argv from, and the refusal is asserted
37+
// by standing up a real neighbouring server on the port and watching the real
38+
// wait function turn it down.
39+
//
40+
// The vacuity guards matter as much as the assertions. `NEIGHBOUR_BODY` proves
41+
// the neighbour was genuinely reachable at the same `http://localhost:<port>/`
42+
// spelling the script probes, and `OUR_LEADER_ALIVE` proves our stand-in server
43+
// was genuinely gone. Without those two lines a green "refused" could mean
44+
// nothing was listening and nothing was checked — the phantom-assertion failure
45+
// the cleanup test's header records paying for once already.
46+
//
47+
// No vite and no console build: the contract under test is one the shell script
48+
// owns, and the ports are picked at run time by the script's own helper so this
49+
// test cannot collide with a concurrent agent — which would be a poor look here.
50+
51+
import { describe, it, expect } from 'vitest';
52+
import { execFileSync } from 'node:child_process';
53+
import fs from 'node:fs';
54+
import os from 'node:os';
55+
import path from 'node:path';
56+
import { fileURLToPath } from 'node:url';
57+
58+
const HERE = path.dirname(fileURLToPath(import.meta.url));
59+
const SCRIPT = path.resolve(HERE, '..', '..', '..', 'scripts', 'gen-sdui-manifest.sh');
60+
61+
function have(bin: string): boolean {
62+
try {
63+
execFileSync('sh', ['-c', `command -v ${bin}`], { stdio: 'ignore' });
64+
return true;
65+
} catch {
66+
return false;
67+
}
68+
}
69+
70+
// Linux-only by construction, like the cleanup test beside it: the failure being
71+
// pinned is an agent-container one, and the helpers read process sessions.
72+
const RUNNABLE =
73+
process.platform === 'linux' && ['setsid', 'pgrep', 'curl', 'node'].every(have);
74+
75+
/** A tiny HTTP server on $SDUI_TEST_PORT, as a `node -e` program. */
76+
const HTTP_STUB = [
77+
'const http = require("node:http");',
78+
'http.createServer((_q, r) => { r.writeHead(200); r.end("SERVER"); })',
79+
' .listen(Number(process.env.SDUI_TEST_PORT), "127.0.0.1");',
80+
].join('');
81+
82+
function runHarness(): Record<string, string> {
83+
const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'sdui-collision-'));
84+
const harness = path.join(dir, 'harness.sh');
85+
86+
fs.writeFileSync(
87+
harness,
88+
[
89+
'#!/usr/bin/env bash',
90+
// Not `set -e`: several steps below are expected to fail, and their exit
91+
// codes are the measurement.
92+
'set -uo pipefail',
93+
// Sourcing runs no generation — the script returns right after defining
94+
// its helpers, so this exercises the REAL functions.
95+
`source ${JSON.stringify(SCRIPT)}`,
96+
`DIR=${JSON.stringify(dir)}`,
97+
'NODE_BIN="$(command -v node)"',
98+
'',
99+
'# ── 1. the argv the script actually spawns ──────────────────────────',
100+
'readarray -t ARGV < <(sdui_dev_server_cmd 4321)',
101+
'printf "DEV_ARGV=%s\\n" "${ARGV[*]}"',
102+
'',
103+
'# ── 2. the free-port search skips a port that is taken right now ────',
104+
'BUSY="$(sdui_pick_free_port 5180)"',
105+
'export SDUI_TEST_PORT="$BUSY"',
106+
`"$NODE_BIN" -e 'require("node:net").createServer().listen(Number(process.env.SDUI_TEST_PORT), "127.0.0.1")' &`,
107+
'BUSY_PID=$!',
108+
// disown: otherwise bash prints its own "Killed" job notice when this is
109+
// reaped below, which reads like a test failure in the vitest output.
110+
'disown "$BUSY_PID" 2>/dev/null || true',
111+
'for _ in $(seq 1 40); do',
112+
' "$NODE_BIN" -e \'const n=require("node:net");const s=n.createServer();s.once("error",()=>process.exit(0));s.once("listening",()=>s.close(()=>process.exit(1)));s.listen(Number(process.env.SDUI_TEST_PORT),"127.0.0.1")\' && break',
113+
' sleep 0.25',
114+
'done',
115+
'printf "BUSY_PORT=%s\\n" "$BUSY"',
116+
'printf "PICKED_WITH_BUSY=%s\\n" "$(sdui_pick_free_port "$BUSY")"',
117+
'kill -KILL "$BUSY_PID" 2>/dev/null',
118+
'',
119+
'# ── 3. a NEIGHBOUR answering on our port is refused, not accepted ───',
120+
'NPORT="$(sdui_pick_free_port 5180)"',
121+
'export SDUI_TEST_PORT="$NPORT"',
122+
`"$NODE_BIN" -e ${JSON.stringify(HTTP_STUB)} &`,
123+
'NEIGHBOUR_PID=$!',
124+
'disown "$NEIGHBOUR_PID" 2>/dev/null || true',
125+
'for _ in $(seq 1 40); do curl -sf "http://localhost:$NPORT/" > /dev/null 2>&1 && break; sleep 0.25; done',
126+
// Vacuity guard: the neighbour must really be answering, at the exact
127+
// spelling the script probes, or "refused" below proves nothing.
128+
'printf "NEIGHBOUR_BODY=%s\\n" "$(curl -sf "http://localhost:$NPORT/" || echo NONE)"',
129+
// Stands in for `vite --strictPort` losing the race: our own server exits
130+
// immediately, leaving the neighbour holding the port.
131+
'PF="$DIR/lost.pid"',
132+
'sdui_spawn_detached "$PF" "$DIR/lost.log" "$NODE_BIN" -e "process.exit(1)"',
133+
'sleep 1',
134+
// Vacuity guard: ours must really be gone before the refusal means anything.
135+
'printf "OUR_LEADER_ALIVE=%s\\n" "$([ -n "$(sdui_live_pids "$(cat "$PF")")" ] && echo yes || echo no)"',
136+
'START=$SECONDS',
137+
'if sdui_wait_for_own_server "$PF" "$NPORT" 20; then',
138+
' printf "ACCEPTED_NEIGHBOUR=yes\\n"',
139+
'else',
140+
' printf "ACCEPTED_NEIGHBOUR=no\\n"',
141+
'fi',
142+
'printf "REFUSAL_SECONDS=%s\\n" "$((SECONDS - START))"',
143+
'kill -KILL "$NEIGHBOUR_PID" 2>/dev/null',
144+
'',
145+
'# ── 4. our OWN server is accepted (the check is not just "always no") ─',
146+
'OPORT="$(sdui_pick_free_port 5180)"',
147+
'export SDUI_TEST_PORT="$OPORT"',
148+
'PF2="$DIR/own.pid"',
149+
`sdui_spawn_detached "$PF2" "$DIR/own.log" "$NODE_BIN" -e ${JSON.stringify(HTTP_STUB)}`,
150+
'if sdui_wait_for_own_server "$PF2" "$OPORT" 20; then',
151+
' printf "ACCEPTED_OWN=yes\\n"',
152+
'else',
153+
' printf "ACCEPTED_OWN=no\\n"',
154+
'fi',
155+
'sdui_stop_detached "$PF2"',
156+
'',
157+
'# ── 5. no fixed per-run state left in the file ──────────────────────',
158+
// A spelling pin, and labelled as one: the behaviour above is what the
159+
// suite really asserts. This exists so a fixed path cannot be reintroduced
160+
// quietly, since nothing else in the run would notice until two agents
161+
// overlapped again.
162+
`printf "FIXED_LOG_LITERALS=%s\\n" "$(grep -c '/tmp/sdui-dump-dev\\.log' ${JSON.stringify(SCRIPT)} || true)"`,
163+
].join('\n'),
164+
{ mode: 0o755 },
165+
);
166+
167+
const out = execFileSync('bash', [harness], { encoding: 'utf8', timeout: 120_000 });
168+
const parsed: Record<string, string> = {};
169+
for (const line of out.split('\n')) {
170+
const m = /^([A-Z_]+)=(.*)$/.exec(line.trim());
171+
if (m) parsed[m[1]] = m[2];
172+
}
173+
fs.rmSync(dir, { recursive: true, force: true });
174+
return parsed;
175+
}
176+
177+
describe.skipIf(!RUNNABLE)('gen-sdui-manifest.sh concurrent-run contract', () => {
178+
const seen = RUNNABLE ? runHarness() : ({} as Record<string, string>);
179+
180+
it('asks vite for a port it must actually get, or none at all', () => {
181+
// Read off the function the script builds its real argv from, so this cannot
182+
// pass against a file that merely mentions the flag.
183+
expect(seen.DEV_ARGV).toContain('--strictPort');
184+
expect(seen.DEV_ARGV).toContain('--port 4321');
185+
});
186+
187+
it('picks a per-run port, skipping one that is already taken', () => {
188+
expect(seen.BUSY_PORT).toMatch(/^\d+$/);
189+
expect(seen.PICKED_WITH_BUSY).toMatch(/^\d+$/);
190+
expect(seen.PICKED_WITH_BUSY).not.toBe(seen.BUSY_PORT);
191+
});
192+
193+
it("refuses a neighbouring run's server answering on this run's port", () => {
194+
// Both guards first: without them a green here can mean "nothing was
195+
// listening and nothing was checked".
196+
expect(seen.NEIGHBOUR_BODY).toBe('SERVER');
197+
expect(seen.OUR_LEADER_ALIVE).toBe('no');
198+
199+
expect(seen.ACCEPTED_NEIGHBOUR).toBe('no');
200+
// And it says so at once rather than after the full 90s wait, because the
201+
// answer never depended on waiting longer.
202+
expect(Number(seen.REFUSAL_SECONDS)).toBeLessThan(10);
203+
});
204+
205+
it('accepts the server this run started', () => {
206+
expect(seen.ACCEPTED_OWN).toBe('yes');
207+
});
208+
209+
it('keeps no fixed dev-server log path (spelling pin)', () => {
210+
expect(seen.FIXED_LOG_LITERALS).toBe('0');
211+
});
212+
});

0 commit comments

Comments
 (0)