fix(messaging): close the teardown intake gate at the first instant of disposal

[rbuergi]

The defect

MessageService.ScheduleNotify's shutdown gate refused new deliveries only from
RunLevel >= MessageHubRunLevel.DisposeHostedHubs, so the entire Quiescing phase was wide
open. A hub inside its drain — including one that had already logged [QUIESCE-OK], having
drained everything it owed — kept accepting inbound requests, kept running their handlers, kept
creating hosted hubs for them, and kept registering response callbacks for the sub-requests those
handlers issued.

Work taken on there cannot finish. The quiesce budget is the only time the hub has left and it is
already spent on what it owed at entry; the next phase cancels whatever the new work is waiting
for, and the requester gets HubDisposedBeforeResponseException after burning its full bound.
[QUIESCE-TIMEOUT] … forcibly cancelling is the tell — and this is the root.

Measured on three bake runs (2026-09-06), counting the RECEIVED runLevel=… stage in the
request-fate trails [QUIESCE-TIMEOUT] prints: six of nine pending callbacks at the timeout had
been taken on by a hub that was already Quiescing.

The fix

The gate is now two tiers, and the new one is deliberately narrower than the old, because a
Quiescing hub is still a live poster and a live router:

	Quiescing (new)	DisposeHostedHubs and beyond (unchanged)
ShutdownRequest / DisposeRequest	passes	passes
a reply carrying PostOptions.RequestId	passes — this is what the drain is waiting for	refused
transit to a hosted child	passes — children are alive until the next phase	refused
fire-and-forget nobody awaits	passes — no promise to break, and answering it is the storm shape AnswerPolicy prevents	refused
a NEW request addressed to this hub	refused, ErrorType.ShuttingDown	refused, ErrorType.ShuttingDown

The refusal is the same transient, owner-minted NACK the later tier already posted —
ShutdownNack.RejectingNow, activation identity and all — so a caller reads "ask again at the
fresh activation", never "gone", and ShutdownNack.IsAnsweredByOwner still identifies the speaker.

The admission test is factored out of NackThroughParent as IsAwaitedBySender, so "refused" and
"NACKed" are the same set by construction rather than by two lists staying in step — a refusal the
gate could not answer would be exactly the silent drop the NACK exists to remove. And because a
Quiescing hub can still post (unlike one past DisposeHostedHubs, where Post and
ReportFailure both decline), a root hub with no parent to carry the NACK now answers the
sender itself through ReportFailure instead of falling back to the historical silent drop.

No bound is widened. #3261 settled the other option: a bigger QuiesceTimeout converts a leaked
callback into a slower leaked callback. Teardown still lets ACCEPTED work FINISH — the second test
below is the half that says so.

Verification

test/MeshWeaver.Messaging.Hub.Test/QuiescingHubRefusesNewWorkTest.cs, two [Fact]s on
HubTestBase (no mocking, no Task.Delay, no SemaphoreSlim — the sink→test signal is an
AsyncSubject awaited through .Should().Within(...).Emit(...), and the victim is held in
Quiescing by a parked callback rather than caught there by timing):

1. NewCorrelatedRequest_IsRefused_WhileTheHubIsDrainingItsQuiesce — the NACK is
   ErrorType.ShuttingDown, passes ShutdownNack.IsAnsweredByOwner, contains RunLevel=Quiescing
   (so it pins the new tier, not the pre-existing one), and the victim's handler — which would
   have answered — provably never ran.
2. ReplySettlingWorkAcceptedBeforeQuiesce_StillLands_AndTheDrainCompletes — the reply to a
   callback registered before the drain still lands, and disposal completes well inside
   TestTimeouts.Convergence against a 2-minute held quiesce budget, i.e. the hub left the phase by
   draining ([QUIESCE-OK]) rather than by timing out.

Negative control. With the tier-1 threshold put back to DisposeHostedHubs, test 1 fails
exactly as the defect predicts — Assert.Throws() Failure: No exception was thrown (the quiescing
hub accepted the request and answered it), plus the secondary symptom
Mesh mesh/1: disposal did not complete within 00:00:10, which is the leaked callback itself.
With the fix: Failed: 0, Passed: 2. Whole project green: Failed: 0, Passed: 370.
dotnet build -c Release -warnaserror clean (0 Warning(s), 0 Error(s)) on
MeshWeaver.Messaging.Hub, MeshWeaver.Documentation and the test project.

Public surface

No public type or member removed; nothing added to a public interface. Both new methods
(RefusesIntake, IsAwaitedBySender) are private.

Pairs-with: none — no public surface is removed or changed, so no in-mesh or satellite caller can be affected.

Work products conserved

• src/MeshWeaver.Documentation/Data/Architecture/TeardownLayers.md — new section
  "The intake gate closes at the FIRST instant of disposal, not the third", with the tier table and
  the measurement.
