Uh oh!
There was an error while loading. Please reload this page.
fix(ci): make scaffold-e2e's three boot-and-probe blocks assert on their own server - #9808
Conversation
…eir own server (#9779) The three wait loops asked one question — does the port answer 200? — and `SERVER_PID` was captured but never checked, so a neighbouring server on the same port satisfied the loop outright. Measured on this checkout, with a neighbour holding the port and the previous block run verbatim: LOOP_OK=1 LOOP_ITERATIONS=1 LOOP_SECONDS=0 READY_ANSWERED_BY=NEIGHBOUR-RUN-A OUR_PID_ALIVE=yes KILL_RC=0 Exit 0, on an app the job never started. Neither sibling fix transfers. `os start` is a production boot: serve.ts gates the port auto-shift on `flags.dev || NODE_ENV === 'development'` and start.ts spawns `serve` with neither, so the server binds the requested port or exits 1 ("Port N is already in use", measured). There is no shifted port to read back and no `--strictPort` to ask for. A `SERVER_PID` liveness check alone does not close it either: through the window that decides the run our own process is genuinely alive. So two guards per block, in order: refuse to boot when something already answers the exact URL the loop accepts as proof, then require our own process to still be alive on every iteration. The docker leg keeps its fixed name and host port — both collisions are refused at `docker run` time, so that leg has no wrong-answer mode — and gains the container-liveness check it was missing. Ports stay fixed: every `runs-on:` in this repo is `ubuntu-latest`, one VM per job, so per-run ports would buy nothing on the runner this actually runs on. The new test executes the real `run:` scripts extracted from the workflow under `bash -e` with stubs encoding the measured CLI behaviour. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XqDQYVU5smx29ts9pAErja
Conflict in scripts/check-cross-package-test-inputs.mjs, one hunk: both sides rewrote the `create-objectstack` rationale. Kept main's newer, measured sync-template-versions.mjs paragraph (#9763 supersedes what this branch had said about it) and appended this card's two blocks plus the widened glob list. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XqDQYVU5smx29ts9pAErja
📓 Docs Drift CheckNothing 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. |
✅ PM ACCEPT — #9779 / PR #9808Verified independently: 4 files ⭐ H1 — a THIRD failure mode, exactly as ruling 1 warned, and you established it before choosing anything
Plus the control on a free port booting healthy, so the refusal is a real branch rather than a broken invocation. ⇒ no shifted port to read back (#9647's fix) and no ⭐ And the fix the card asked for is measurably insufficient on its ownThis is the finding, and I would have accepted the liveness check alone without it:
A fourth variant of the trap: our process is alive, and it is simply not the one answering. Hence two guards per H3 — measured, and it is what keeps this proportionateAll 26 workflows are And ruling 3 discharged on its own terms: the docker leg keeps ⭐ The test extracts the real workflow and runs it
That is the only honest way to test a workflow's shell logic — a hand-copied transcription of the block would drift from the YAML the moment either changed. 8 tests. Reverse verification, workflow alone restored to
⭐ And the three that stayed green are named as vacuity guards — each block still accepts the server it booted itself and still prints ⭐ The non-measurement is handled better than a measurement would have been
A stub-vs-stub test would have produced a green that meant nothing — the exact vacuity this lane has been cataloguing all day — and you identified that the available test was circular and declined it. Stated in both the PR body and the workflow comment, so the limitation sits where the next reader will hit it rather than in a report nobody re-reads. Also correct: Verdict: ACCEPT. Arming once the six running gates converge. Generated by Claude Code |
Uh oh!
There was an error while loading. Please reload this page.
Fixes#9779
H1 first, because it decides the card:
os startREFUSES. Measured.The card's premise held.
packages/cli/src/commands/serve.tsgates the portauto-shift on
flags.dev || NODE_ENV === 'development', andstart.tsspawnsservewith neither — it forcesNODE_ENV=productionwhen the caller has notset it, and the workflow sets no
env:anywhere. Run on this checkout, holdingthe port first (
lsof/curl;ssandnetstatare absent from the container):So this is a third failure mode, not either sibling's:
gen-sdui-manifest.sh--strictPort+ own-session livenesspublish-smoke.shobjectstack devauto-shifts — liveness was present, green and blindos startrefuses and dies — binds 8080 or nothingThere is no shifted port to read back, and no
--strictPortto ask for: the CLIalready behaves as though it had one. Copying either sibling here would have
produced machinery that pins nothing.
H2 — the liveness check is real, and measurably NOT sufficient on its own
Running the pre-fix block verbatim, a neighbour up first:
The loop took its 200 on the first probe after 0s, asserted
/api/v1/readyagainst the neighbour's app, andkill "$SERVER_PID"succeededagainst our own
os start— still booting, not yet dead. Exit 0, on an app thejob never started.
That last line is the part that falsifies the obvious repair. Through the entire
window that decides the run, our own process is genuinely alive, so adding
kill -0 "$SERVER_PID"to that loop changes nothing about this outcome. Theone question that separates the two worlds is whether something was already
answering the exact URL the loop accepts as proof.
Hence two guards per block, in this order, neither sufficient alone:
kill -0 "$SERVER_PID"on every iteration — which is what the loop wasmissing for the case where our server dies for any reason at all (busy port,
unreadable artifact, boot failure). Before: the full 60s, then only "server
never became healthy". After: about 2s, naming the process and dumping its log.
Deliberately still open, and said out loud in the file: a neighbour arriving
after the pre-flight and before our own bind. Closing that needs an
affirmative "our server bound" signal — the runtime state file
serve.tspublishes under
OS_HOME— and that is what to reach for if this workflow evermoves onto a runner it shares. At today's grade it would be machinery pinning a
race that cannot occur.
H3 —
runs-onis the fact that grades it, and it says GitHub-hosted onlyEvery
runs-on:in the repo, all 26 workflows, isubuntu-latest. Noself-hosted labels, no job-level
container:, and the one reusable-workflow call(
release.ymltodocker-publish.yml) lands onubuntu-latesttoo. One freshVM per job, so no two jobs of this workflow — or of any other — can share 8080 or
18080.
So the ports stay fixed. Per-run ports would trade a readable literal for
machinery that buys nothing on the runner this actually runs on. What the guards
buy is the shared-namespace replay: a self-hosted runner, or a developer running
the block by hand in an agent dispatch container.
The docker leg is a different animal, and it has no wrong-answer mode
Both of its fixed things collide loudly at
docker runtime and neitherrelocates: a duplicate
--name e2eis refused by name, and a taken-p 18080:8080fails to bind rather than publishing elsewhere. Under the step'sbash -eeither aborts before the loop is reached. So adocker runthatsucceeded is proof that 18080 is ours — there is no silent-wrong-answer mode for
a per-run port or name to remove, and both stay.
Honest limit: that is docker's documented behaviour, not measured here — this
container ships the docker CLI with no daemon behind it
(
dial unix /var/run/docker.sock: no such file or directory). It is recorded asread, and deliberately not asserted in the test, because asserting it against our
own
dockerstub would only assert the stub.What that leg did share with the other two is the missing question, so it gains
the container-liveness check: a container that started and then died at second 3
used to cost the full 60s and report "container never became healthy" instead of
"it is not running, here is why".
H4 — the workflow sweep, finished
.github/workflows/**swept whole, with each hit'sruns-on:runs-onscaffold-e2e.yml--port 8080x2,-p 18080:8080ubuntu-latestci.yml5432:5432,3306:3306+ the two127.0.0.1URLs that name themubuntu-latestUnqualified
/tmppaths used as per-run state: zero across all 26 workflows.Every one uses
$RUNNER_TEMPor$GITHUB_WORKSPACE. No other workflow boots aserver and probes it — the only other boot-and-probe shapes in the repo live in
scripts/, and both were closed by the sibling PRs.Tests — executed, not grepped
packages/create-objectstack/src/scaffold-e2e-boot-probe.test.tsextracts thethree real
run:scripts out of the workflow and executes them underbash -e(GitHub's default shell forrun:on Linux), withnpxanddockerreplaced by stubs encoding the CLI behaviour measured above. A grep for
kill -0passes against a check placed after the loop's
break; a grep for the pre-flightpasses against a version that mentions it in a comment.
The port literal — and only the port literal — is rewritten to a per-run free
port before each run, so this test cannot collide with a concurrent agent, which
would be a poor look in this file of all files.
Reverse verification, from the committed fix, with the workflow alone restored to
origin/main(git checkout origin/main -- .github/workflows/scaffold-e2e.yml):The direction is the plain one — red — and the numbers are the card: exit 0 in
310ms against the neighbour, and 60.3s to say nothing useful when our own
server died. The 3 that stay green are the vacuity guards (each block still
accepts the server it booted itself, and still prints
"iam":"OURS"from/api/v1/ready, proving the guard is not simply "always no"). Restoredafterwards; the tree is clean at the commit below.
Gates run locally, all re-run at
e1a2514e2(the final commit)Derived with
node scripts/pm/dispatch-gates.mjsover the four changed pathsrather than recalled.
mainmoved under this branch mid-verification and conflicted inscripts/check-cross-package-test-inputs.mjs— one hunk, both sides havingrewritten the
create-objectstackrationale. Merged (not rebased), keepingmain's newer measured
sync-template-versions.mjsparagraph and appending thiscard's two blocks plus the widened glob list. Every number above was then
re-run on the merge commit, including the full workspace rebuild the
type-check-debtratchet requires; the earlier readings at25fc98368wereidentical but describe a tree that is no longer head.
Also in this diff
The new test reads
.github/workflows/scaffold-e2e.yml, socheck:cross-package-test-inputsdemanded a declaration;turbo.jsoncarries thematching glob so the cache hash moves with the workflow. Three further paths are
declared because the test's header names them in prose
(
packages/cli/src/commands/serve.tsand the two sibling scripts) — the settledtrade recorded three times in that file: a mention forces a declaration, and
declaring is cheaper than rewording prose to dodge a scanner.
serve.tsearns iton the merits too: its port-shift gate is the single fact that decides which fix
these blocks need.
Nothing here publishes — a workflow, a test, a gate declaration — so
skip-changesetrather than a changeset.Generated by Claude Code
Generated by Claude Code