Repository navigation
fix: a recycle now reaches the subscribers it orphans (#2551) - #2582
Merged
Merged
Conversation
…them
`WithOverlaySelfHeal` posts a bare `DisposeRequest` the moment a NodeType reaches a
usable build, on the premise that "the next access re-enriches against the settled
type". An already-open subscription is not a next access — and the self-heal's whole
population is the viewer currently looking at the compile-progress overlay.
`JsonSynchronizationStream`'s `StreamEndedEvent` is deliberately suppressed once the
owning hub is winding down ("a hub must speak only for itself, and never while it is
dying" — a dying owner reaching up the tree resurrects the Orleans activation it is
retiring). It delegates that case to the recycle re-arm and the change-feed latch, and
neither can fire for a recycle: the re-arm needs an in-flight SubscribeRequest to be
NACKed, and the latch needs a WRITE — "a recycle IS NOT A WRITE". So the subscriber got
no frame, no completion and no error, and held its last snapshot forever. On a
framework-identity bump that is every open page in the portal at once, since the bump
recompiles every dynamic NodeType.
The fix is the ordering: announce on the recycle's own turn, while the hub is whole, and
DELIVER after DisposalCompleted through the parent hub, which outlives it. A new
`RecycleAnnouncement` seam is invoked by `MessageHub.HandleDispose` before `Dispose()`
(never for an ancestor's cascade, where the address is not coming back); `Workspace`
implements it over the client-subscription registry that knows who is listening. No bound
raised, no timer, poll, retry or watchdog added — DisposalCompleted is the event and the
re-ask it feeds is the pre-existing bounded ladder.
Deterministic repro `RecycleStrandsLiveSubscriberTest` fails on main after its full 60 s
wait and passes in 659 ms with the fix; `RecycleAnnouncementTest` pins the seam's firing
contract.
Closes #2551
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
This PR fixes a lifecycle gap where a message-routed hub recycle (DisposeRequest) could strand already-established client subscriptions by tearing down the owner without emitting any terminal signal, leaving the subscriber stuck on its last snapshot (e.g., the compile-progress overlay) until manual reload.
Changes:
- Introduces a
RecycleAnnouncementseam and invokes it fromMessageHub.HandleDispose(only for true “recycle” cases, not shutdown cascades) before disposal begins. - Implements announcement delivery in
Workspaceby snapshotting live client subscriptions and, afterDisposalCompleted, postingStreamEndedEventvia the parent hub to trigger the existing bounded re-ask path. - Adds deterministic regression tests and documents the disposal/recycle contract (plus a What’s New entry).
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| test/MeshWeaver.Messaging.Hub.Test/RecycleAnnouncementTest.cs | Pins the new recycle-announcement firing contract (once, before teardown; not on direct Dispose()). |
| test/MeshWeaver.Hosting.Monolith.Test/RecycleStrandsLiveSubscriberTest.cs | End-to-end repro asserting a live layout-area subscription re-converges after a bare DisposeRequest recycle. |
| src/MeshWeaver.Messaging.Hub/MessageHub.cs | Announces recycle on routed DisposeRequest (guarded by IsShuttingDown), then disposes. |
| src/MeshWeaver.Messaging.Contract/RecycleAnnouncement.cs | Defines the new seam type hung on hubs to capture the “announce-before-dispose” turn. |
| src/MeshWeaver.Data/Workspace.cs | Registers and implements announcement delivery to orphaned client subscriptions via parent hub after DisposalCompleted. |
| src/MeshWeaver.Documentation/Data/WhatsNew/2026-08-28-open-pages-recover-when-their-hub-restarts.md | User-facing release note for the recovery behavior. |
| src/MeshWeaver.Documentation/Data/Architecture/HubDisposalModel.md | Documents the recycle “goodbye” requirement and the new delivery model. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Contributor
Test Results (shard 2)1 087 tests 895 ✅ 5m 51s ⏱️ Results for commit 0d57089. |
Contributor
Test Results (shard 1)742 tests 738 ✅ 6m 44s ⏱️ Results for commit 0d57089. |
Contributor
Test Results (shard 0)1 317 tests 1 317 ✅ 6m 22s ⏱️ Results for commit 0d57089. |
Contributor
Test Results (shard 3)1 134 tests 1 134 ✅ 16m 28s ⏱️ Results for commit 0d57089. |
Contributor
Test Results (shard 5)1 612 tests 1 611 ✅ 9m 50s ⏱️ Results for commit 0d57089. |
Contributor
Test Results (shard 4) 8 files 8 suites 11m 32s ⏱️ Results for commit 0d57089. |
Contributor
Test Results 44 files 44 suites 56m 50s ⏱️ Results for commit 0d57089. |
This was referenced Aug 28, 2026
rbuergi
added a commit
that referenced
this pull request
Sep 1, 2026
…vious fix loses writes #2776 asked one question: what ended the activity node's two sync streams while the activity was still running. This answers it, records the measurement that shows the obvious cure is worse than the disease, and makes the diagnostic that caused the mis-triage state its own age. It changes NO behaviour. ── the answer ── Its own previous progress write did, at +0.12 s. IMeshChangeFeed is published after EVERY create/update/delete, and Workspace.EvictForPath drops every cached remote stream whose owner matches the changed path — including the mirror the writer itself is using. A write leases that mirror only for its Observable.Create subscription, so the lease is released moments after its own change event lands; the evicted stream then has no declared holder and ReclaimIfUnheld DISPOSES it — UnsubscribeRequest to the owner, both sync/{id} hubs gone — and the owner then announces StreamEndedEvent to a subscriber that no longer exists. Measured: six progress writes to one activity node with one live reader mint 7 client-side and 11 owner-side sync/ hubs, with 5 mirrors disposed mid-run. ── the two arithmetic traps that made it read as a hub teardown ── 1. The drop line is written FIVE SECONDS after the event it describes: RouteStreamMessage holds a StreamMessage whose sync/{id} hub is missing for SyncStreamOptions.SyncHubRegistrationGrace (5 s) and logs only on that Timeout. In #2776 the streams ended at +0.12 s — confirmed independently by the failure's own arithmetic (FAILED seq=3 elapsedMs=31015.7 at 13:28:58.952 ⇒ EnteredAt 13:28:27.936). The ADVANCE_WITHOUT_HANDOFF beside it is a DIFFERENT 5 s bound (QueueAdvanceBound) counting from the same instant. Two unrelated five-second bounds produced a coincidence that looked like the hosted-hub drain cap. 2. The SENDER rules out a recycle. The recycle announcement is posted by a carrier (a dying hub must never speak for itself); the healthy-owner announcement is posted by the owner and refuses to fire once RunLevel > Started. Both #2776 lines name the activity node — so the owner was Started and NOT disposing. #2763/#2582 are eliminated by the trace, not by argument. ── why the obvious fix is NOT taken ── Giving EvictForPath the liveness gate its two sibling listeners already have (MeshNodeStreamCache.ResetFailureState: "A healthy live entry is left untouched"; JsonSynchronizationStream's version-gated Resubscribe: "resubscribing on it is pure churn") was implemented and measured, and it LOSES WRITES. The eviction is incidentally what keeps a cross-hub writer's BASE current with respect to writes the OWNER makes for itself: _pendingSelfWrites carries only this cache's writes forward, while an owner-side write (an activity's messageCount, a sealed segment) reaches the mirror only through the async fan-out. Controlled, same box, DOTNET_PROCESSOR_COUNT=4, on StaticRepoImportActivityWriteCountTest.AppendCost_DoesNotGrowWithTheLengthOfTheActivity: gate ON FAILED — 2000 messages appended, 1975 recorded (a 25-message batch lost) gate OFF PASSED and on CI the gated build turned 80 append calls into 99 writes with 44 OWNER_NACK_REENQUEUE / "MergeGuard: refused stale/reordered cross-hub write to 'messageCount'". The O(1) append pin (6a48f9f, d1a239a) was right and is not touched. EvictForPath now carries a comment saying the unconditional eviction is deliberate, and why. ── what DOES change ── LogStreamMessageDrop states how long the message was HELD, so its timestamp can never again be read as the time the stream ended. That is the whole behavioural diff; the rest is a doc page and comments. ── the real fix, for whoever takes it ── Make the writer's base version-aware (MeshChangeEvent already carries Version) so a write waits for the mirror to reach the announced version instead of throwing the mirror away to force a fresh snapshot. That is a change to the write path, not to EvictForPath. ── not claimed ── #2776's visible failure was a write with no verdict at 31 s. That silence has a separate, already-fixed cause (#2882 / #2890, 4d23117 — the write registered its response subject after posting; the trail could only say REGISTERED_AFTER_POST, which is exactly what that trace says). Verified: Release -warnaserror 0/0 for Graph.Test and Documentation.Test (which transitively build MeshWeaver.Data, Hosting, Mesh.Contract, Graph, Layout, Messaging.Hub, Documentation). Graph.Test 1086/1086 and Documentation.Test 166/166 at DOTNET_PROCESSOR_COUNT=4; earlier full runs on this tree also had Data.Test 419/419 and Hosting.Test 274/274. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SD7aiLSo59xng2TzU32xEa
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
A recycle tore the hub down under the exact audience it exists for
NodeTypeEnrichmentHelpers.WithOverlaySelfHealposts a bareDisposeRequestthe moment aNodeType reaches a usable build, on the stated premise that "the next access re-enriches against
the settled type". An already-open subscription is not a next access — and the self-heal's
whole population is the viewer who is currently looking at the compile-progress overlay.
The exact edge
JsonSynchronizationStreamemitsStreamEndedEventfrom the server-side stream's disposal, andthat emission is deliberately suppressed once the owning hub is winding down:
It delegates the owning-hub-tearing-down case to two other recoveries. Neither can fire for a
recycle, and the same file says so 600 lines earlier:
SubscribeRequestto be NACKed — an establishedsubscription has none;
So the delegation is to a mechanism that does not cover the case. The subscriber gets no frame,
no completion and no error, and its mirror serves the last snapshot it ever saw forever. That is
the "burst of work, then total silence" signature in run
33150985489: the compile SUCCEEDED at07:39:04.9and the type was usable at07:39:05.2— 43 s before the test gave up.StreamEndedEvent's own docstring already promises this case ("the owning hub tearing down —deactivation, recycle, restart"). The emitter refuses to send it in exactly that case. The gap
between those two comments is the bug.
Why it is a production risk, not a test nuisance
A framework-identity bump recompiles every dynamic NodeType fleet-wide, so on that deploy every
instance hub takes the cold-compile path, crosses the 5 s overlay grace, is overlaid and is then
recycled — and every user with a page open at that moment is stranded on the progress page until
they reload. #2568 (removing
MeshWeaver.AIfromFrameworkBuildIdentity.ContentSurfaceAssemblies)flips exactly that identity, which is why the FutuRe test fails 2/2 there and only rarely on
main, where a warm assembly cache keeps the compile under 5 s.Only the automatic recycles were affected, which is why this hid for so long:
MeshOperations.Recycle(the MCP tool) publishes aMeshChangeEventfor the path before itsDisposeRequest— that fed the change-feed latch and made the recycle path look healthy — andRecycleLayoutAreais driven by the page shell, which navigates on its own. The self-heal posts thedispose alone.
The fix — announce before the teardown, deliver after it
A new
RecycleAnnouncementseam (MeshWeaver.Messaging.Contract), hung on a hub withhub.Set(...)and invoked byMessageHub.HandleDisposeon the recycle's own turn:Workspaceregisters the one real implementation, because the client-subscription registry thatknows who is listening lives there. Three properties, each load-bearing:
Dispose()Configuration.ParentHubstill resolves. Both are gone or unreliable a phase later.DisposalCompleted, through the PARENT hubShuttingDownand burns a budget meant for a genuinely non-converging owner (the #1360 shape). Delivered by the dying hub itself it is the up-the-tree post that resurrects the activation. The parent outlives the target and speaks to a third party.Dispose()— howHostedHubsCollectiontears its children down — never reachesHandleDisposeat all, so a whole-tree teardown stays silent by construction.No band-aid: no bound was raised, no timer, poll, retry or watchdog was added.
DisposalCompletedis the event; the re-ask it feeds is the pre-existing, bounded
MaxRecycleReArmsladder. A recycleof a hub nobody is subscribed to costs exactly what it did before.
Verification
Deterministic repro, written first and confirmed red on
main—RecycleStrandsLiveSubscriberTest(Hosting.Monolith.Test): a client holds a live layout-areasubscription to a static-NodeType instance whose area renders a per-activation marker; a bare
DisposeRequestrecycles it; the SAME subscription must land on the re-activated hub, with no nodewrite anywhere.
ACTIVATION_1— and the log shows thehub re-activating on its own 4.8 s in (the heartbeat), with the subscriber never told.
RecycleAnnouncementTest(Messaging.Hub.Test) pins the seam: announced exactly once and beforethe teardown starts (
IsDisposing == falseinside the callback), and silent on a directDispose().Local runs (all green):
MeshWeaver.Messaging.Hub.TestMeshWeaver.Data.TestMeshWeaver.Layout.TestSubscribeDuringRecycleTest)MeshWeaver.FutuRe.TestMeshWeaver.Hosting.Monolith.TestMeshWeaver.Documentation.TestMeshWeaver.Hosting.Orleans.Testdotnet build … -c Release -warnaserrorclean forMeshWeaver.Messaging.Contract,MeshWeaver.Messaging.Hub,MeshWeaver.Data,MeshWeaver.Graph,MeshWeaver.Layout,MeshWeaver.Hosting,MeshWeaver.Documentation.Docs:
HubDisposalModel.mdgains "A recycle owes its subscribers a goodbye". What's New entryshipped (
Category: Fix).Closes #2551. Very likely also Systemorph/MeshWeaver.Plugins#1141 (same test, same mechanism — a waiter that never observes a
terminal state reports whatever it last held).
🤖 Generated with Claude Code