fix(#2480): the only pooled leaf in src/ that cannot be settled by the drain, plus a guard for the class by rbuergi · Pull Request #5281 · Systemorph/MeshWeaver

rbuergi · 2026-09-22T18:00:13Z

Refs #2480 — not closed by this, and the reason is the whole point (below).

Scope note. This PR originally also carried the disposal-drain NACK fix (the ShutdownNack
composition), reached from #4888. #5272 got there first from #4866 and now
owns that half; those four files are restored to main here, so there is no overlap between the
two pull requests. What is left is what is uniquely this one's. The one assertion this branch had
that #5272 lacks — Contain(ShutdownNack.FormatActivationTag(hub)), the EXACT activation identity
rather than "some hex is present", which is #2376's drift in the other direction — was handed to
that author rather than merged here.

The only pooled leaf in `src/` that cannot be settled by the drain

IoPool.InvokeCore runs a leaf as return await io(ct) and releases the gate permit in that
await's finally. So a task that cannot observe ct holds its permit for as long as the
underlying call takes, whatever the drain does: IoPool.Disposed never fires and
IoPoolSiloTeardown's bounded join expires — "pooled I/O did not finish within 00:00:30 … A leaf
ignored its cancellation token; fix the leaf, do not widen the budget".

That is a lexical property of a call site, so the class can be swept without waiting for an
occurrence. Invoke and IoPoolExtensions.Run are the entry points with no projection of their own
— InvokeObservable waits through ObserveCompletion(…, ct), InvokeStream enumerates
.WithCancellation(ct), InvokeBlocking holds no permit — and the sweep of src/ returned exactly
one offender (1 of 30 over Invoke alone, which is how it was found; 1 of 49 once Run was
added, below):

src/MeshWeaver.Connection.Orleans/OrleansRoutingService.cs
    ioPool.Invoke(_ => subscription.UnsubscribeAsync())      // pool "RoutingStream"

Orleans' StreamSubscriptionHandle.UnsubscribeAsync() takes no CancellationToken at all — it is a
grain call to the pub-sub rendezvous, which at silo stop is precisely a grain the catalog is
deactivating — so there was no token to hand down and the leaf could only be settled by not
awaiting it. It now projects the token onto the wait (task.WaitAsync(ct), the same shape
NuGetAssemblyResolver.ResolveAsync already uses) and reports the abandonment, because a leaf
that settles on its token and then says nothing has traded a loud teardown residual for a silent
one. The abandoned grain call runs on Orleans' own threads over Orleans' own types, so it is not the
use-after-unload hazard the join exists to prevent — the held permit is, since it is what makes the
silo release over every OTHER leaf too. The task is still observed, so a late fault can never become
an unobserved-task exception.

PooledLeafObservesItsTokenGuard now holds src/ at zero for the class, asserts its detector in
both directions, and asserts its own denominator (a zero over no sites scanned is not a reading).

🚨 It states what it CANNOT see, because a claim wider than its coverage is the defect it
exists to prevent (both points from the automatic review, accepted). It scans the LAMBDA form of
Invoke and of IoPoolExtensions.Run — Run composes straight onto Invoke, so pool.Run(_ => …)
reaches the same await io(ct) and the first draft missed it entirely; adding it leaves the offender
count at zero and moves the DENOMINATOR from 30 to 49. A leaf passed as a METHOD GROUP or a
delegate variable cannot be decided lexically at all, so that is declared out of scope and pinned by
TheDetectorCannotSeeAMethodGroup_AndSaysSo rather than left to a comment, which cannot fail. A
non-lambda argument is deliberately not failed: PluginBundleClient.FetchIndex and
IoPoolExtensions.Run both use the form correctly today (ReadIndex does observe its ct), and
turning a correct form red to protect a lexical scan is the wrong trade. Real coverage there is a
call-graph pass or an analyzer.

🚨 This does not name the subject of #2480's 18 production occurrences — the sweep is static and
every occurrence on file is the bare line with no pool, no site and no stack. What it changes is
that the next occurrence is a discriminator rather than a repetition: Did NOT report: either names
RoutingStream, or names another site, or is empty (the Zip defect). #2480 stays open on that
reading. The five-issue grouping this came out of (#4886 · #2833 · #2480 · #4847 · #4888 — three
roots, no duplicates) is posted on #2480.

Control

Red on the unfixed branch with the named failure, green on the fix. The guard reads the file tree,
so reverting that one line IS the variable:

	red (leaf reverted to `_ =>`)	green (fixed)
`PooledLeafObservesItsTokenGuard`	`🚨 1 of 49 pooled async leaves discard the token …: src/MeshWeaver.Connection.Orleans/OrleansRoutingService.cs: .Invoke(_ => …) — _ is never referenced in the leaf` — `total: 4 failed: 1`	`total: 4 failed: 0`

Suites, Release: the IO-pool suites (--filter IoPool, MeshWeaver.Hosting.Test) total: 68,
failed: 0; MeshWeaver.Hosting.Orleans.Test total: 324, failed: 0 — and zero was ABANDONED
lines in that run, so the new report is a no-op on the healthy path;
MeshWeaver.Messaging.Hub.Test total: 504, failed: 0 (run before this PR was narrowed, on the
same MeshWeaver.Messaging.Hub, which is now back to main's); DocumentationLinkIntegrityTest
green after the doc edits, on a MeshWeaver.Documentation.dll rebuilt from them (an embedded asset
edited under --no-build is a known false pass here, so it was rebuilt and the text verified
present in the assembly).

Every touched project built -c Release -warnaserror, each 0 Warning(s) 0 Error(s);
MeshWeaver.Connection.Orleans.dll mtime 1790100889 (epoch s) at the final green run,
MeshWeaver.Documentation.dll rebuilt from the edited pages before the link test read them.

Docs

Doc/Architecture/ControlledIoPooling — the token-LESS-API clause: project the token onto the
WAIT, report the abandonment, and which entry points already do it for you (measured, per entry
point, because that boundary is what decides whether a site is a defect).
Doc/Architecture/DisposedScopeAndDyingHubs (R4) — what a static sweep settles and what it
cannot, and why one fixed instance of a class is not a measurement of that class's population in
a running portal.

Notes

Pairs-with: none — no public type or member is removed; the new method is private.
Recycle addresses: none. Process-level framework code carried by the image; no NodeType
source, no in-mesh layout area, nothing a per-node hub binds at activation. A roll is the whole
delivery.

🤖 Generated with Claude Code

…ot composed by ShutdownNack Two teardown defects the ObjectDisposedException cluster triage made decidable from the code. 1. IoPool.Invoke is `await io(ct)` with the gate permit released in that await's finally, so a leaf whose task cannot observe `ct` holds its permit through the whole drain — IoPool.Disposed never fires and IoPoolSiloTeardown's bounded join expires over live work. That is a lexical property of a call site, and the sweep of src/ found 1 of 30: OrleansRoutingService's stream teardown discarded the token on a call (Orleans' UnsubscribeAsync) that takes none. It now projects the token onto the WAIT and REPORTS the abandonment; the abandoned grain call runs on Orleans' own threads over its own types, so it is not the use-after-unload hazard the join exists to prevent — the held permit is. PooledLeafObservesItsTokenGuard holds src/ at zero, asserts its detector in both directions and asserts its own denominator. This does NOT name the subject of any production occurrence; a static sweep bounds the class, and only a residual report that NAMES a pool and call site decides an occurrence. Refs #2480. 2. The disposal drain's [DISPOSE-DISCARD] NACK was the one owner-side refusal not composed by ShutdownNack. It read correctly to a human — it had even copied the reactivation promise across by hand — and carried neither machine-readable half: no banner, so IsAnsweredByOwner and the three text classifiers read a transient refusal as terminal; no activation tag, so the sync stream's re-arm charged it against the distinct-activation budget instead of the paced same-activation one. Composed by RetryForTheAuthoritativeAnswer now, gates and teardown attribution riding in the `what` clause. Refs #4888. FailedGateAnswersBeforeTheTeardownTest pinned a fragment of the old sentence as its positive control; it asserts the same property through the producer's own predicate instead, plus that the answer still names the gate. Docs: ControlledIoPooling (project the token onto the wait when the API takes none, and which entry points already do it), DisposedScopeAndDyingHubs R4 (what a static sweep settles and what it cannot), RidingOutAShuttingDownAddress (a refusal not composed by the seam defeats both axes). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

github-actions · 2026-09-22T18:05:25Z

Test Results (shard 0)

1 files ±0 1 suites ±0 2m 41s ⏱️ -9s
320 tests ±0 320 ✅ ±0 0 💤 ±0 0 ❌ ±0
324 runs ±0 324 ✅ ±0 0 💤 ±0 0 ❌ ±0

Results for commit c69071b. ± Comparison against base commit 636e5cb.

♻️ This comment has been updated with latest results.

github-actions · 2026-09-22T18:05:40Z

Test Results (shard 1)

396 tests - 1 215 396 ✅ - 1 215 45s ⏱️ - 2m 30s
1 suites - 1 0 💤 ± 0
1 files - 1 0 ❌ ± 0

Results for commit c69071b. ± Comparison against base commit 636e5cb.

♻️ This comment has been updated with latest results.

github-actions · 2026-09-22T18:05:50Z

Test Results (shard 3)

934 tests +491 934 ✅ +491 7m 37s ⏱️ + 6m 41s
1 suites - 2 0 💤 ± 0
1 files - 2 0 ❌ ± 0

Results for commit c69071b. ± Comparison against base commit 636e5cb.

♻️ This comment has been updated with latest results.

Two agents reached the same root independently: #5272 from issue #4866, this branch from #4888. #5272 was first, is narrower, and covers three things this half did not — the 105-discard production measurement, the OwnerAnswerRecognitionGuard extension that closes the enumeration blind spot (the drain was its seventh unlisted terminal), and the Address-vs-path pair in the live test. So it owns MessageService.cs, both Messaging.Hub tests and the RidingOutAShuttingDownAddress page; those four files are restored to main here and there is no overlap left between the two pull requests. What remains on this branch is what is uniquely its own: the OrleansRoutingService pooled-leaf token projection, PooledLeafObservesItsTokenGuard, and the ControlledIoPooling / DisposedScopeAndDyingHubs pages. The one assertion this half had that #5272 lacks — Contain(FormatActivationTag(hub)), the EXACT activation identity rather than 'some hex is present', which is the #2376 drift in the other direction — has been handed to that author rather than merged here. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

Copilot

Copilot review overview

🟡 Changes recommended

Unresolved moderate findings remain in MessageService and the pooled-leaf guard.

Get a fresh assessment by requesting another Copilot review.

Review effort: Lite
Findings: 1 · 1

Open (2)

Detector misses method-group Invoke arguments · New
Use immutable collection for accumulated test findings · New

What changed in this PR

This PR addresses pooled Orleans teardown cancellation and incomplete shutdown NACK metadata.

Changes:

Projects cancellation onto UnsubscribeAsync() and reports abandoned waits.
Composes deferred-delivery refusals through ShutdownNack.
Adds regression tests, a static guard, and architecture documentation.

File	Summary
`test/MeshWeaver.Messaging.Hub.Test/FailedGateAnswersBeforeTheTeardownTest.cs`	Strengthens shutdown-answer assertions.
`test/MeshWeaver.Messaging.Hub.Test/DeferredDeliveryNackedOnDisposeTest.cs`	Tests shutdown NACK metadata.
`test/MeshWeaver.Documentation.Test/PooledLeafObservesItsTokenGuard.cs`	Adds pooled-leaf enforcement; moderate matcher gaps and nit-level mutability concerns remain.
`src/MeshWeaver.Messaging.Hub/MessageService.cs`	Adds shutdown NACK composition; moderate fallback and alternate-producer issues remain.
`src/MeshWeaver.Documentation/Data/Architecture/RidingOutAShuttingDownAddress.md`	Documents NACK composition; a nit requires qualifying the broad invariant or covering another path.
`src/MeshWeaver.Documentation/Data/Architecture/DisposedScopeAndDyingHubs.md`	Documents static sweep limits.
`src/MeshWeaver.Documentation/Data/Architecture/ControlledIoPooling.md`	Documents cancellation projection for tokenless APIs.
`src/MeshWeaver.Connection.Orleans/OrleansRoutingService.cs`	Makes stream unsubscription cancellation-aware.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

rbuergi · 2026-09-22T18:12:34Z

+    private static readonly Regex PooledAsyncLeaf = new(
+        @"\.Invoke\s*\(\s*(?:async\s+)?(\w+)\s*=>",
+        RegexOptions.Compiled | RegexOptions.CultureInvariant);


Accepted, and it is the more serious of the two findings: the guard's CLAIM was wider than its COVERAGE, which on this codebase is the defect class the guard itself exists to prevent — a gate that can pass having checked nothing.

I checked the two sites you name before deciding what to do, because the answer changes which remedy is honest:

PluginBundleClient.FetchIndex — _httpPool.Invoke(ReadIndex), and private async Task<BundleIndex> ReadIndex(CancellationToken ct) does observe it (Receive(request, "Bundle index", ct)). Clean today. There is a second one on the next line, _httpPool.Run(ReadIndex), same method group, same verdict.

IoPoolExtensions.Run — pool.Invoke(io) is a FORWARDER: the delegate is the caller's, and the caller's own lambda is what needs scanning. That one is not a hole in itself.

So nothing is being papered over — but the form is genuinely unverifiable lexically (you would have to resolve the method and read its body), so I am taking BOTH halves of what you offer rather than choosing:

Expanded to Run / RunStream — IoPoolExtensions.Run composes on Invoke, so pool.Run(_ => neverObserves()) reaches the same await io(ct) and was missed entirely. That is a real hole and it is now covered, with a regression case. (Measured before and after: no src/ site offends through that form today, so the zero does not move — the DENOMINATOR does, which is the point.)

Narrowed the claim to what a lexical scan can actually support: the guard now says it verifies the LAMBDA form, and that a method group or delegate variable is deliberately out of scope and unverifiable here. A detector case pins that limitation (TheDetectorCannotSeeAMethodGroup_AndSaysSo), so it is a recorded boundary rather than a forgotten one — the same treatment the in-mesh warning ratchet gives a type it could not compile: warned, never silently counted as passing.

I deliberately did NOT make a non-lambda argument fail. That would forbid a form both the sites above use correctly, and turning a correct pattern red to protect a lexical scan is the wrong trade. If the method-group form ever needs real coverage it wants a call-graph pass or a roslyn analyzer, not a wider regex — and that is now written down at the site instead of implied by silence.

rbuergi · 2026-09-22T18:12:46Z

+    {
+        var root = SourceScan.FindRepoRoot();
+        var scanned = 0;
+        var offenders = new List<string>();


Accepted — the house rule is absolute ("NEVER use mutable collections") and a local is not an exemption, so I have removed both the List<string> and the var scanned = 0 counter rather than swapping the list for a builder.

The restructure is not just a type change: the scan now produces ONE immutable array of (File, Parameter, Observed) in a single LINQ pass, and both readings are derived from it — scanned is its Length, offenders is the projection of the rows where Observed is false. That removes the thing that made the mutable pair tempting in the first place: two numbers being accumulated in lockstep by hand, where a future edit can advance one and forget the other, and the denominator assertion (scanned > 0) silently stops being about the same population as the offender list. Derived from one collection they cannot drift.

Sibling guards in this directory already use the same shape (CachedHubReferenceGuard is SelectMany(...).OrderBy(...).ToArray()), so this also stops the new file being the odd one out.

… no mutable state Two findings from the automatic review on #5281, both accepted. COVERAGE. The detector matched only a lambda immediately after .Invoke(, so two things were outside a guard that claimed the whole class: * IoPoolExtensions.Run composes straight onto Invoke, so pool.Run(_ => …) reaches the same 'await io(ct)' and was not scanned at all. Now in scope — the offender count is unchanged (no src/ site offends through that form) but the DENOMINATOR moves from 30 to 49, which is the half that was missing. * A leaf passed as a METHOD GROUP or delegate variable cannot be decided lexically. That is not fixable with a wider regex, so the CLAIM is narrowed instead: the guard states it verifies the lambda form, and TheDetectorCannotSeeAMethodGroup_AndSaysSo pins the boundary as a test rather than a comment, because a comment cannot fail. A non-lambda argument is deliberately NOT failed — PluginBundleClient.FetchIndex and IoPoolExtensions both use the form correctly (ReadIndex does observe its ct), and turning a correct form red to protect a lexical scan is the wrong trade. DeliveryObservable.Run is excluded by name with both-direction cases: it is MessageHub's handler bridge, not a pool, and passes CancellationToken.None deliberately — matching it would make the guard red on a correct line. MUTABLE STATE. The List<string> and the paired int counter are gone. The scan produces ONE immutable array of (File, Parameter, Observed) and both readings are derived from it, so the denominator cannot drift out of step with the offender list — which is what two hand-advanced counters invite. Controls re-run after the regex change, on the real tree: red 'll 1 of 49 … OrleansRoutingService.cs' with the leaf reverted, green 4 of 4 with it fixed. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

…nt, which the guard no longer matches The guard grew IoPoolExtensions.Run in the review fix; this page still said 'Invoke alone has no projection, which is why it is the one the guard scans'. A doc that contradicts the code it documents is worse than one that says less, and this one is the reference a future author checks before writing a leaf. Also states the guard's lexical boundary here, not only in its own remarks: a method group or delegate variable is outside it by construction, and the reason a non-lambda argument is not failed. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

Swept both pages rather than fixing the one I happened to re-read: the triage page carried the identical sentence, and a claim that is wrong in two places is the one a reader will find in the place nobody checked. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

github-actions · 2026-09-22T18:30:15Z

Test Results (shard 4)

2 files - 1 2 suites - 1 2m 44s ⏱️ - 3m 19s
1 596 tests - 381 1 596 ✅ - 381 0 💤 ±0 0 ❌ ±0
1 596 runs - 382 1 596 ✅ - 382 0 💤 ±0 0 ❌ ±0

Results for commit c69071b. ± Comparison against base commit 636e5cb.

github-actions · 2026-09-22T18:32:55Z

Test Results (shard 2)

1 files - 2 1 suites - 2 5m 53s ⏱️ - 1m 12s
1 954 tests +1 224 1 954 ✅ +1 416 0 💤 - 192 0 ❌ ±0
1 955 runs +1 225 1 955 ✅ +1 417 0 💤 - 192 0 ❌ ±0

Results for commit c69071b. ± Comparison against base commit 636e5cb.

github-actions · 2026-09-22T18:34:22Z

Test Results (shard 5)

2 files - 3 2 suites - 3 7m 26s ⏱️ - 8m 14s
2 505 tests - 1 474 2 505 ✅ - 1 472 0 💤 - 2 0 ❌ ±0
2 509 runs - 1 474 2 509 ✅ - 1 472 0 💤 - 2 0 ❌ ±0

Results for commit c69071b. ± Comparison against base commit 636e5cb.

github-actions · 2026-09-22T18:35:02Z

Test Results

8 files - 9 8 suites - 9 27m 9s ⏱️ - 8m 42s
7 705 tests - 1 355 7 705 ✅ - 1 161 0 💤 - 194 0 ❌ ±0
7 714 runs - 1 355 7 714 ✅ - 1 161 0 💤 - 194 0 ❌ ±0

Results for commit c69071b. ± Comparison against base commit 636e5cb.

Copilot AI lite review requested due to automatic review settings September 22, 2026 18:00

meshweaver-cloud Bot enabled auto-merge September 22, 2026 18:00

This was referenced Sep 22, 2026

Pooled I/O leaf ignores its cancellation token, letting silo teardown dispose the mesh over live work #2480

Closed

Plugin-install hub disposed via direct Dispose() discards a deferred SubscribeRequest behind closed init gates #4888

Closed

Copilot started reviewing on behalf of rbuergi September 22, 2026 18:00

View session

rbuergi changed the title ~~fix: a pooled leaf that cannot be cancelled, and a refusal that was not composed by ShutdownNack~~ fix(#2480): the only pooled leaf in src/ that cannot be settled by the drain, plus a guard for the class Sep 22, 2026

Copilot AI reviewed Sep 22, 2026

View reviewed changes

rbuergi and others added 3 commits September 22, 2026 20:15

meshweaver-cloud Bot added this pull request to the merge queue Sep 22, 2026

rbuergi mentioned this pull request Sep 22, 2026

test(#4866): the activation tag must be THIS activation's, not merely present #5289

Merged

Merged via the queue into main with commit f2f00c9 Sep 22, 2026
38 checks passed

rbuergi deleted the fix/2480-4888-teardown-leaf-and-nack-attribution branch October 10, 2026 13:10

Navigation Menu

Repository navigation

fix(#2480): the only pooled leaf in src/ that cannot be settled by the drain, plus a guard for the class - #5281

fix(#2480): the only pooled leaf in src/ that cannot be settled by the drain, plus a guard for the class#5281
meshweaver-cloud[bot] merged 5 commits into
mainfrom
fix/2480-4888-teardown-leaf-and-nack-attribution

rbuergi commented Sep 22, 2026 •

edited

Loading

Uh oh!

github-actions Bot commented Sep 22, 2026 •

edited

Loading

Uh oh!

github-actions Bot commented Sep 22, 2026 •

edited

Loading

Uh oh!

github-actions Bot commented Sep 22, 2026 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

rbuergi Sep 22, 2026

Uh oh!

rbuergi Sep 22, 2026

Uh oh!

github-actions Bot commented Sep 22, 2026

Uh oh!

github-actions Bot commented Sep 22, 2026

Uh oh!

github-actions Bot commented Sep 22, 2026

Uh oh!

github-actions Bot commented Sep 22, 2026

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

rbuergi commented Sep 22, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

The only pooled leaf in src/ that cannot be settled by the drain

Control

Docs

Notes

Uh oh!

github-actions Bot commented Sep 22, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Test Results (shard 0)

Uh oh!

github-actions Bot commented Sep 22, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Test Results (shard 1)

Uh oh!

github-actions Bot commented Sep 22, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Test Results (shard 3)

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Copilot review overview

🟡 Changes recommended

Uh oh!

rbuergi Sep 22, 2026

Choose a reason for hiding this comment

Uh oh!

rbuergi Sep 22, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions Bot commented Sep 22, 2026

Test Results (shard 4)

Uh oh!

github-actions Bot commented Sep 22, 2026

Test Results (shard 2)

Uh oh!

github-actions Bot commented Sep 22, 2026

Test Results (shard 5)

Uh oh!

github-actions Bot commented Sep 22, 2026

Test Results

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

rbuergi commented Sep 22, 2026 •

edited

Loading

The only pooled leaf in `src/` that cannot be settled by the drain

github-actions Bot commented Sep 22, 2026 •

edited

Loading

github-actions Bot commented Sep 22, 2026 •

edited

Loading

github-actions Bot commented Sep 22, 2026 •

edited

Loading