• What's New entry (Category: Fix).

Closes #3506

🤖 Generated with Claude Code

https://claude.ai/code/session_018LzrcSMRzD2R5UcGDk5TKU

[Copilot]

🟡 Changes recommended

The new shared IsAwaitedBySender RawJson exclusion relies on a broad substring match for DeliveryFailure, which can suppress NACK/refusal behavior for unrelated awaited RawJson payloads containing that text.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

This PR tightens MeshWeaver’s messaging teardown behavior by closing the “intake gate” at the start of disposal (Quiescing), preventing hubs that are already draining from accepting new correlated work that cannot complete before teardown progresses.

Changes:

• Introduces a two-tier intake gate in MessageService.ScheduleNotify, refusing new awaited requests during Quiescing while still allowing replies (correlated by PostOptions.RequestId) and transit traffic to hosted children.
• Refactors the “is this awaited by the sender?” admission predicate to be shared by both the intake gate and NackThroughParent, keeping “refused” and “NACKed” sets aligned.
• Adds deterministic tests pinning both halves: (1) new correlated work is refused while draining, and (2) replies to pre-quiesce accepted work still land and disposal completes.

File summaries

File	Description
src/MeshWeaver.Messaging.Hub/MessageService.cs	Adds the Quiescing-tier intake gate and factors shared admission logic used for refusal/NACK behavior during teardown.
test/MeshWeaver.Messaging.Hub.Test/QuiescingHubRefusesNewWorkTest.cs	Adds deterministic regression tests ensuring Quiescing refuses new correlated work but still admits replies that settle existing callbacks.
src/MeshWeaver.Documentation/Data/WhatsNew/2026-09-07-a-hub-that-is-shutting-down-stops-taking-on-new-work.md	Adds a user-facing “What’s New” entry describing the behavioral fix.
src/MeshWeaver.Documentation/Data/Architecture/TeardownLayers.md	Documents the updated teardown intake-gate layering and rationale.

Review details

• Files reviewed: 4/4 changed files
• Comments generated: 1
• Review effort level: Lite

---

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

[rbuergi]

Good catch on the shape, but this is a verbatim move of the pre-existing test out of NackThroughParent (it sat two lines above the intake gate before this diff), and both halves of the concern point the other way here.

The false positive is benign in both callers, and it is the direction that was chosen deliberately.

• At the new Quiescing tier a false positive makes IsAwaitedBySender return false, so RefusesIntake does not refuse — the delivery is accepted and handled exactly as it is on today's main. There is no silent drop: the request is served.
• In NackThroughParent a false positive means "do not NACK", which reverts that one delivery to the historical silent drop. The comment that shipped with Both Orleans routers' answer-once guards are dead code: Package() erases the payload type before they run, so the router NACKs heartbeats and can NACK a NACK #1485 says exactly that, which is why the sniff is a fallback rather than the rule.

And the sniff only ever decides an UNPACKAGED RawJson. Every delivery that crossed a hub boundary went through MessageDelivery.Package, which stamps AnswerPolicy.SuppressedProperty on the envelope before the payload type is erased — so delivery.MayAnswer() (the second half of the &&) already answers "is this a DeliveryFailure / [CanBeIgnored]" authoritatively for those, off the envelope rather than off the bytes. The content test exists solely for a frame that never went through Package: an external client's pre-serialised body, which carries no stamp.

Scoping it to the $type value would trade a benign failure for the storm. The tight form has to match a literal ("$type":"DeliveryFailure"), and the writer's settings decide whether there is a space after the colon. A miss there is not the safe direction: it would let a DeliveryFailure through as "awaited", and NACKing a NACK between two concurrently-disposing hubs is the ping-pong this exclusion exists to prevent — the 465k-DeliveryFailure storm documented in ReportFailure. A brittle exact match guarding against a storm is worse than a loose match whose miss costs one delivery a NACK it was never guaranteed.

If the substring test is worth tightening it should be its own change against AnswerPolicy — reading the discriminator through the JSON parser rather than by string search, for both call sites at once — pinned by a test that feeds it a payload whose user text contains "DeliveryFailure". It is not something to change inside a fix whose whole point is that a teardown refusal must be answerable.

[rbuergi]

Good catch on the shape, but this is a verbatim move of the pre-existing test out of NackThroughParent (it was two lines above the intake gate before this diff), and both halves of the concern point the other way here.

The false positive is benign in both callers, and it is the direction that was chosen deliberately.

