Skip to content

Commit f93047a

Browse files
os-steveclaude[bot]claude
authored
fix(scripts): publish-smoke targets the dev server this run started, not the port it asked for (#9782)
* fix(scripts): publish-smoke targets the dev server this run started, not the port it asked for (#9647) `scripts/publish-smoke.sh` picked its dev-server port from a fixed default (3210) and derived BASE_URL from it. Agent dispatch containers run several agents against one filesystem and one network namespace, so that default was shared state between concurrent runs. Measured with a neighbour holding the requested port: $ objectstack dev --port 34217 --fresh ↪ server bound to port 34218 (requested 34217) $ curl http://localhost:34217/api/v1/health → 200, the neighbour's body $ curl http://localhost:34218/api/v1/health → 200, ours `objectstack dev` auto-shifts (serve.ts gates it on `flags.dev`, and `dev` always spawns `serve --dev`). So run B's app came up on the neighbour port while run B's wait loop and BASE_URL still named the requested one, and every auth and CRUD probe ran against run A's app. The sibling fix in gen-sdui-manifest.sh does not transfer. There is no `--strictPort` equivalent a caller can opt into, and a liveness check on our own spawn was ALREADY in this wait loop — it passes throughout the measurement above, because our server did not die, it succeeded somewhere else. So the script reads the port its own server actually bound, from the runtime state file serve.ts publishes under OS_HOME for external supervisors, in an OS_HOME this run can prove is its own because it pins the dev child's TMPDIR. A per-run free port ships alongside it to make the shift rare; an explicit SMOKE_PORT is still passed through exactly, with no search around it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XqDQYVU5smx29ts9pAErja * chore(ci): declare serve.ts as a spec test input for the publish-smoke collision test (#9647) check-cross-package-test-inputs flags the new test: it names packages/cli/src/commands/serve.ts, and no declared glob covered it. Declared rather than reworded, following the three precedents recorded in the gate itself (check-nul-bytes.mjs, sync-template-versions.mjs, the realtime protocol page): the literal collector takes quoted paths without parsing, so a mention forces a declaration, and declaring the file is cheaper than rewording prose to dodge a scanner. One file, not the commands tree — the test reads publish-smoke.sh and nothing else. turbo.json mirrors the glob, which the gate also verifies. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XqDQYVU5smx29ts9pAErja * test(scripts): the collision harness must not leak listeners when a step exits early (#9647) Observed after a run: two stubs orphaned on low ports, holding them in a container several agents share — the exact collision this file is about. Cause: `fail` inside the sourced script calls `exit` directly, so a harness step that is EXPECTED to fail leaves before its explicit `kill` line. An EXIT trap over `jobs -p` covers that path and every other way the harness can leave. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XqDQYVU5smx29ts9pAErja --------- Co-authored-by: claude[bot] <209825114+claude[bot]@users.noreply.github.com> Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
1 parent 55d2993 commit f93047a

4 files changed

Lines changed: 446 additions & 18 deletions

File tree

Lines changed: 240 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,240 @@
1+
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
2+
//
3+
// Pins the CONCURRENT-RUN contract of `scripts/publish-smoke.sh` — the half that
4+
// decides whether the app this gate smoke-tests is the one this run built.
5+
//
6+
// ## What was measured
7+
//
8+
// Agent dispatch containers run several agents against one filesystem and one
9+
// network namespace, so the script's fixed default port (3210) was shared state
10+
// between overlapping runs. `objectstack dev` AUTO-SHIFTS off a busy port —
11+
// `packages/cli/src/commands/serve.ts` gates that on `flags.dev`, and `dev`
12+
// always spawns `serve --dev`. Measured with a neighbour holding 34217:
13+
//
14+
// $ objectstack dev --port 34217 --fresh
15+
// ↪ server bound to port 34218 (requested 34217)
16+
// $ curl http://localhost:34217/api/v1/health → 200, the NEIGHBOUR's body
17+
// $ curl http://localhost:34218/api/v1/health → 200, ours
18+
//
19+
// So run B's app came up on the neighbour port while run B's wait loop and
20+
// BASE_URL still named 34217: run B ran every auth and CRUD probe against run
21+
// A's app.
22+
//
23+
// ## Why the sibling fix does not transfer, which is what this test exists for
24+
//
25+
// `scripts/gen-sdui-manifest.sh` (the same defect, one file over) shipped
26+
// `--strictPort` plus a probe requiring the session that run spawned to be
27+
// alive. Neither half transfers:
28+
//
29+
// * There is no `--strictPort` here. `dev` always passes `--dev` to `serve`,
30+
// so the auto-shift cannot be declined by a caller, and giving the CLI a
31+
// flag to decline it is a CLI contract change, not a fix to this script.
32+
// * A liveness check on our own spawn was ALREADY in this wait loop, and it
33+
// passes throughout the measurement above — our server did not die, it
34+
// succeeded on another port. Liveness is not the question here.
35+
//
36+
// What the script does instead is read the port its own server actually bound,
37+
// from the runtime state file `serve.ts` publishes under OS_HOME expressly for
38+
// external supervisors, in an OS_HOME this run can prove is its own because it
39+
// pinned the dev child's TMPDIR. That is the contract pinned below.
40+
//
41+
// ## Why these are executed assertions and not greps
42+
//
43+
// A grep for `TMPDIR` passes against a file that names it only in a comment, and
44+
// a grep for "reads the runtime file" passes against a check that runs in the
45+
// wrong order. So the argv is asserted through `smoke_dev_server_argv`, the
46+
// function the script itself builds its argv from, and the retarget is asserted
47+
// by standing up a real neighbour on the requested port and watching the real
48+
// wait function decline it in favour of the port its own state file names.
49+
//
50+
// The vacuity guards matter as much as the assertions. `NEIGHBOUR_BODY` proves
51+
// the neighbour was genuinely reachable at the same spelling the old wait loop
52+
// probed, and `OURS_BODY` proves the retargeted port was genuinely a different
53+
// server. Without those a green "retargeted" could mean nothing was listening
54+
// and nothing was turned down.
55+
//
56+
// No `objectstack dev` boot and no scaffold: the contract under test belongs to
57+
// the shell script, and the ports are picked at run time by the script's own
58+
// helper so this test cannot collide with a concurrent agent — which would be a
59+
// poor look here.
60+
61+
import { describe, it, expect } from 'vitest';
62+
import { execFileSync } from 'node:child_process';
63+
import fs from 'node:fs';
64+
import os from 'node:os';
65+
import path from 'node:path';
66+
import { fileURLToPath } from 'node:url';
67+
68+
const HERE = path.dirname(fileURLToPath(import.meta.url));
69+
const SCRIPT = path.resolve(HERE, '..', '..', '..', 'scripts', 'publish-smoke.sh');
70+
71+
function have(bin: string): boolean {
72+
try {
73+
execFileSync('sh', ['-c', `command -v ${bin}`], { stdio: 'ignore' });
74+
return true;
75+
} catch {
76+
return false;
77+
}
78+
}
79+
80+
// Linux-only by construction, like the sibling collision test: the failure being
81+
// pinned is an agent-container one, and the helpers read `/proc`-backed liveness.
82+
const RUNNABLE = process.platform === 'linux' && ['bash', 'curl', 'jq', 'node'].every(have);
83+
84+
/** A tiny HTTP server on $STUB_PORT announcing $STUB_NAME, as a `node -e` program. */
85+
const HTTP_STUB = [
86+
'const http = require("node:http");',
87+
'http.createServer((_q, r) => {',
88+
' r.writeHead(200, { "content-type": "application/json" });',
89+
' r.end(JSON.stringify({ iam: process.env.STUB_NAME }));',
90+
'}).listen(Number(process.env.STUB_PORT));',
91+
].join('');
92+
93+
/**
94+
* Run a bash harness that SOURCES the real script (so the real functions run)
95+
* and prints `KEY=value` lines. Returns them parsed.
96+
*/
97+
function runHarness(body: string[]): Record<string, string> {
98+
const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'publish-smoke-collision-'));
99+
const harness = path.join(dir, 'harness.sh');
100+
fs.writeFileSync(
101+
harness,
102+
[
103+
'#!/usr/bin/env bash',
104+
'set -u',
105+
`export SMOKE_ROOT=${JSON.stringify(dir)}`,
106+
'export SMOKE_KEEP=1',
107+
`export STUB=${JSON.stringify(HTTP_STUB)}`,
108+
// Sourcing defines the helpers and runs nothing.
109+
`source ${JSON.stringify(SCRIPT)}`,
110+
// The script's own `set -euo pipefail` came with it. Several steps below
111+
// are EXPECTED to fail and their exit codes are the thing being reported,
112+
// so hand errexit back — without this the harness dies at the first
113+
// expected failure, before it can kill its stubs, and the run hangs on a
114+
// stdout pipe held open by an orphan rather than failing an assertion.
115+
'set +e +o pipefail',
116+
// Belt and braces on top of that: every stub below is a listener, and a
117+
// leaked one holds a low port in a container several agents share — the
118+
// very collision this file is about. `jobs -p` on EXIT kills them however
119+
// the harness leaves, including paths no explicit `kill` line reaches.
120+
'trap \'for j in $(jobs -p); do kill "$j" 2>/dev/null; done\' EXIT',
121+
'echo "SOURCED=ok"',
122+
...body,
123+
].join('\n'),
124+
{ mode: 0o755 },
125+
);
126+
127+
const out = execFileSync('bash', [harness], { encoding: 'utf8', timeout: 120_000 });
128+
const parsed: Record<string, string> = {};
129+
for (const line of out.split('\n')) {
130+
const m = /^([A-Z_]+)=(.*)$/.exec(line);
131+
if (m) parsed[m[1]] = m[2];
132+
}
133+
return parsed;
134+
}
135+
136+
describe.skipIf(!RUNNABLE)('[#9647] publish-smoke.sh smoke-tests its OWN dev server', () => {
137+
it('sources cleanly and defines the collision helpers without running the gate', () => {
138+
const r = runHarness([
139+
'echo "PICK_FN=$(type -t smoke_pick_free_port)"',
140+
'echo "WAIT_FN=$(type -t smoke_wait_for_own_server)"',
141+
'echo "ARGV_FN=$(type -t smoke_dev_server_argv)"',
142+
// The gate itself must NOT have run: no scaffold, no tarballs.
143+
'echo "SCAFFOLDED=$([ -d "$SMOKE_ROOT/smoke-app" ] && echo yes || echo no)"',
144+
]);
145+
expect(r.SOURCED).toBe('ok');
146+
expect(r.PICK_FN).toBe('function');
147+
expect(r.WAIT_FN).toBe('function');
148+
expect(r.ARGV_FN).toBe('function');
149+
expect(r.SCAFFOLDED).toBe('no');
150+
});
151+
152+
it('picks a per-run port and skips one that is already held', () => {
153+
const r = runHarness([
154+
'FIRST="$(smoke_pick_free_port 3210)"',
155+
'echo "FIRST=$FIRST"',
156+
// Hold it, then ask again from the same base.
157+
'STUB_PORT="$FIRST" STUB_NAME=holder node -e "$STUB" >/dev/null 2>&1 & HOLDER=$!',
158+
'sleep 1',
159+
'SECOND="$(smoke_pick_free_port 3210)"',
160+
'echo "SECOND=$SECOND"',
161+
'echo "HOLDER_REACHABLE=$(curl -sS "http://localhost:$FIRST/" | jq -r .iam)"',
162+
'kill "$HOLDER" 2>/dev/null',
163+
]);
164+
expect(r.FIRST).toMatch(/^\d+$/);
165+
expect(r.SECOND).toMatch(/^\d+$/);
166+
// Vacuity guard: the port really was held, so the skip really was a skip.
167+
expect(r.HOLDER_REACHABLE).toBe('holder');
168+
expect(r.SECOND).not.toBe(r.FIRST);
169+
});
170+
171+
it('pins the dev child TMPDIR in the argv it actually spawns', () => {
172+
const r = runHarness([
173+
'DEV_TMPDIR="$SMOKE_ROOT/dev-tmp"',
174+
'SMOKE_PORT=31234',
175+
'echo "ARGV=$(smoke_dev_server_argv | tr "\\n" " ")"',
176+
]);
177+
// `env` and the assignment are part of the invocation, not a comment about it.
178+
expect(r.ARGV).toContain('env NO_COLOR=1 ');
179+
expect(r.ARGV).toContain('TMPDIR=');
180+
expect(r.ARGV).toContain('dev-tmp');
181+
expect(r.ARGV).toContain('objectstack dev --port 31234 --fresh');
182+
});
183+
184+
it("declines a neighbour on the requested port and targets the port its OWN state file names", () => {
185+
const r = runHarness([
186+
'DEV_TMPDIR="$SMOKE_ROOT/dev-tmp"',
187+
'mkdir -p "$DEV_TMPDIR/objectstack-dev-XXXX"',
188+
'REQUESTED="$(smoke_pick_free_port 3210)"',
189+
'OURS="$(smoke_pick_free_port $((REQUESTED + 50)))"',
190+
'echo "REQUESTED=$REQUESTED"',
191+
'echo "OURS=$OURS"',
192+
// Run A: the neighbour, answering 200 on the port run B asked for. This is
193+
// exactly what the pre-fix wait loop accepted.
194+
'STUB_PORT="$REQUESTED" STUB_NAME=NEIGHBOUR node -e "$STUB" >/dev/null 2>&1 & NEIGHBOUR=$!',
195+
// Run B: our own server, on the port the auto-shift moved us to, plus the
196+
// runtime state file `serve.ts` writes under our pinned OS_HOME.
197+
'STUB_PORT="$OURS" STUB_NAME=OURS node -e "$STUB" >/dev/null 2>&1 & SERVER_PID=$!',
198+
'sleep 1',
199+
'printf \'{"pid":%s,"port":%s,"url":"http://localhost:%s","environmentId":"env_local"}\' \\',
200+
' "$SERVER_PID" "$OURS" "$OURS" > "$DEV_TMPDIR/objectstack-dev-XXXX/runtime.env_local.json"',
201+
// Vacuity guards: both servers genuinely reachable, at the spelling probed.
202+
'echo "NEIGHBOUR_BODY=$(curl -sS "http://localhost:$REQUESTED/api/v1/health" | jq -r .iam)"',
203+
'echo "OURS_BODY=$(curl -sS "http://localhost:$OURS/api/v1/health" | jq -r .iam)"',
204+
'smoke_wait_for_own_server 10',
205+
'echo "BOUND_PORT=$BOUND_PORT"',
206+
'kill "$NEIGHBOUR" "$SERVER_PID" 2>/dev/null',
207+
]);
208+
// Both were up, so the choice below was a real choice.
209+
expect(r.NEIGHBOUR_BODY).toBe('NEIGHBOUR');
210+
expect(r.OURS_BODY).toBe('OURS');
211+
expect(r.REQUESTED).not.toBe(r.OURS);
212+
// The whole card: the requested port answered 200 and was turned down anyway.
213+
expect(r.BOUND_PORT).toBe(r.OURS);
214+
expect(r.BOUND_PORT).not.toBe(r.REQUESTED);
215+
});
216+
217+
it('refuses rather than guessing when no runtime state file is published', () => {
218+
const r = runHarness([
219+
'DEV_TMPDIR="$SMOKE_ROOT/dev-tmp"',
220+
'mkdir -p "$DEV_TMPDIR"',
221+
'REQUESTED="$(smoke_pick_free_port 3210)"',
222+
// A neighbour answering on the requested port, and a live process of our
223+
// own that never published where it bound.
224+
'STUB_PORT="$REQUESTED" STUB_NAME=NEIGHBOUR node -e "$STUB" >/dev/null 2>&1 & NEIGHBOUR=$!',
225+
'sleep 30 >/dev/null 2>&1 & SERVER_PID=$!',
226+
'sleep 1',
227+
'echo "NEIGHBOUR_BODY=$(curl -sS "http://localhost:$REQUESTED/api/v1/health" | jq -r .iam)"',
228+
'echo "OUR_LEADER_ALIVE=$(kill -0 "$SERVER_PID" 2>/dev/null && echo yes || echo no)"',
229+
'OUT="$(smoke_wait_for_own_server 1 2>&1)"; echo "WAIT_RC=$?"',
230+
'echo "WAIT_SAID_STATE_FILE=$(printf %s "$OUT" | grep -c "runtime state file")"',
231+
'kill "$NEIGHBOUR" "$SERVER_PID" 2>/dev/null',
232+
]);
233+
// Vacuity guards: a 200 WAS available on the requested port, and our own
234+
// process WAS alive — the two facts the pre-fix loop accepted as sufficient.
235+
expect(r.NEIGHBOUR_BODY).toBe('NEIGHBOUR');
236+
expect(r.OUR_LEADER_ALIVE).toBe('yes');
237+
expect(r.WAIT_RC).not.toBe('0');
238+
expect(r.WAIT_SAID_STATE_FILE).toBe('1');
239+
});
240+
});

