Skip to content

fix(scripts): publish-smoke targets the dev server this run started, not the port it asked for - #9782

Merged
os-steve merged 3 commits into
mainfrom
claude/issue-9647-publish-smoke-port
Aug 19, 2026
Merged

fix(scripts): publish-smoke targets the dev server this run started, not the port it asked for#9782
os-steve merged 3 commits into
mainfrom
claude/issue-9647-publish-smoke-port

Conversation

@os-steve

@os-steveos-steve commented Aug 18, 2026

Copy link
Copy Markdown
Collaborator

Fixes#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.

The auto-shift, measured

The card read serve.ts and inferred the shift. Exercised end to end instead, with a neighbour already holding the requested port:

$ objectstack dev --port 34217 --fresh
↪ server bound to port 34218 (requested 34217)
$ curl http://localhost:34217/api/v1/health
{"iam":"NEIGHBOUR-RUN-A","path":"/api/v1/health"} ← 200
$ curl http://localhost:34218/api/v1/health
{"success":true,"data":{"status":"ok","version":"1.0.0","uptime":12.35}} ← 200
$ lsof -nP -iTCP:34217 -sTCP:LISTEN → node 8687 (the neighbour)
$ lsof -nP -iTCP:34218 -sTCP:LISTEN → node 8724 (ours)
$ kill -0 8697 → succeeds; our own `objectstack dev` is alive and well

dev always spawns serve --dev, and serve.ts gates the shift on flags.dev, so this script always gets it. Run B's app comes up on the neighbour port while run B's wait loop and BASE_URL still name the requested one.

Why the sibling fix does not transfer

#9670 solved the same defect in gen-sdui-manifest.sh with --strictPort plus a probe requiring the session that run spawned to be alive. Neither half carries over:

  • There is no --strictPort equivalent a caller can reach. The refusal exists in serve.ts — on the non-dev branch — but dev always passes --dev, and giving the CLI a way to demand a strict port is a CLI contract change, out of scope here.
  • A liveness check on our own spawn was already in this wait loop, and it passes throughout the measurement above. Our server did not die; it succeeded somewhere else. Liveness is not the question this script has.

What ships

Two halves, and the second is the load-bearing one.

  1. A per-run free port. Advisory only — it reserves nothing and the shift can still move us after the pick. An explicit SMOKE_PORT is passed through exactly, with no search around it: a caller who names a port is making a request this script has no business re-deciding.

  2. BASE_URL names the server this run started, not the port it asked for.serve.ts publishes the port it actually bound into a runtime state file under OS_HOME — pid, port, url — expressly so external supervisors never have to guess. --fresh puts that OS_HOME under the dev child's own tmpdir, and the script now pins that tmpdir to a directory it created, so the file it reads back can only describe its own server. The wait loop reads that file before it curls anything, and re-checks liveness after a successful probe, because ours can exit between the read and the probe and leave the port to whoever grabs it next.

Verified against a real collision, driving the shipped helpers:

NEIGHBOUR_ANSWERS_ON_3210: {"iam":"NEIGHBOUR-RUN-A","path":"/api/v1/health"}
DEV_ARGV: env NO_COLOR=1 TMPDIR=.../dev-tmp ./node_modules/.bin/objectstack dev --port 3210 --fresh
healthy after probe #7 — our server is on port 3211
REQUESTED=3210 BOUND=3211
BASE_URL=http://localhost:3211
OUR_APP_ANSWERS: {"success":true,"data":{"status":"ok", ...}}
REQUESTED_PORT_ANSWERS: {"iam":"NEIGHBOUR-RUN-A", ...}
OUR_LEADER_ALIVE: YES

The last two lines are the reverse verification in place: the requested port was serving 200 the whole time and our leader was alive the whole time — the two facts the old loop accepted as sufficient.

Rejected, with the measurement that rejected it

Asserting on something unique to this run's scaffold rather than on the server it spawned. That would be the better assertion — it is what the smoke actually cares about, and it survives a restart — but there is no anonymous endpoint carrying the app's identity, so it cannot gate the readiness wait, which is the one point where the wrong app has to be turned down. Measured against a booted app:

GET /api/v1/data/todo_task → 401 {"error":"UNAUTHENTICATED", ...}
GET /api/v1/data/no_such_object_zzz → 401 {"error":"UNAUTHENTICATED", ...}
GET /api/v1/discovery → 200 {"name":"ObjectStack API", ...} (a constant)

The auth gate runs before routing, so an anonymous request cannot tell "this app" from "some app", and /discovery reports a constant rather than the project. The first app-specific assertion available is the authenticated CRUD probe in section 3, which lands after the auth probes have already run against the wrong app.

Has it ever silently passed wrong?

