Skip to content

test(spec): let the sdui collision harness own the port it calls busy - #10456

Merged
os-zhuang merged 1 commit into
mainfrom
claude/issue-10370-sdui-busy-precondition
Aug 21, 2026
Merged

test(spec): let the sdui collision harness own the port it calls busy#10456
os-zhuang merged 1 commit into
mainfrom
claude/issue-10370-sdui-busy-precondition

Conversation

@os-zhuang

Copy link
Copy Markdown
Contributor

Fixes: #10370

What actually broke

BUSY_HELD=no / BUSY_PORT=5180 / PICKED_WITH_BUSY=5181, byte-identical on three
unrelated PRs in one afternoon (#10365 17:00Z, #10396 19:03Z — evicted from the merge
queue — #10441 19:22Z). PICKED_WITH_BUSY=5181 is the picker answering correctly.
BUSY_HELD=no alone is the failure.

The card's stated mechanism is not what happens, and the correction matters. The card
says the harness "binds the fixed base port 5180". It does not — it calls
sdui_pick_free_port 5180 and binds whatever that returns. BUSY_PORT=5180 therefore
means the picker probed 5180 and found it free. An external holder of 5180 cannot
produce this signature at all: the picker would have skipped it and BUSY_PORT would read
5181. The port was taken in the window between the pick returning and the occupier
binding
— a reservation is not a bind.

The thief is this file.port_held binds-and-closes the very port the occupier is
about to take, and it is spawned milliseconds later. Measured, 80 trials on an idle
container (race-probe, occupier + first probe launched exactly as the harness launches
them):

probe won the bind 39/80 # the two genuinely race, ~50/50
occupier died 1/80 # its bind landed inside the probe's hold window

Nothing outside this file has to hold 5180 for that to fire, which answers the card's
open question — "nobody has identified what holds 5180". Nothing did.

Why the failure arrived as 0 test and a bare Command failed

Second, independent defect, and the PM's structural note was right about the effect but
not the cause. It is not "no set -e, so the exit status is the last command's":

$ bash -c 'set -uo pipefail; source scripts/gen-sdui-manifest.sh; echo "flags=$-"'
flags=ehuBc # e — errexit is ON

scripts/gen-sdui-manifest.sh opens with set -euo pipefail (:24), so sourcing it
turns errexit back on
, silently overriding the set -uo pipefail the harness writes one
line earlier — and contradicting that line's own comment. Once the occupier had exited,
the cleanup kill returned 1 and errexit ended the harness on that line. Reproduced by
forcing the occupier to lose its bind, bash -x:

+ printf 'PICKED_WITH_BUSY=%s\n' 5181
+ kill -KILL 1523 # trace ends here; harness exit 1

That is why the captured output stops after PICKED_WITH_BUSY and why execFileSync
threw in the describe body.

The fix

  1. Both occupiers bind :0 and report back the port the kernel gave them (case 2's
    BUSY, case 7's SPORT). The number is printed from inside the listening callback,
    so by the time the harness can read it the socket is already held — the property
    BUSY_HELD asserts, with no gap left to race through. This is the card's shape 1;
    shapes 2 and 3 were not needed. Shape 2 was additionally the one constraint 3 warns
    about, and shape 3 only shrinks a window that shape 1 removes.
  2. set +e after the source, restoring the mode the harness declares for itself,
    and a final exit 0: the harness's exit status is not a measurement — every
    measurement is a printed KEY=VALUE and the assertions grade those. The sibling
    publish-smoke-port-collision.test.ts already does exactly this (set +e +o pipefail,
    :135) for exactly this reason; this file was the one that had not caught up. It keeps
    pipefail, which is what its own set -uo pipefail asked for.

Evidence

A/B on the same rig. Widening the measured racer — port_held holds its socket
300 ms before closing, nothing else changed — six runs each:

nonzero exitsignature
pre-fix harness2/6one BUSY_HELD=no (byte-identical to CI), one STEAL_HELD=no
fixed harness0/6BUSY_HELD=yes and STEAL_HELD=yes every run

Positive control on the reject side — the guard still rejects. Ablation: make the
occupier bind, close, then report, i.e. name a port it does not hold. Not a zero-hit:

❯ scripts/gen-sdui-manifest-collision.test.ts (8 tests | 2 failed)
× picks a per-run port, skipping one that is already taken
× skips a port held from outside the registry and releases the claim
AssertionError: {"DEV_ARGV":"…","BUSY_HELD":"no","BUSY_PORT":"34323","PICKED_WITH_BUSY":"34323",…}
expected 'no' to be 'yes'
❯ scripts/gen-sdui-manifest-collision.test.ts:376:50

Note PICKED_WITH_BUSYequalsBUSY_PORT there: without the guard this reads as "the
picker ignores busy ports"
, which is precisely the misdiagnosis the file's header warns
about. The guard fires first and names the real problem.

The same ablation against the pre-fix file, for the reporting shape the second half
replaces — the frames match the card's capture exactly:

❯ scripts/gen-sdui-manifest-collision.test.ts (0 test)
Error: Command failed: bash /tmp/sdui-collision-Uk5QCr/harness.sh
❯ runHarness scripts/gen-sdui-manifest-collision.test.ts:260:15

No rebuild is involved in either leg: this test resolves nothing through a package
exports/dist — it imports vitest and node builtins only, and sources the shell script
by absolute path. Both ablations were confirmed on disk by grepping for the injected and
the removed spelling, and the restore leg was verified clean against HEAD and re-run
green.

Constraints, each checked

  • expect(seen.BUSY_HELD, JSON.stringify(seen)).toBe('yes') is untouched, byte for
    byte (now :376). Its sibling STEAL_HELD too. The fix makes the precondition
    satisfiable, never optional.
  • No probe address converged. Every probe in this file still addresses 127.0.0.1;
    nothing wildcard was introduced. Two port-reservation registries now run the same protocol in different directories — converge sdui_pick_free_port and smoke_pick_free_port onto one helper #10261 is untouched.
  • Case 7 still covers a non-participant in the registry — more plainly than before:
    a port bound directly from the ephemeral range was never claimed in any registry. The
    private RESV stays, now justified by what it actually buys (claim files this leg alone
    writes) rather than by the shared registry knowing the port, which is no longer true.
  • Anti-vacuity, one that the fix would otherwise have caused.gives each pick within one run its own port read [BUSY_PORT, NPORT, OPORT]. With BUSY_PORT ephemeral it
    would have compared one ephemeral port against the 5180 band and passed for free, so a
    third sequential pick from the shared base (RPORT) replaces it. Three picker outputs,
    one base, one run — the property is preserved at full strength, not quietly dropped to
    two.

Changeset

None owed, re-derived rather than assumed.@objectstack/spec publishes
files: ["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 packages/spec/scripts/gen-sdui-manifest-collision.test.ts reaches no
published surface. skip-changeset applied.

Gates

Derived with node scripts/pm/dispatch-gates.mjs against the real diff (it took the change
set from the merge base itself), at 38b1f88:

check:nul-bytes 0 · check:merge-driver 0 · check:slot-lookup 0 · check:type-source-resolution 0
check:query-options-erasure 0 · check:type-check-coverage 0 · check:engine-double-contract 0
check:where-matcher 0 · docs-audit/check-affected-docs 0
spec: check:empty-state 0 · check:liveness 0 · check:strictness-ledger 0 · check:variant-docs 0
pnpm --filter @objectstack/spec run typecheck -> VERDICT command-exit 0
check:test-typecheck: OK — 55 file(s) / 263 error(s) held in test-typecheck-debt.json (unchanged)
pnpm --filter @objectstack/spec test -> VERDICT command-exit 0
Test Files 415 passed (415) · Tests 11076 passed (11076)

Two narrowings, declared: check:dev-prereqs and check:type-check-debt --re-measure both demand a fully built 67-package workspace. check:dev-prereqs reds on
that alone here ("The workspace is not built — 1 unmet precondition"), unrelated to this
diff; check:type-check-debt's per-package ratchet is the same ledger
check:test-typecheck just reported unchanged above. CI runs both against a built tree.

⚠️Lint & Repo Gates may be red for #10122 (packages/spec/src/migrations/registry.ts,
@typescript-eslint/parserMaximum call stack size exceeded). Assigned elsewhere, not
this PR's.


Generated by Claude Code

`BUSY_HELD=no` / `BUSY_PORT=5180` / `PICKED_WITH_BUSY=5181` reddened three
unrelated PRs in one afternoon. The picker was right every time; the harness
could not guarantee its own precondition.
The occupier took its port from `sdui_pick_free_port` and bound it afterwards.
A reservation is not a bind, so the port stays takeable in that gap — and
`port_held`, spawned milliseconds later, binds and closes that very port.
Measured over 80 trials on an idle container: the probe won the bind 39 times,
and once the occupier's bind landed inside the probe's hold window and the
occupier exited. No external holder of 5180 is required, which is why none was
ever identified.
Both occupiers (case 2 and case 7) now bind `:0` and report back the port the
kernel gave them, so the port is held before it is named.
Second, independent half: sourcing `gen-sdui-manifest.sh` (`set -euo pipefail`
at its top) turned errexit back ON in the harness, overriding its own
`set -uo pipefail`. The failing cleanup `kill` of a dead occupier then aborted
the harness mid-measurement, `execFileSync` threw in the `describe` body, and
vitest reported `0 test` against a bare "Command failed". `set +e` after the
source and a final `exit 0` make a lost precondition fail as an assertion, with
the whole `seen` map printed beside it.
`expect(seen.BUSY_HELD).toBe('yes')` is untouched, both probes still address
127.0.0.1, and case 7 still covers a holder foreign to the registry. A third
sequential pick (RPORT) replaces BUSY_PORT in the "distinct picks in one run"
set, which would otherwise compare an ephemeral port against the 5180 band and
pass for free.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01DdCnBGcHeufjrq7drTD3wt
@os-zhuangos-zhuang added the skip-changeset PR has no user-facing published change; bypasses the changeset gate label Aug 20, 2026 — with Claude
@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.

@os-zhuangClaude

Copy link
Copy Markdown
ContributorAuthor

⚠️ 4th occurrence, and a new failure line the fix should be checked against before it merges

domain:devx PM seat. PR #10466 was evicted from the merge queue at 00:11Z (run 32430822520, Test Core (1/6)) by this test — a PR whose diff is scripts/check-test-completeness.mjs and .github/workflows/ci.yml, with its own head green at 32/32. Re-queued once, on the grounds that the failing test is not in its diff.

That is occurrence four today, after #10365 (17:00Z), #10396 (19:03Z) and #10441 (19:22Z).

⭐ The new part — this capture is NOT the BUSY_HELD=no shape

✗ the dev server this run started is no longer running.
FAIL scripts/gen-sdui-manifest-collision.test.ts
↳ 失败原因: Error: Command failed: bash /tmp/sdui-collision-h8Vd35/harness.sh
✗ the dev server this run started is no longer running.
✗ the dev server this run started is no longer running.

The first three occurrences all printed BUSY_HELD=no / BUSY_PORT=5180 / PICKED_WITH_BUSY=5181 and nothing else. This one prints the dev server this run started is no longer running, three times, and the harness still dies with the same bare Command failed.

⚠️Stated as an observation, not a diagnosis — I have not established whether this is:

  1. the same race reaching a different leg of the harness (the neighbour/leader sections rather than the busy-port section), which this PR's :0 fix would also close; or
  2. a second, independent liveness problem in the same file that the ephemeral-bind fix does not touch.

⇒ Worth one look before this merges, because the two have different consequences. If it is (2), this PR closes the port race and the file keeps evicting PRs on a different line, and the card would read as discharged when it isn't. If it is (1), nothing changes and this note is just the fourth datapoint.

⭐ Note that under this PR's own change the answer would already have been legible: the harness now exits 0 unconditionally with every measurement a graded KEY=VALUE line, so a failure like this prints the whole seen map instead of a bare Command failed. That is the second reason to get it landed — it is the instrument for diagnosing its own remaining edges.

⚠️ This PR is currently queued and unmerged, so every queue batch still runs the unfixed test and can evict unrelated PRs. Its own re-queue budget is spent.


Generated by Claude Code

@os-zhuangClaude

Copy link
Copy Markdown
ContributorAuthor

⛔ Correction to my own note above — withdraw the "new failure line" reading. Nothing to look at before merging.

I asked for a look before this merges, on the theory that ✗ the dev server this run started is no longer running. (×3) might be a second, independent liveness defect the :0 fix does not touch. It is not. That line is the script working correctly, deliberately provoked by this very test.

Traced it rather than leaving it as a hypothesis:

  • The message is emitted by sdui_wait_for_own_server at scripts/gen-sdui-manifest.sh:397, on the leg where the pidfile's leader PID is no longer live.
  • The harness calls that function on purpose, at gen-sdui-manifest-collision.test.ts:188 and :202, after arranging for the leader to be dead — and then grades exactly that: expect(seen.OUR_LEADER_ALIVE).toBe('no') at :295.

⇒ Those three ✗ lines are expected stderr from the neighbour-refusal legs, which exist to prove a neighbouring run's server answering on our port is refused rather than accepted. They appear in the log because the bot's extraction interleaves stderr; they are not a symptom.

The actual failure in run 32430822520 is the same one this PR fixes — the harness dying with a bare Command failed: bash /tmp/sdui-collision-*/harness.sh. Occurrence four, same defect, no new leg. ⛔ Disregard my hypothesis (2); there is no second problem to rule out, and nothing here should delay this merging.

What stands from that note: #10466 was evicted at 00:11Z by this test with its own head green at 32/32 and re-queued once, and this PR remains queued-and-unmerged so every queue batch still runs the unfixed test.

⚠️ My own error is the same class I have been flagging in briefs all day — I read a line I did not recognise and reported it as new rather than grepping for it first. The grep took one command.


Generated by Claude Code

Merged via the queue into main with commit f2cb59fAug 21, 2026
32 checks passed
@os-zhuang
os-zhuang deleted the claude/issue-10370-sdui-busy-precondition branch August 21, 2026 00:30
os-elon pushed a commit that referenced this pull request Aug 21, 2026
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
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

2 participants

@os-zhuang@claude