• At the new Quiescing tier a false positive means IsAwaitedBySender returns false, so RefusesIntake does not refuse — the delivery is accepted and handled exactly as it is on today's main. There is no silent drop: the request is served.
• In NackThroughParent a false positive means "do not NACK", which reverts that one delivery to the historical silent drop. The comment that shipped with Both Orleans routers' answer-once guards are dead code: Package() erases the payload type before they run, so the router NACKs heartbeats and can NACK a NACK #1485 says exactly that, and it is why the sniff is a fallback rather than the rule.

And the sniff only ever decides an UNPACKAGED RawJson. Every delivery that crossed a hub boundary went through MessageDelivery.Package, which stamps AnswerPolicy.SuppressedProperty on the envelope before the payload type is erased — so delivery.MayAnswer() (the second half of the &&) already answers "is this a DeliveryFailure / [CanBeIgnored]" authoritatively for those, off the envelope rather than off the bytes. The content test exists solely for a frame that never went through Package — an external client's pre-serialised body, which carries no stamp.

Scoping it to the $type value would trade a benign failure for the storm. The tight form has to match a literal ("$type":"DeliveryFailure"), and System.Text.Json writer settings decide whether there is a space after the colon. A miss there is not the safe direction: it would let a DeliveryFailure through as "awaited", and NACKing a NACK between two concurrently-disposing hubs is the ping-pong this exclusion exists to prevent — the 465k-DeliveryFailure storm documented in ReportFailure. A brittle exact match protecting against a storm is worse than a loose match whose miss costs one delivery a NACK it was never guaranteed.

If the substring test is worth tightening it should be its own change against AnswerPolicy — reading the discriminator through the JSON parser rather than by string search, for both call sites at once — and pinned by a test that feeds it a payload whose user text contains "DeliveryFailure". It is not something to change inside a fix whose whole point is that a teardown refusal must be answerable.

[github-actions]

Test Results (shard 1)

492 tests  ±0   492 ✅ ±0   39s ⏱️ -1s
  1 suites ±0     0 💤 ±0 
  1 files   ±0     0 ❌ ±0 

Results for commit 5c7573c. ± Comparison against base commit 1fceaef.

♻️ This comment has been updated with latest results.

[github-actions]

Test Results (shard 0)

251 tests  ±0   251 ✅ ±0   1m 42s ⏱️ -31s
  1 suites ±0     0 💤 ±0 
  1 files   ±0     0 ❌ ±0 

Results for commit 5c7573c. ± Comparison against base commit 1fceaef.

♻️ This comment has been updated with latest results.

[github-actions]

Test Results (shard 3)

1 034 tests  ±0   1 034 ✅ ±0   1m 5s ⏱️ -13s
    2 suites ±0       0 💤 ±0 
    2 files   ±0       0 ❌ ±0 

Results for commit 5c7573c. ± Comparison against base commit 1fceaef.

♻️ This comment has been updated with latest results.

[github-actions]

Test Results (shard 4)

1 629 tests  ±0   1 435 ✅ ±0   2m 10s ⏱️ -2s
    4 suites ±0     194 💤 ±0 
    4 files   ±0       0 ❌ ±0 

Results for commit 5c7573c. ± Comparison against base commit 1fceaef.

♻️ This comment has been updated with latest results.

[github-actions]

Test Results (shard 5)

    5 files  ±0      5 suites  ±0   2m 28s ⏱️ +5s
1 517 tests +1  1 517 ✅ +1  0 💤 ±0  0 ❌ ±0 
1 518 runs  +1  1 518 ✅ +1  0 💤 ±0  0 ❌ ±0 

Results for commit 5c7573c. ± Comparison against base commit 1fceaef.

♻️ This comment has been updated with latest results.

[github-actions]

Test Results (shard 2)

1 117 tests  +2   1 117 ✅ +2   3m 58s ⏱️ -6s
    3 suites ±0       0 💤 ±0 
    3 files   ±0       0 ❌ ±0 

Results for commit 5c7573c. ± Comparison against base commit 1fceaef.

♻️ This comment has been updated with latest results.

[rbuergi]

CI found the one thing local runs could not: a test built on the precondition this fix removes

The first run was green everywhere except Run tests (shard 5), on
MeshWeaver.Layout.Test.SubscribeDuringRecycleTest. Not a flake, and not collateral — that test's
own class summary states the defect as its fixture:

Message intake, meanwhile, stays open until DisposeHostedHubs — the whole Quiescing drain sits
inside the gap. So a disposing hub happily ACCEPTS work it can no longer perform.

That is #3506, written down and depended on. With the gate closed the subscribe is turned away at
the door, so it never reaches LayoutAreaHost and the assertion "names HubDisposingException, and
was not answered by MessageService's intake NACK" can no longer hold.