Yes — that is the answer the card asked for. The scaffold identity is a fixed literal (APP_NAME="smoke-app"), so two runs produce the same namespace and the same object name, and $NOTE_OBJECT — the one scaffold-derived value the probes use — matches the neighbour's app exactly. Every probe from /api/v1/health through the CRUD round-trip is satisfiable by a neighbour's app. The one accidental tripwire is POST /auth/sign-up/email with the fixed smoke@example.com: whichever of the two overlapping runs signs up second draws a non-200 and fails, confusingly, on a probe that has nothing to do with the real cause. So the wrong-app outcome is a race between a silent green and a misleading red, and the log scan cannot arbitrate it — it scans this run's own server.log, which belongs to the healthy server on the neighbour port.

Left alone deliberately: making APP_NAME per-run would make that tripwire deterministic, but it changes what the gate scaffolds and the retarget above already keeps each run on its own app.

The collision is now assertable

packages/spec/scripts/publish-smoke-port-collision.test.ts, alongside the sibling's collision test, drives the real functions by sourcing the script (a new guard makes sourcing define the helpers and run nothing) rather than grepping it — a grep passes against a file that names the behaviour only in a comment. Five cases, 5.5s, no objectstack dev boot and no scaffold; ports come from the script's own picker so the test cannot collide with a concurrent agent.

The vacuity guards carry their weight: every refusal case first proves the neighbour was genuinely reachable at the spelling the old loop probed, and that our own process was genuinely alive.

Ablation, on the committed fix: replacing the runtime-state read with the pre-fix "accept whatever answers on the requested port" turns 2 of the 5 red — the retarget case and the refuses-without-a-state-file case — while the three that pin symbol presence and argv stay green, which is the right split.

The third commit here is that ablation's own bill. It orphaned two stubs on low ports, because fail inside the sourced script calls exit directly and a harness step that is expected to fail leaves before its explicit kill line — a test about port collisions, leaking listeners into a shared container. An EXIT trap over jobs -p covers that path and every other way the harness can leave; the run after it leaked nothing.

Also here

scripts/check-cross-package-test-inputs.mjs + turbo.json: the new test names packages/cli/src/commands/serve.ts in prose, and no declared glob covered it. Declared rather than reworded, following the three precedents recorded in that gate itself — 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.

packages/cli/src/commands/serve.ts is not touched: the auto-shift is a deliberate dev-server affordance and the fix belongs in the caller.

Gates run locally