scripts/check-cross-package-test-inputs.mjs

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -96,6 +96,17 @@ const CROSS_PACKAGE_TEST_INPUTS = {
9696
'content/docs/references/**',
9797
// scripts/dist-freshness.test.ts stages a fixture around the root scripts dir
9898
'scripts/**',
99+
// `serve.ts` is named in a comment rather than read, the same shape as
100+
// `check-nul-bytes.mjs` / `sync-template-versions.mjs` / the realtime
101+
// protocol page below, and settled the same way: the literal collector
102+
// takes quoted paths without parsing, so a mention forces a declaration,
103+
// and declaring the file is cheaper than rewording prose to dodge the
104+
// scanner. scripts/publish-smoke-port-collision.test.ts cites it for the
105+
// measurement that justifies its whole existence — `serve.ts` auto-shifts
106+
// off a busy port whenever `flags.dev` is set, which is the only reason
107+
// publish-smoke.sh cannot trust the port it asked for. One file, not the
108+
// commands tree: the test reads publish-smoke.sh and nothing else.
109+
'packages/cli/src/commands/serve.ts',
99110
// scripts/liveness/evidence.test.ts resolves the evidence paths the
100111
// liveness ledgers cite, so those files' existence is a spec input.
101112
'packages/runtime/src/**',

0 commit comments

Comments
 (0)