Skip to content

test(spec): end the publish-smoke collision harness with exit 0 - #10916

Merged
os-elon merged 1 commit into
mainfrom
claude/issue-10671-publish-smoke-exit-zero
Aug 21, 2026
Merged

test(spec): end the publish-smoke collision harness with exit 0#10916
os-elon merged 1 commit into
mainfrom
claude/issue-10671-publish-smoke-exit-zero

Conversation

@os-elon

Copy link
Copy Markdown
Collaborator

Fixes#10671

Finishes the reporting fix PR #10456 landed for the sdui sibling. That PR shipped set +eand a trailing exit 0; this file has carried only the former since it was written. PR #10684 (this file's THIEF fix) is where the missing half was measured and filed.

What changed

runHarness builds the harness body and hands it to execFileSync, so the harness's exit status decides whether the test body gets to run its assertions at all. Every case in this file ends by killing its stubs:

'kill "$HOLDER" 2>/dev/null',
'kill "$NEIGHBOUR" "$SERVER_PID" 2>/dev/null',
'kill "$THIEF" 2>/dev/null',

Reaping a stub that has already exited returns 1. The set +e +o pipefail at :184 stops that from aborting the harness mid-line — which is all it buys — but it does not stop it from becoming the harness's exit status. execFileSync then throws in the test body and the failure arrives as a bare Command failed, pre-empting the vacuity guard written to name the real problem.

One line, at the end of the harness body: 'exit 0'. The harness's exit status is not a measurement; every measurement is a printed KEY=VALUE line and the assertions grade those.

The exit 0 sits after the body but the trap … EXIT at :189 still runs, and a trap that does not itself exit leaves the status alone — so the stubs are still reaped and 0 is still what execFileSync sees. Measured, not assumed: it is what makes the after-fix leg below report an assertion rather than Command failed.

Positive control — the card's two shapes, reproduced on one rig

Mutation: the HOLDER stub in "picks a per-run port and skips one that is already held" exits early, so the vacuity guard has an empty value to name and the cleanup kill reaps a dead pid. Same mutation both legs; the only difference is whether the harness ends exit 0.

Before the fix — the card's captured shape, assertion never reached:

 FAIL scripts/publish-smoke-port-collision.test.ts > … > picks a per-run port and skips one that is already held
Error: Command failed: bash /tmp/publish-smoke-collision-DDcr9m/harness.sh
curl: (7) Failed to connect to localhost port 3210 after 1 ms: Couldn't connect to server
❯ runHarness scripts/publish-smoke-port-collision.test.ts:196:15
❯ scripts/publish-smoke-port-collision.test.ts:225:15

After the fix — the report the file's guards were written to produce:

 FAIL scripts/publish-smoke-port-collision.test.ts > … > picks a per-run port and skips one that is already held
AssertionError: expected '' to be 'holder' // Object.is equality
❯ scripts/publish-smoke-port-collision.test.ts:260:32
260| expect(r.HOLDER_REACHABLE).toBe('holder');

Firing first is the point. With no holder, smoke_pick_free_port legitimately returns the same port and expect(r.SECOND).not.toBe(r.FIRST) at :261 would fail too — which reads as "the picker hands back busy ports", the misdiagnosis this file's header warns about. The guard names the real problem instead.