The seam it guards is real and still reachable, so it is now pinned properly rather than
relaxed. MessageHub.Dispose freezes hosted-hub creation synchronously, on its first statement,
and only then posts the ShutdownRequest that moves RunLevel off Started — so a delivery
admitted at Started can still reach a handler that can no longer create the sub-hub it needs. The
intake gate cannot help there by construction: the delivery was already in the queue when the door
closed. The test now pins both doors:

• Door one (the original subject, re-aimed): accepted at Started, executed after the freeze.
  Made deterministic the way DisposalRaceNackTest does it — a Blocker parks the turn loop,
  Dispose() is called (creation freezes synchronously while its ShutdownRequest queues behind
  the blocker, so RunLevel is asserted to still be Started), the subscribe is admitted, and the
  fate ledger is polled for ENQUEUED@area/1 before the blocker is released — otherwise the
  shutdown could run first and hand the test the intake refusal, a correct answer to a different
  question and a flake decided by a routing race. Assertions unchanged: ShuttingDown, names
  HubDisposingException, never NullReferenceException.
• Door two (new): the subscribe that merely arrives during the drain. Refused at the door, and
  asserted on what a subscriber actually acts on — ShuttingDown, ShutdownNack.IsAnsweredByOwner,
  RunLevel=Quiescing in the banner, and still no NullReferenceException. The sync stream's
  resubscribe latch survives exactly as before; what changed is that the hub no longer takes the
  work on first.

Two follow-ups came out of the same read:

• TeardownLayers.md now states the distinction — the gate is at intake, so it cannot un-accept
  a queued delivery, and the freeze-before-Quiescing window is a second, narrower one it does not
  touch, answered one layer down by design.
• HubRunLevelObservableTest's class summary described the window as "…but intake is still open".
  That sentence was the defect, asserted as fact in a test file — the same prose drift that left
  SubscribeDuringRecycleTest's fixture obsolete without anyone noticing. Corrected; the test
  asserts RunLevelChanged emissions and is otherwise untouched.

Local after the change: SubscribeDuringRecycleTest 2/2 · MeshWeaver.Layout.Test 451/451 ·
MeshWeaver.Hosting.Test 441/441 · MeshWeaver.Messaging.Hub.Test 370/370 ·
MeshWeaver.Documentation.Test 304/304. dotnet build -c Release -warnaserror clean on
MeshWeaver.Messaging.Hub, MeshWeaver.Hosting.Monolith, Memex.Portal.Shared,
MeshWeaver.Documentation, MeshWeaver.Layout.Test, MeshWeaver.Hosting.Test and
MeshWeaver.Messaging.Hub.Test.

[rbuergi]

Second CI red, root-caused rather than re-run: the log collector disagreed with itself

Run 34102515449, Run tests (shard 4) — and not on anything the test was measuring. The
LATE_NACK_REENQUEUE line it waits for had already been logged (it is right there in the captured
output). The failure is inside the collector:

System.ArgumentException : Value does not fall within the expected range.
   at System.Collections.Immutable.ImmutableExtensions.ToArray[T](IEnumerable`1, Int32)
   at System.Collections.Immutable.ImmutableArray.CreateRange[T](IEnumerable`1)
   at …CapturingLoggerProvider…FirstMatching…b__0(Int64)

lines is a ConcurrentQueue<string> that the hub's action-block thread writes to while the poller
reads it. lines.ToImmutableArray() goes through ImmutableArray.CreateRange, which reads the
sequence's Count, allocates exactly that many slots, and only then enumerates — two separate
reads of a moving collection. One enqueue in between yields more items than slots, and
ImmutableExtensions.ToArray throws. Nothing about the queue is unsafe; the materialisation is,
because it asks the queue two questions and requires the answers to agree.

Fixed by enumerating the queue directly — its own enumerator is a single moment-in-time snapshot, so
it cannot disagree with itself. The poll shape the surrounding remarks argue for (never await a live
Subject, because the continuation would resume on the hub's thread) is untouched; only the
snapshot mechanism changed.

Attributed, not assumed. Shard 4 was green on this PR's previous run (34099955896) and red
on 34102515449 with the same code under test — the signature of a race, not a break — and the stack
names the racing line exactly. Swept the rest of test/ for the same shape: every other
ToImmutableArray() sits on a static file list or an already-finished LINQ pipeline, none on a live
concurrent collection.

Local after the change: LateNackReenqueue* 2/2, MeshWeaver.Graph.Test 1387/1387,
dotnet build -c Release -warnaserror clean.

[github-actions]

Test Results

   16 files  ±0     16 suites  ±0   12m 4s ⏱️ -47s
6 040 tests +3  5 846 ✅ +3  194 💤 ±0  0 ❌ ±0 
6 041 runs  +3  5 847 ✅ +3  194 💤 ±0  0 ❌ ±0 

Results for commit 5c7573c. ± Comparison against base commit 1fceaef.

♻️ This comment has been updated with latest results.