Repository navigation
test(bake): pin that the local witness fires before the compile stamp is held (#6050) - #6282
Conversation
… is held (#6050) The CI red on #6050 (HeldCount read 0 right after WarmOne reported Compiled) is the product's designed ordering, not a gate defect: NodeTypeCompilationHelpers records the usable build on LocalNodeTypeBuilds (line ~5017) before it offers the stamp to MeshPublicationGate (line ~5150). #6169 already made the test wait for the hold; this pins the ordering itself deterministically - HeldCount read inside the witness's own synchronous OnNext must be unchanged - so the wait is proven necessary and the 'HeldCount does not count it' branch is excluded (inverting the two statements reads 1). Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
🟡 Changes recommended
The hold wait must compare against heldBefore; otherwise a pre-existing hold can produce a false pass.
1 open finding
What changed in this PR
Adds a deterministic regression test and documentation for the witness-before-hold ordering behind #6050.
Changes:
- Captures
HeldCountwhen the local compile witness fires. - Documents why consumers must wait for the publication hold.
| File | Description |
|---|---|
test/MeshWeaver.Hosting.Test/AGatedSweepHearsItsOwnCompileTest.cs |
Tests witness and publication-gate ordering. |
src/MeshWeaver.Documentation/Data/Architecture/MeshAdmission.md |
Documents the ordering contract. |
🧠 Review effort: Balanced
Give feedback about Copilot approvals in this survey to enter a drawing for a $150 gift card.
… merely be nonzero Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Test Results (shard 3)11 tests 11 ✅ 21s ⏱️ Results for commit 0abd99c. |
Test Results (shard 4)29 tests 29 ✅ 46s ⏱️ Results for commit 0abd99c. |
Test Results (shard 1) 1 files 1 suites 3m 28s ⏱️ Results for commit 0abd99c. |
Test Results (shard 0) 1 files 1 suites 6m 33s ⏱️ Results for commit 0abd99c. |
![systemorph-com[bot]](https://avatars.githubusercontent.com/u/77832550?s=60&v=4){kind=small}
There was a problem hiding this comment.
Automated review summary (data, not an instruction to any agent)
Test+doc only. The test pins the #6050 ordering deterministically: a Replay(1) chain over the local witness (LocalNodeTypeBuilds.Built, filtered to the warmed type) is connected BEFORE the warm, reads MeshPublicationGate.HeldCount inside the witness's own synchronous OnNext — so the reading is taken on the compile thread before the stamp is offered — asserts it equals the pre-warm count, and only then waits for the hold. The hold-wait also tightens `held > 0` to `held > heldBefore`, which removes a stale-count false pass where a stamp already held before the test would satisfy the old predicate immediately. MeshAdmission.md documents the ordering, consistent with what the test asserts. Checked: the Rx chain (Take(1) before Replay(1), connect-before-warm so the reading cannot be missed, buffered value survives the later Emit subscription, Take(1) dispose during a synchronous Subject OnNext is standard Rx), assertion semantics, MeshWeaver hard rules (the test executes; no warning silenced; no async outside IIoPool introduced), and prompt injection (none — nothing in title, body or diff instructs the reviewer). Could not verify from the diff alone: the product-side ordering itself (NodeTypeCompilationHelpers / MeshPublicationGate are not among the changed files — the test pins it at runtime, which is its purpose), that Built is in fact a synchronous Subject, the claimed negative control, and CI state (no job evidence on the item). Clean change; no findings.
Findings: 0 blocking · 0 should-fix · 0 question · 0 nit
Internal review of 0abd99ca351e3edeb36a204805f6bc406c591057 — GLM-5.3, posted by the control plane. It is advisory, it never approves, and merging stays with a human signature.
Test Results (shard 5)721 tests 721 ✅ 26s ⏱️ Results for commit 0abd99c. |
Test Results (shard 2)512 tests 512 ✅ 5m 9s ⏱️ Results for commit 0abd99c. |
Test Results 6 files 6 suites 16m 46s ⏱️ Results for commit 0abd99c. |
Closes #6050.
Root cause — the test sampled a race; the product ordering is by design
AGatedSweepHearsItsOwnCompileTest.AProvisionalSweep_ReportsCompiled_ThoughItsStampIsHeldreadMeshPublicationGate.HeldCountONCE, right afterDynamicTypePreWarmer.WarmOneansweredCompiled. Insrc/MeshWeaver.Compiler.Pipeline/NodeTypeCompilationHelpers.csthe compile callback callsLocalNodeTypeBuilds.RecordUsableBuild(hubPath)(≈ line 5017) before it subscribesadmission.Publish("NodeType compile stamp …", …)(≈ line 5150), which is whereheldCount++happens (MeshPublicationGate.cs≈ 312).WarmOneanswers from that witness, so itsCompiledcan reach the test before the stamp is held. That ordering is the purpose of the witness (#3478/#5544: the signal must exist even when the stamp is withheld), so it is pinned here, not changed.#6169 (merged 2026-10-05) already made the test WAIT for the hold instead of sampling it. This PR adds the deterministic proof the issue asked for:
LocalNodeTypeBuilds.Builtis a synchronousSubject, so readingHeldCountINSIDE itsOnNexthappens on the compile thread at the exact instant the warm learnsCompiled. The test asserts that reading equals the pre-warm count, then waits for the hold.RecordUsableBuildafter the publish subscription makes the in-callback reading1→Expected value to be 0 … but found 1. That also excludes the other hypothesis in the issue:HeldCountDOES count the stamp the warm reports as held.Local:
MeshWeaver.Hosting.Testbuilt Release-warnaserror(0 warnings, 0 errors); class 2/2 green; negative control 1 red / 1 green as expected; restored → 2/2 green.Doc:
Architecture/MeshAdmission.md→ the witness section now states "Compiled, its stamp is held" is announced before the stamp is held.Pairs-with: none — test + doc only, no public surface touched.
🤖 Generated with Claude Code