Both legs confirmed on disk by grepping for the injected and the removed spelling — never by the editor's exit code: mutation leg injected=1 removed=0, restore leg mutated=0 original=1, restore driven by trap … EXIT INT TERM so a mid-run kill could not leave a mutated tree behind for later measurements. No rebuild leg is involved, re-derived rather than inherited from the sibling PR: the file's whole import list is vitest plus node:child_process/node:fs/node:os/node:path/node:url — no @objectstack/* specifier resolving through a package exports/distpnpm --filter '@objectstack/spec^...' build reports No projects matched the filters, and packages/spec declares no workspace dependency at all.

The HTTP_STUB error guard — judged, and deliberately NOT taken

The card raised this as a separate judgement: give the four fixed-port HTTP_STUB users the s.once("error", …) guard OWNED_HTTP_STUB already carries. Measured, and left out — it changes nothing the harness can observe.

Every one of HTTP_STUB's four call sites (:229, :266, :269, :296) spells >/dev/null 2>&1. Against that spelling, with a squatter forcing the stub to lose its bind:

stub exit statuscleanup kill rcstderr bytes
unguarded (today)11896
guarded110

The two forms are indistinguishable to the harness. The only difference is a stack trace on a stream nothing reads, and neither form changes the exit status or the dead-pid kill that was making the failure illegible. What made a lost bind report as Command failed was never the trace — it was the missing exit 0, which is what this PR fixes; four inert guards on top would not have reported it.

Not unsafe, then — inert, so the card's stated condition for inclusion ("changes the failure shape the right way") is not met, and adding it would be speculative surface. The measurement is recorded in HTTP_STUB's docblock rather than only here, because the file now visibly raises the question: a reader comparing the two stubs would otherwise close the asymmetry on symmetry grounds alone.

Fences, each checked

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 this file reaches no published surface. Tests-only goes through the skip-changeset label in this repo, applied to this PR (the label is a real mechanism here, per PR #10684).

Evidence

All at HEAD 374697959e, clean tree:

pnpm --filter @objectstack/spec exec vitest run scripts/publish-smoke-port-collision.test.ts
Test Files 1 passed (1) · Tests 8 passed (8)
pnpm --filter @objectstack/spec test -> Test Files 415 passed (415) · Tests 11049 passed (11049)
pnpm --filter @objectstack/spec typecheck -> check:test-typecheck: OK — 55 file(s) / 263 error(s)
held in test-typecheck-debt.json (unchanged)

Gates

Re-derived against the real diff with node scripts/pm/dispatch-gates.mjs (it took the change set from the merge base itself, 7e75dc701), all run at 374697959e. 12 path-matched + 5 convention-triggered, quoting each gate's own verdict line rather than a bare $?:

check:empty-state 0 · check:liveness 0 · check:strictness-ledger 0 · check:variant-docs 0
check:merge-driver 0 · check:slot-lookup 0 · check:test-source-alias 0
check:type-source-resolution 0 · check:nul-bytes 0 · check-ci-filter-parity 0
check-plugin-teardown-shape 0 · check-affected-docs 0 · check:query-options-erasure 0
check:type-check-coverage 0 · check:engine-double-contract 0 · check:where-matcher 0
✓ slot-lookup ratchet holds: 107 unswept site(s) in 25 file(s), none new
check-engine-double-contract: OK — 376 pinned, 133 in the DEBT ledger, 2 exempt
✓ where-matcher conformance holds: 275 matcher(s) discovered, 275 answer … (165 refuse)
check-test-source-alias OK — 72 packages with tests scanned
✓ query-options-erasure ratchet holds: 67 unswept non-test site(s) in 17 file(s), none new
check-type-check-coverage: OK — 64/77 workspace packages type-checked, 13 in the DEBT ledger
check-nul-bytes: OK (scanned 6298 text file(s); no raw ASCII control bytes)

Each pnpm --filter gate was checked for the zero-match trap — a filter matching no script exits 0 having run nothing — by confirming the > @objectstack/spec@17.1.0 … banner is present in its log. All four show it.

Three narrowings, declared:

  1. check:type-check-debt and check-dev-prereqs were NOT MEASURED, not "not applicable". Both refuse outright on an unbuilt worktree — --re-measure cannot run: 55 workspace dependenc(ies) … have no built type entry point on disk, and check-dev-prereqs ends Fix: pnpm build. That refusal is the gates working as designed (measuring from an unbuilt closure would record a different world), not a verdict about this diff. CI runs both against a built tree. The ratchet half of the former is the same ledger check:test-typecheck reported unchanged above.
  2. Repo-wide pnpm lint is CI's run. Narrowed to the changed file, and the narrowing is measured rather than asserted: the file is in ESLint's own population (isPathIgnoredfalse, config resolved with 4 rules); lintFiles reports 1 file linted, 0 errors, 0 warnings, 0 suppressed; and the diff cannot move any untouched file's verdict because type-aware linting is not enabled anywhere here — calculateConfigForFile reports parserOptions.project: null and projectService: null.
  3. The gate union above ran after the final commit, at the HEAD this PR points to.

Generated by Claude Code

The harness had `set +e +o pipefail` after sourcing publish-smoke.sh but no
trailing `exit 0`, so its exit status was whatever the last command returned —
and every case ends by killing its stubs. Reaping a stub that has already
exited returns 1, execFileSync then throws, and the failure arrives as a bare
`Command failed` with the vacuity guard that would have named the problem
never reached.
This is the second half of the reporting fix PR #10456 landed for the sdui
sibling; this file had only the first.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01B4h3medzvhB9rpfoja9jcw
@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-elonos-elon added skip-changeset PR has no user-facing published change; bypasses the changeset gate and removed size/s labels Aug 21, 2026 — with Claude
@os-elon
os-elon marked this pull request as ready for review August 21, 2026 17:37
@os-elon
os-elon added this pull request to the merge queueAug 21, 2026
Merged via the queue into main with commit 39c682fAug 21, 2026
38 of 39 checks passed
@os-elon
os-elon deleted the claude/issue-10671-publish-smoke-exit-zero branch August 21, 2026 17:53
@github-actions

Copy link
Copy Markdown
Contributor

⛔ merge queue 构建失败 — 先分诊,再决定要不要重排

队列构建 32509184310 红了。队列跑的是全量套件(PR 侧 CI 只跑 affected 子集),
所以失败的测试可能在本 PR 没碰过的包里 —— 那不是重排能修的。每次盲目重排都会让排在后面的所有 PR 重建一轮。

失败的 job(日志抽取,best effort):

  • Console Pin Gate — 失败步骤: Build the Console SPA at the pinned objectui SHA

    ✗ Build failed in 5.84s
    

↳ 失败原因 是判读的关键:超时Test timed out in … / Hook timed out in …)多半是负载/时序,不是本 PR 的回归;
断言AssertionError: …)才指向真实的行为改变。两者的 FAIL 行长得一模一样,只有这一行能区分。

跨 PR 相同签名(24h,按失败测试文件聚合):

  • ⚠️本次没有可用的聚合签名(日志里没有能解析出测试文件名的 FAIL 行)—— 这不是「没有同签名的其他 PR」,是这一轮没测到。跨 PR 聚合本次不可用,请手工比对其他 PR 的同类评论。
  • ⚠️ 24h 评论账本没读完(超过 5 页仍未读到窗口尽头),所以上面的「不同 PR 数」是下界,不是全量。

历史信号:

  • 本 PR 过去 24h 无队列失败记录(首次)。
  • 过去 24h 队列共有 41 个失败构建(不含本次)。

分诊清单:

  1. 失败测试在本 PR 改动的包里 → 真回归,修 PR。
  2. 失败测试与本 PR 无关 → 看上面的「跨 PR 相同签名」;已有汇总 issue ⇒ flaky/环境问题实锤,去那张 issue 上谈,修好前重排只会再烧一轮全队列。
  3. 两者都不是 → 可能与同组 PR 语义冲突;等前面的 PR 落地或失败出队后再重排一次即可,不要连续重排。

Generated by Claude Code · merge-queue-triage workflow (#4859)

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

skip-changesetPR has no user-facing published change; bypasses the changeset gateteststooling

Projects

None yet

2 participants

@os-elon@claude