test(spec): let the publish-smoke THIEF own the port it calls held - #10684
Merged
Conversation
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. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01B4h3medzvhB9rpfoja9jcw
Contributor
📓 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. |
This was referenced Aug 21, 2026
os-elon
marked this pull request as ready for review
August 21, 2026 09:43
This was referenced Aug 21, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #10457
Ports the shape PR #10456 landed for the sdui sibling. That PR is the settled direction for this defect class; this is the same change one file over, with the one deliberate divergence noted under Fences.
What changed
The "skips a port held from outside the registry" case staged its holder by picking a port, deleting the claim, then binding it:
Between the
rm -fand the stub'slistenthe port is claimed by nobody, so any concurrentsmoke_pick_free_portcaller scanning from 3210 may legitimately take it. Losing there does not report as "the precondition evaporated" — it reports asSTEAL_HELD != THIEF, which reads as an accusation of the picker on a PR that never touched it.Now the stub binds
:0and reports back the port the kernel gave it, printed from inside thelisteningcallback, so by the time the harness can read the number the socket is already held. The pick-then-rmdance is gone, and the fixedsleep 1with it — waiting for the number is waiting for the hold, so there is no second thing to wait for and no fixed sleep to be wrong about on a loaded container.The registry property gets plainer, not weaker: a port bound straight from the ephemeral range was never claimed in any registry, so there is no claim to remove. Being outside the registry is now a property of how the thief got the port rather than something staged by deleting a claim.
Fences, each checked
OWNED_HTTP_STUBtakes no host argument, unlike the sdui sibling'ss.listen(0, "127.0.0.1", …).smoke_pick_free_port's own probe binds the wildcard address —scripts/publish-smoke.shsays so in place: "No host argument: the wildcard bind is the spelling serve.ts's own isPortAvailable() uses" — so an occupier here must hold the wildcard address for that probe to see it at all. The only127.0.0.1this diff adds is the word inside the docblock that explains why it is absent. Two port-reservation registries now run the same protocol in different directories — convergesdui_pick_free_portandsmoke_pick_free_portonto one helper #10261 is untouched, and stays open.set +e +o pipefailhalf was not touched. It is present at what is now:184, byte for byte, with its original comment.scripts/publish-smoke.shnorscripts/gen-sdui-manifest.shis in this diff.git diff --name-only origin/main...HEADreturns exactly one path. Both shell scripts were read only.STEAL_HELD,STEAL_PICK,STEAL_CLAIM_ON_PICK,STEAL_CLAIM_RELEASED. The change makes the precondition satisfiable, never optional.STEAL_CLAIM_RELEASEDstill exercises the path it always did, and for a better reason. The picker claims the base it is asked to scan, probes it, finds it busy, and hands the claim back; previously therm -fwas what made that claim attempt succeed, and now nothing ever claimed the ephemeral port in the first place.One assertion added, and why it is not ceremony
The base is now reported by the thief rather than returned by the picker, so "is it a number at all" is a question this case did not used to have. It closes a vacuity hole the change would otherwise open: with
STEALempty,smoke_pick_free_portfalls back to its own3210default (local base="${1:-3210}") andSTEAL_PICKwould then differ fromSTEAL_BASEfor free.No analogue of the sibling's
RPORTrepair is needed here. That was required because sdui'sgives each pick within one run its own portcountedBUSY_PORTamong three picker outputs, and an ephemeral port differs from the 5180 band however the registry behaves. This file's equivalent test drawsA/B/Cfrom base 3210 and never referenced the thief's port, so it is untouched and stays at full strength.Evidence
The case runs green, and so does the package. Full suite at this PR's HEAD
a57228046:Positive control on the vacuity guard, after the change. Ablation: make the thief bind, close, then report — naming a port it does not hold. The guard fires, and fires first:
Firing first is the point. With the port released,
smoke_pick_free_portreturns it andSTEAL_PICK === STEAL_BASE— which without the guard reads as "the picker hands back busy ports", the exact misdiagnosis this file's header warns about. The guard names the real problem instead.No rebuild is involved in either leg, and this was checked rather than inherited from the sibling PR. The file's complete import list is
vitestplusnode:child_process/node:fs/node:os/node:path/node:url— no@objectstack/*specifier, so nothing resolves through a packageexports/dist, and the script under test is sourced by absolute path.pnpm --filter '@objectstack/spec^...' buildreportsNo projects matched the filters, corroborated bypackages/specdeclaring no workspace dependency at all.Both ablation legs were confirmed on disk by grepping for the injected and the removed spelling, never by the editor's exit code: mutation leg
removed=0 injected=1, restore legremoved=1 injected=0, and the restore ran from atrap … EXIT INT TERMso a mid-run kill could not leave a mutated tree behind. The tree is clean ata57228046.A finding this turned up, filed rather than absorbed
The first ablation attempt failed in an unexpected shape —
Error: Command failed: bash /tmp/publish-smoke-collision-*/harness.shatrunHarness, with the assertion never reached. Cause: this harness has no trailingexit 0, so its exit status is whatever the last command returned, and every case ends with a cleanupkillthat returns 1 against a stub which has already exited.This partially falsifies the card's "Not in this finding" note. The note is right that the
set +e +o pipefailhalf is already here — but that is only the first half of #10370's reporting fix; PR #10456 landedset +eand a finalexit 0, and this file has only the former. Filed as #10671 with the measurement, and left out of this diff: the card fences the reporting machinery off, and the second ablation above isolates the guard from it rather than absorbing a fix for it. #10671 stays open.After this change the THIEF stub binds
:0and cannot realistically lose its bind, so this case no longer trips that. The file's four other stubs still bind ports handed to them by the picker, so it remains live for them.Changeset
None owed, re-derived rather than assumed.
@objectstack/specpublishesfiles: ["dist","json-schema","liveness","prompts","llms.txt","README.md","src/**/*.zod.ts","CHANGELOG.md","api-surface","spec-changes.json"]—scripts/is not among them, so this file reaches no published surface. Tests-only in this repo goes through theskip-changesetlabel rather than an empty changeset (the label exists here as a real mechanism: "PR has no user-facing published change; bypasses the changeset gate"), and it is applied to this PR.Gates
Re-derived against the real diff with
node scripts/pm/dispatch-gates.mjs(it took the change set from the merge base itself,9dd192d48), all run ata57228046on a clean tree:Verdict lines rather than bare exit codes, e.g.
check-test-source-alias OK — 72 packages with tests scanned;check-engine-double-contract: OK — 371 pinned, 133 in the DEBT ledger, 2 exempt;✓ slot-lookup ratchet holds: 107 unswept site(s) in 25 file(s), none new.The derivation named
check:cross-package-test-inputsin the dispatch but does not derive it for this path; it named five convention-triggered families instead (check:query-options-erasure,check:type-check-coverage,check:type-check-debt,check:engine-double-contract,check:where-matcher), which are run above except as noted below.Two narrowings, declared:
check:dev-prereqsandcheck:type-check-debt --re-measureboth require a fully built 67-package workspace, which this worktree does not have; CI runs both against a built tree. The ratchet half of the latter is the same ledgercheck:test-typecheckreported unchanged above.pnpm lintis CI's run. Narrowed to the changed file, and the narrowing is measured rather than asserted: the file is in ESLint's own population (ESLint#isPathIgnored→false, config resolved with 4 rules);--format jsonreports 1 file linted, 0 errors, 0 warnings, 0 suppressed; and the diff cannot move any untouched file's verdict because this repo never enables type-aware linting —calculateConfigForFilereports noparserOptions.projectand noprojectService, matchingeslint.config.mjs's own measured note that it enables type-aware linting "for ANY file, test or not" nowhere.Generated by Claude Code