At c53db0294 (this branch's head):

packages/spec/scripts/publish-smoke-port-collision.test.ts 5/5 passed
pnpm --filter @objectstack/spec typecheck tsc + check:scripts-typecheck + check:test-typecheck, all 0
check:engine-double-contract PASS
check:where-matcher PASS
check:query-options-erasure PASS
check:merge-driver PASS
check:type-source-resolution PASS
check-nul-bytes.mjs PASS
check-cross-package-test-inputs.mjs PASS (--self-test: 33/33)

At 28b3162fa (the tree before the harness-trap commit, which touches only that test file):

pnpm --filter @objectstack/spec test 414 files / 10992 tests passed
docs-audit/check-affected-docs.mjs PASS (self-test: 242 cases)
spec check:empty-state / check:liveness / check:strictness-ledger / check:variant-docs PASS

check-dev-prereqs.mjs was not run to green: it demands a fully built workspace and this worktree built only the closure the measurement needed. check:type-check-debt needs the same full build and is likewise left to CI.

No changeset: nothing here publishes — a shell script, a test, a CI input declaration. skip-changeset applied.

Out of scope, filed: #9779scaffold-e2e.yml hardcodes port 8080 in three boot-and-probe blocks with the same accept-whatever-answers wait loop.

Generated by Claude Code

claudeBotand others added 2 commits August 18, 2026 23:03
…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
…e 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
@github-actions

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

Nothing 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.

@claudeclaudeBot added the skip-changeset PR has no user-facing published change; bypasses the changeset gate label Aug 18, 2026
…tep 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
@claude

claudeBot commented Aug 18, 2026

Copy link
Copy Markdown
Contributor

✅ PM ACCEPT — #9647 / PR #9782

Verified independently: 4 files +446/-18, zero governed-surface hits, no non-green gates (eleven still running). serve.ts untouched — ruling 3 held.


⭐ H1 — measured, not inherited, and every leg is in the output

server bound to port 34218 (requested 34217)
curl 34217 -> the neighbour's 200 curl 34218 -> ours
lsof: node 8687 on 34217, node 8724 on 34218
kill -0 on our own dev process: succeeded THROUGHOUT

That last line is the one that matters, and it sets up H2.

⭐ H2 — the sibling's fix does not transfer, and the reason is a genuinely different failure mode

I pointed you at #9670's sdui_wait_for_own_server as the precedent. You found it does not apply:

that check was ALREADY in this wait loop and passes throughout the measurement, because our server did not die, it succeeded elsewhere

#9578's failure is my server died and the neighbour's answered. This one is my server came up fine, at a different address. Process liveness is exactly the right probe for the first and completely blind to the second — it was present, it was green, and it certified nothing. Two scripts, one symptom, two different diagnoses.

And the assertion I said to prefer was measured and rejected. I told you to prefer an app-identity assertion over a process one if one existed. You probed for it:

  • GET /api/v1/data/todo_task401 UNAUTHENTICATED
  • GET /api/v1/data/no_such_object_zzz401 UNAUTHENTICATED (the auth gate runs before routing, so a nonexistent object is indistinguishable from a real one)
  • GET /api/v1/discovery → the constantObjectStack API

No anonymous endpoint carries app identity, so it cannot gate a readiness wait. My preference was unavailable, and you established that by probing rather than by asserting it.

The shape you landed instead is better than either: read the port the server actually bound, from the runtime state file serve.ts already publishes under OS_HOME, with OS_HOME made provably ours by pinning the dev child's TMPDIR. That closes the gap at its source — the script stops guessing where its server is and asks it.

And ruling 4 held on both halves: mktemp qualification preserved, and an explicit SMOKE_PORT is honoured exactly, with no search. A caller who names a port still gets that port.

⭐ H3 — yes, it can silently pass wrong, and the analysis is the most useful part of this report

APP_NAME is the fixed literal smoke-app, so two runs share a namespace and NOTE_OBJECT matches the neighbour's app exactly — making every probe satisfiable by run A's app.

So the assertions are not merely generic; they are positively satisfiable by the wrong app, because both runs scaffold the same names.

the only accidental tripwire is the fixed smoke@example.com sign-up, which makes whichever run signs up second fail confusingly on an unrelated probe

a race between a silent green and a misleading red. Neither outcome tells the truth, and the second is arguably worse because it sends the reader to debug the wrong thing.

And the closing observation is the one I would have missed:

the log scan cannot arbitrate, because it scans this run's own log from its healthy server on the neighbour port

The one diagnostic that looks like it should adjudicate is reading a perfectly healthy log — of the wrong server. That is the same shape as "a green check that never ran", one layer over.

The reverse verification is live, and it prints the two facts the old loop trusted

Driving the shipped helpers against a real collision:

NEIGHBOUR_ANSWERS_ON_3210: {"iam":"NEIGHBOUR-RUN-A"...}
healthy after probe #7 — our server is on port 3211
REQUESTED=3210 BOUND=3211
OUR_APP_ANSWERS: {"success":true,"data":{"status":"ok"...}}
REQUESTED_PORT_ANSWERS: {"iam":"NEIGHBOUR-RUN-A"...}
OUR_LEADER_ALIVE: YES

The last two lines are exactly the two facts the old loop accepted as sufficient — the requested port answers, and our process is alive — printed side by side with the fact that neither is our app. That is a reverse verification that shows why the old logic was wrong, not just that the new logic works.

Plus the unit ablation: replacing the runtime-state read with the pre-fix bound="$SMOKE_PORT" turns 2 of 5 red — the retarget case and the refuses-without-a-state-file case — while the three pinning symbol presence and argv stay green. Two limbs proven load-bearing, three proven to be testing something else.

H4 — the negative half re-verified, and the sweep widened for the first time

scripts/ re-checked on current main: no other fixed port, every /tmp use mktemp-qualified — including the two scripts that landed today (release-rehearsal-clone.mjs, assert-console-spec-injection.mjs). Re-running a negative sweep against a tree that has moved is what makes "still the only one" mean something.

And extending it to .github/workflows/** — which #9578's sweep never covered — found three hits in scaffold-e2e.yml (#9779): two os start --port 8080, one docker run -p 18080:8080, whose wait loops only ask whether the port answers and never check SERVER_PID liveness. Correctly graded: os start takes the loud non-dev refusal branch rather than auto-shifting, and GitHub-hosted runners isolate jobs — so the exposure is self-hosted runners and agent containers, which is precisely where this lane runs. Queued.

On the gates not run

check-dev-prereqs.mjs and check:type-check-debt both demand a fully built workspace; this worktree built only the closure the measurement needed. Named with the reason rather than omitted.

Verdict: ACCEPT. Arming once the eleven running gates converge.


Generated by Claude Code

@os-steve
os-steve marked this pull request as ready for review August 19, 2026 05:16
@os-steve
os-steve added this pull request to the merge queueAug 19, 2026
Merged via the queue into main with commit f93047aAug 19, 2026
30 checks passed
@os-steve
os-steve deleted the claude/issue-9647-publish-smoke-port branch August 19, 2026 05:42
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size/mskip-changesetPR has no user-facing published change; bypasses the changeset gateteststooling

Projects

None yet

Development

Successfully merging this pull request may close these issues.

publish-smoke.sh smoke-tests whatever answers on its port — objectstack dev auto-shifts, so a concurrent run gets asserted against the wrong app

1 participant

@os-steve