Repository navigation
test: the test tree holds zero hand-woven concurrency gates — 79 → 0, allow file deleted - #2806
Conversation
… allow file deleted The ratchet added yesterday froze the debt at a MEASURED 79 sites across 23 files, every one a `ManualResetEventSlim`, 32 of them in `IoPoolTest.cs`. All 79 are converted here, so `test` moves into `HandWovenGateRatchetGuard`'s `ProductionRoots` and `test/HandWovenGateSites.allow` is deleted — there is no allow file left, for any root. ## Two shapes, chosen by which way the signal travels - **Producer → test** (a handler reporting it started, a pooled leaf, a `.Finally`, a disposal callback) → an `AsyncSubject<Unit>` the producer completes, awaited through the house assertion helpers: `await x.Should().Within(...).Emit(because)`, or `.NotEmit(within)` for the negative. Never `.Wait()` on it — that swaps this ratchet for `BlockingBridgeInTestRatchetGuard`, the exact mistake made on #2786 and caught. - **Test → a worker it deliberately parks** (a wedged action block, a leaf that ignores its cancellation token, a merge turn held so an ack provably cannot arrive) → the park STAYS, because the park is the subject; the kernel handle goes. A volatile `int` the worker polls under a bounded `SpinWait.SpinUntil(predicate, budget)` — already the house idiom in these same files — returning the same `bool` the event's `Wait(timeout)` did, so sites that MEASURED whether the release arrived keep their assertion unchanged. 🚨 Every release is written in a `finally`, several of which are new. Without it an assertion that throws first leaves the worker parked for its full bound, into the next test — the two-minute pool-thread leak the allow file's own header records. No assertion weakened, no timeout widened. `Thread.Join(timeout)` and `IsSet`-style state reads were left as they are: they answer "did it finish" / "has it happened", not "wait". ## Measured before / after | | before | after | |---|---|---| | sites (guard's own scanner) | 79 | **0** | | files | 23 | 0 | | `IoPoolTest.cs` | 32 | 0 | | allow file | 79 lines | deleted | Touched: Hosting.Test (IoPoolTest 32, LiveQueryHandoffDropTest 6, LiveQueryForeignTrampolineTest 1), Messaging.Hub.Test (MessageHubTest 6, + 4 files × 2), Hosting.Monolith.Test (7 files × 2), Hosting.Orleans.Test (RoutingGrainDeliveryRetryTest 3, IoPoolSiloTeardownTest 2, RoutingGrainTurnIsolationTest 1), Graph.Test (3), Layout.Test (1), Persistence.Test (1), Autocomplete.Test (1). ## Verified - `dotnet build -c Release -warnaserror`, one project per invocation, on all 9 touched test projects → `0 Warning(s) 0 Error(s)` each. - Every converted class run by filter: 105 tests, `Passed!` with 0 failures — Hosting 30, Autocomplete 23, Messaging.Hub 21, Orleans 11, Graph 10, Monolith 8, Layout 1, Persistence 1, plus the 20 guard tests. - `HandWovenGateRatchetGuard`, `BlockingBridgeInTestRatchetGuard`, `ImpersonationScopeSiteRatchetGuard`, `ObservableToTaskBridgeGuard`, `RouterAsTestRequestOriginRatchetGuard` — all green (20/20). ## Mutation proof — the ratchet still bites, twice 1. Planted `new ManualResetEventSlim(false)` in `QueryInRenderDeadlockTest.cs` → `ProductionCodeHasNoHandWovenGate_OutsideTheSanctionedOne` **FAILED**, naming `test/MeshWeaver.Layout.Test/QueryInRenderDeadlockTest.cs (1)`. Reverted → green. 2. Re-created `test/HandWovenGateSites.allow` → the new `TheTestTreeAllowFileStaysDeleted` **FAILED** ("…is back"). Deleted → green (4/4). Work products conserved: `Doc/Architecture/RemovingHandWovenGates` carries the two shapes, the `finally` rule, the sweep's numbers and how the ratchet is proven; AGENTS.md, the `/async` skill and `Doc/Architecture/AsynchronousCalls` now state the rule as covering `test/` too. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
This PR completes the “test tree to zero” sweep by removing all remaining hand-woven concurrency gates from test/, deleting the last allow file, and updating the ratchet + documentation so the whole repo (including tests) is now held at zero.
Changes:
- Replaced
ManualResetEventSlim-style gates across the test suite withAsyncSubject<Unit>(producer → test) and volatile flags + boundedSpinWait.SpinUntil(test → deliberately parked worker). - Moved
test/into the guard’s zero-tolerance roots and deletedtest/HandWovenGateSites.allow, adding a regression check that fails if it reappears. - Added/updated docs (architecture + What’s New) and repo guidance (
AGENTS.md,/asyncskill,AsynchronousCalls) to describe the two sanctioned replacement shapes.
Reviewed changes
Copilot reviewed 31 out of 31 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| test/MeshWeaver.Persistence.Test/PartitionObjectsSubscriberIndependenceTest.cs | Replaces event gate with volatile flag + SpinUntil to keep the “blocked context” scenario without kernel handles. |
| test/MeshWeaver.Messaging.Hub.Test/TeardownHubCreationFreezeTest.cs | Converts handler/test gating to AsyncSubject + volatile release; test becomes async Task. |
| test/MeshWeaver.Messaging.Hub.Test/MessageHubTest.cs | Converts multiple gate sites; adds finally-release to avoid stranding a parked turn thread on assertion failure. |
| test/MeshWeaver.Messaging.Hub.Test/InflightHubCreationDrainTest.cs | Converts construction/test gating; wraps release in finally to prevent stranded construction thread. |
| test/MeshWeaver.Messaging.Hub.Test/HostedHubDisposalJoinTest.cs | Same conversion pattern as inflight construction drain; ensures release is guaranteed. |
| test/MeshWeaver.Messaging.Hub.Test/DisposalRaceNackTest.cs | Converts deterministic handler stall/release gates to AsyncSubject + volatile flag. |
| test/MeshWeaver.Layout.Test/QueryInRenderDeadlockTest.cs | Replaces ManualResetEventSlim with volatile flag + SpinUntil to preserve the “blocked subscribing thread” subject. |
| test/MeshWeaver.Hosting.Test/LiveQueryHandoffDropTest.cs | Removes event gates; introduces GatedAdapter helpers using AsyncSubject + volatile release; converts key tests to async Task. |
| test/MeshWeaver.Hosting.Test/LiveQueryForeignTrampolineTest.cs | Replaces event with volatile flag + SpinUntil for trampoline-blocking probe. |
| test/MeshWeaver.Hosting.Test/IoPoolTest.cs | Broad gate conversion to AsyncSubject + volatile releases, adding try/finally releases to prevent stranded pool threads. |
| test/MeshWeaver.Hosting.Orleans.Test/RoutingGrainTurnIsolationTest.cs | Converts injected stall gate from ManualResetEventSlim to volatile flag + SpinUntil. |
| test/MeshWeaver.Hosting.Orleans.Test/RoutingGrainDeliveryRetryTest.cs | Converts callback completion signals to AsyncSubject<Unit> and awaits via assertion helpers. |
| test/MeshWeaver.Hosting.Orleans.Test/IoPoolSiloTeardownTest.cs | Replaces event gates with AsyncSubject + volatile state to assert teardown joins pooled leaf. |
| test/MeshWeaver.Hosting.Monolith.Test/WriteWaitsForCommitVerdictTest.cs | Reworks merge-turn parking helper to be async and uses AsyncSubject + volatile release via an OwnerGate. |
| test/MeshWeaver.Hosting.Monolith.Test/ShutdownRoutingRejectClassificationTest.cs | Converts mesh action-block stall gating to AsyncSubject + volatile release in finally. |
| test/MeshWeaver.Hosting.Monolith.Test/ReleaseRequestWatcherHighWaterTest.cs | Converts “park owner write queue” gate to AsyncSubject + volatile release with finally to avoid wedging the class. |
| test/MeshWeaver.Hosting.Monolith.Test/QueuedWriteAdvancesOnHandoffTest.cs | Converts parked-turn handshake to AsyncSubject + volatile release, adding finally release. |
| test/MeshWeaver.Hosting.Monolith.Test/OwnerDisposalNackTest.cs | Same conversion for owner merge executor park/release. |
| test/MeshWeaver.Hosting.Monolith.Test/LayoutRenderTeardownDrainTest.cs | Converts render-in-flight and release signals to AsyncSubject + volatile release, removes event disposal. |
| test/MeshWeaver.Hosting.Monolith.Test/LateNackReenqueueTest.cs | Converts parked-turn handshake to AsyncSubject + volatile release with finally. |
| test/MeshWeaver.Hosting.Monolith.Test/DeleteNodeBehaviorTest.cs | Updates commentary to reflect reactive-await assertion shape (no hand-woven gate). |
| test/MeshWeaver.Graph.Test/SyncedQueryRlsForeignTrampolineTest.cs | Replaces event gate with volatile flag + SpinUntil to preserve trampoline-blocking subject. |
| test/MeshWeaver.Graph.Test/NodeTypeAdoptionRegistryTest.cs | Replaces event gating with AsyncSubject<Unit> and reactive assertion waits. |
| test/MeshWeaver.Documentation.Test/HandWovenGateRatchetGuard.cs | Moves test into zero-tolerance roots, deletes ratcheted inventory logic, and adds “allow file stays deleted” check. |
| test/MeshWeaver.Autocomplete.Test/AutocompleteIntegrationTest.cs | Removes ManualResetEventSlim waiting in timing diagnostics in favor of AsyncSubject + reactive waiting. |
| test/HandWovenGateSites.allow | Deletes the seeded inventory allow file (no longer tolerated anywhere). |
| src/MeshWeaver.Documentation/Data/WhatsNew/2026-08-30-the-test-tree-holds-zero-hand-woven-gates.md | Adds What’s New entry explaining the sweep and new invariant. |
| src/MeshWeaver.Documentation/Data/Architecture/RemovingHandWovenGates.md | Adds architecture doc describing the two replacement shapes and why finally-release is required. |
| src/MeshWeaver.Documentation/Data/Architecture/AsynchronousCalls.md | Updates rule to explicitly cover test/ and points to the new companion doc. |
| AGENTS.md | Updates repo-wide guidance to make the “no gates in test” rule explicit and actionable. |
| .claude/skills/async/SKILL.md | Updates /async skill to include test/ and the two sanctioned replacement shapes. |
Suppressed comments (1)
test/MeshWeaver.Hosting.Test/IoPoolTest.cs:498
- After switching the async gate to an
AsyncSubject<int>, complete it by publishing a value and callingOnCompleted()(idempotently, including in thefinally) rather thanTrySetResulton aTaskCompletionSource.
Volatile.Write(ref release, 1);
asyncGate.TrySetResult(0);
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Test Results (shard 0)1 178 tests ±0 985 ✅ ±0 5m 26s ⏱️ -8s Results for commit b3cf53c. ± Comparison against base commit afb5a5e. This pull request removes 1 and adds 1 tests. Note that renamed tests count towards both. |
Test Results 42 files ±0 42 suites ±0 35m 0s ⏱️ + 1m 26s Results for commit b3cf53c. ± Comparison against base commit afb5a5e. This pull request removes 1 and adds 1 tests. Note that renamed tests count towards both. |
The ratchet landed in #2782 froze a measured inventory: 79 hand-woven concurrency gates across 23 files in
test/, every one aManualResetEventSlim, 32 of them inIoPoolTest.cs. This is the sweep. All 79 are converted,testmoves into the guard'sProductionRoots, andtest/HandWovenGateSites.allowis deleted — there is no allow file left, for any root.Why a gate in a TEST is a defect, not a style choice
In product code the failure is obvious: a parked hub action block or grain turn is a deadlock. In a test the same primitive fails more quietly, and it has bitten twice already:
IoPoolResidualNamesItsPoolTestput itsrelease.Set()after anawaitof the method-timeout token. On timeout that line never ran and the leaf it had blocked kept a pool thread for two minutes, into the next test.AsyncSubject+.Wait()— an observable→blocking bridge, which parks the thread just the same and tripsBlockingBridgeInTestRatchetGuardinstead. Satisfying one guard by violating another is not a fix, and this PR deliberately does not do it: the blocking-bridge count is unchanged.Two shapes, chosen by which way the signal travels
Producer → test — a handler reporting it started, a pooled leaf, a
.Finally, a disposal callback:The negative direction is the same subject with
NotEmit(within)— the one place a fixed wait is correct, because there is no positive signal to await.Test → a worker it deliberately parks — a wedged action block, a leaf that ignores its cancellation token, a merge turn held open so an ack provably cannot arrive. The park stays: it is the subject. The kernel handle goes:
SpinUntil(predicate, timeout)is already the house idiom in these same files (IoPoolTest's own remarks named it), needs nousing, and returns the sameboolthe event'sWait(timeout)did — so the sites that measured whether the release arrived (legAReleasedCleanly,GateTimedOut,pumpCompletedWhileBlocked,answered) keep their assertion unchanged. Like the event, it never pumps aSynchronizationContext, which is load-bearing where the test's whole point is that the blocked thread must not run a queued continuation.🚨 Every release is in a
finally, several of them newly added. Where a helper owns the park (WriteWaitsForCommitVerdictTest'sOwnerGate),Dispose()releases and the call site isusing var gate = await ParkOwnerMergeTurn(path).Not touched:
Thread.Join(timeout)andIsSet-style state reads. They answer "did that thread finish" / "has this happened", not "wait for a signal", andJoinis the stricter primitive (#2792).No assertion weakened, no timeout widened. Several
voidtests becameasync Taskto await the reactive assertions — the normal shape.Measured
test/MeshWeaver.Hosting.Test/IoPoolTest.cstest/HandWovenGateSites.allowPer project: Hosting.Test 39 · Messaging.Hub.Test 14 · Hosting.Monolith.Test 14 · Hosting.Orleans.Test 6 · Graph.Test 3 · Layout.Test 1 · Persistence.Test 1 · Autocomplete.Test 1.
Verified
dotnet build -c Release -warnaserror, one project per invocation0 Warning(s)0 Error(s)Passed!, 0 failed — Hosting 30, Autocomplete 23, Messaging.Hub 21, Orleans 11, Graph 10, Monolith 8, Layout 1, Persistence 1HandWovenGateRatchetGuard·BlockingBridgeInTestRatchetGuard·ImpersonationScopeSiteRatchetGuard·ObservableToTaskBridgeGuard·RouterAsTestRequestOriginRatchetGuardMutation proof — the ratchet still bites, in both directions
1. A new gate must fail. Planted
new ManualResetEventSlim(false)inQueryInRenderDeadlockTest.cs:Reverted → green.
2. A resurrected allow file must fail. Re-created
test/HandWovenGateSites.allow:Deleted → green (4/4). That second check is new here: a cleared inventory that is quietly re-seeded is exactly how one grows back, and
SourceScan.ReadAllowFileis explicit that a regenerated allow file blesses whatever happens to be in the tree.Work products conserved
finallyis load-bearing,Thread.Joinvs an event, what the sweep measured, and how the guard is proven.Category: Feature).AGENTS.md, the/asyncskill andDoc/Architecture/AsynchronousCallsnow state the rule as coveringsrc/ortest/, and name the two replacement shapes so the next author does not reach for an event.The only surviving exemptions are the two verified
SanctionedGates: the one sealed insideIoPool, and the standaloneThumbnailGeneratorCLI whose premise (no MeshWeaverProjectReference) is re-checked on every run.🤖 Generated with Claude Code