fix(mesh): issue the recycle teardown off the router, and derive the lifecycle guard's denominator (#4463) by rbuergi · Pull Request #4477 · Systemorph/MeshWeaver

rbuergi · 2026-09-16T07:27:43Z

Fixes #4463. (#4464 is a same-minute duplicate of it, already closed.)

The call site

MeshOperations.RecycleCore posted its DisposeRequest straight off the DI-injected IMessageHub,
which in the mesh's root container IS the router. The teardown left stamped Sender = mesh/{id}
and put node-hub lifecycle on the one action block that must never queue — the 2026-06-11
portal-wedge shape, measured in production 2026-09-16 01:10:13Z:

ROUTER_TRAFFIC ORIGIN: DisposeRequest was POSTED with the mesh hub as sender
  (sender: mesh/Mw6ZXCJcUEmJTXAXoeuGxw, target: Hosting/Triage/ci-failure/systemorph-meshweaver-35041090763)
     at MeshWeaver.AI.MeshOperations+<>c__DisplayClass95_0.<RecycleCore>b__3 (MeshOperations.cs:4091)

It now hops onto MeshExtensions.NodeOperationIssuingHub(), which is the identity function for
every hub whose address type is not the mesh type — so the MCP / session / portal callers that
dominate this surface are byte-for-byte unaffected.

HubRecycleExtensions.RecycleNode — the framework's one recycle surface — carried the same shape
with no production sighting, and hops too. It is a public extension on IMessageHub, so its
documented rule ("the caller holds a hub that outlives the target") constrains who should call it,
not who can; adopting the seam costs the documented callers nothing and removes the undocumented
one.

The half that matters more: the guard could not see this

RouterAsNodeOperationOriginRatchetGuard keyed on a literal list of six node-CRUD request names.
DisposeRequest — a teardown, the most router-hostile lifecycle message there is — was not on it, so
this site was invisible to the guard and only the runtime ORIGIN detector caught it. Adding one more
name buys exactly one more message: the list is the trap one level along.

The denominator is now DERIVED from production, not restated in the test. A message is lifecycle
when the framework itself registers a lifecycle handler for it, and there are exactly two such
registrations:

family	read from	today
hub lifecycle	every `Register<T>(…)` in `MessageHub`'s constructor — what a hub answers because it is a hub	`DisposeRequest`, `ShutdownRequest`, `PingRequest`, `InitializeHubRequest`
node lifecycle	every `.WithHandler<T>(…)` in `MeshExtensions.WithNodeOperationHandlers` — what the off-router node-CRUD execution hub answers	`CreateNodeRequest`, `CreateNodesRequest`, `CreateOrUpdateNodeRequest`, `DeleteNodeRequest`, `ValidateDeleteRequest`, `MoveNodeRequest`, `CopyNodeRequest`

Why that is the right denominator: you cannot add a lifecycle message without passing through one
of those two registrations — a handler is what makes a message lifecycle in the first place. A
marker attribute, or a name in a test, can be forgotten; a handler cannot, because without one the
message does nothing. On its first run the derivation picked up ValidateDeleteRequest, which the
hand-written list had already missed.

The exclusions are sourced the same way: the derived verbs are filtered through
RouterTrafficRule itself — the pure predicate both runtime detector sites evaluate — so
HeartBeatEvent (which WithNodeOperationHandlers also registers, and which the rule calls
routing's own liveness) leaves the denominator automatically, and a future exclusion added there is
inherited rather than copied. Every derived name must also resolve to a real type, so a rename or
a move reddens the guard instead of silently shrinking what it measures.

One structural exclusion — a self-directed hub-lifecycle post (no target, or
o.WithTarget(thatHub.Address)) — and not because the detector is quiet about it: because the
remedy does not apply. The seam's whole effect is to issue the delivery from a different hub,
which for a self-directed teardown would send it to the wrong hub and change what is torn down. A
rule whose fix is nonsense at a site must not report that site. (NodeTypeRebindWatcher, the
stale-build convergence, the overlay self-heal, and LayoutAreaHost's own InitializeHubRequest.)
The node family is deliberately not excluded that way: a target-less CreateNodeRequest on the
router is the literal prod wedge, and the seam does fix it.

Measured

29 lifecycle post sites in the denominator (up from 23 node-CRUD-only), 4 self-directed
hub-lifecycle posts excluded, 1 allowed violation.
That one allowance is the pre-existing #981 entry, unchanged. TotalBudget is still 1.
No new allowances — the two sites the widening surfaced were fixed, not listed.

Controls — each armed, shown RED, disarmed, rebuilt, shown GREEN

Remove the seam at RecycleCore → red:
NEW SITE src/MeshWeaver.Mesh.Operations/MeshOperations.cs (1) …
src/MeshWeaver.Mesh.Operations/MeshOperations.cs:4104 DisposeRequest posted on 'hub'
Break the hub-lifecycle derivation (as a rename would) → the ratchet went green over 25
sites with DisposeRequest unguarded, and only the derivation test went red. That is exactly why
they are two tests that fail independently: a broken derivation makes the ratchet pass over an
unguarded tree.

Also run green: DocumentationLinkIntegrityTest (3/3 with the guard),
RecycleNodeWaitsForTheInstallHoldingTheRootTest (2/2),
OperatorRecycleWaitsForTheInstallHoldingTheRootTest + SessionDenialIsAnAnswerTest (15/15) — both
recycle paths end to end. dotnet build -c Release -warnaserror clean (0 Error(s)) on
MeshWeaver.Mesh.Contract, MeshWeaver.Mesh.Operations, MeshWeaver.Documentation,
MeshWeaver.Documentation.Test, MeshWeaver.Graph.Test, Memex.Portal.Shared.Test.

Review round (`dc37098`)

All three findings from the automatic review were acted on — details in the PR comment.

Matcher could be evaded by a syntax-only refactor (valid): new A.B.DisposeRequest, a
target-typed DisposeRequest request = new();, hub.Post<DisposeRequest>(new()) and a message
arriving as a parameter all produced no site. All four are matched now, one planted case each.
Measured: the widening adds zero sites over src/ and the denominator stays 29/1.
Control: reverting the two new rules reds the spelling cases while the main ratchet stays green.
Both fixes were pinned only by SOURCE (valid): every existing test of both verbs runs through a
session/client hub where the seam is the identity function, so a revert would have left them green.
Two runtime tests now drive both recycle surfaces from the root mesh hub and read the detector's
own output. Reverted in rehearsal they reproduce RecycleCore posts DisposeRequest from the root mesh hub instead of the NodeOperationIssuingHub seam #4463's line down to the frame
(…<RecycleCore>b__3), and each asserts a positive anchor first so the silence is not vacuous.
Ordering race on RecycleNode (real, not introduced here): a root-hub caller already had
two independent hubs before this change (mesh/{id} + portal/reads-*); for every documented
caller both seams are the identity function, so both deliveries still leave the same hub. The
precondition is now stated on the code and in the doc rather than assumed, alongside this class's
existing named residual. Closing it means an explicit disposal-completion barrier — a different
change.

Is #1140 the same defect?

Same family, not the same defect — and this does not close it. #1140's evidence is
receiver-side, where a routed payload arrives packed, so its type list is mostly the honest but
useless RawJson; its stated hypothesis (portal hubs not assigning their own address) was wrong; and
the bulk of its 1,386 lines was the HOP mis-keying, already fixed by keying the rule on the
delivery's own ends. Of the message families it names, ValidateTokenRequest is already on
ReadIssuingHub, and ClickedEvent / PatchDataChangeRequest have no new … site in src/ at
all. What is left, measured 2026-09-16: JsonSynchronizationStream posts
new UnsubscribeRequest(reduced.StreamId) on its own hub. Hopping that is not a no-op — it
would change which hub the unsubscribe originates from, which is a correlation question rather than a
routing one — so it is named as a residue in the doc rather than half-fixed here. #1140 stays open.

Docs

Doc/Architecture/RouterTrafficDetection gains the derivation, the structural exclusion, the two
recycle callers, and the #1140 verdict above. What's New entry: Recycling a node no longer competes
with the portal's routing (Category: Fix).

Pairs-with: none — this diff removes no public top-level type and no public member from src/; it adds comments and changes one receiver expression at two call sites.
Implementers: none — no member is added to a public interface or abstract class.
Mirror-sync: none — no i18n catalog key or value is touched.

🤖 Generated with Claude Code

…guard's denominator MeshOperations.RecycleCore posted its DisposeRequest straight off the DI-injected IMessageHub, which in the mesh's root container IS the router. The teardown therefore left stamped `Sender = mesh/{id}` and put node-hub lifecycle on the one action block that must never queue — the 2026-06-11 portal-wedge shape, measured in production 2026-09-16 01:10:13Z as `ROUTER_TRAFFIC ORIGIN: DisposeRequest was POSTED with the mesh hub as sender` with that exact frame on top (#4463). It now hops onto MeshExtensions.NodeOperationIssuingHub(), the identity function for every hub that is not the router. HubRecycleExtensions.RecycleNode — the framework's ONE recycle surface — had the same shape with no production sighting. It is a public extension on IMessageHub, so its documented rule ("the caller holds a hub that outlives the target") constrains who should call it, not who can; it hops too, at zero cost to every documented caller. The half that matters more: the ratchet that exists to stop this class keyed on a LITERAL list of six node-CRUD request names, so DisposeRequest — the most router-hostile lifecycle message there is — was invisible to it and only the runtime detector caught the site. Enumerating more names buys exactly one more message, so the denominator is now DERIVED from production instead of restated in the test: * hub lifecycle <- every Register<T>(...) in MessageHub's CONSTRUCTOR (DisposeRequest, ShutdownRequest, PingRequest, InitializeHubRequest) * node lifecycle <- every .WithHandler<T>(...) in MeshExtensions.WithNodeOperationHandlers You cannot add a lifecycle message without passing through one of those two registrations — a handler is what makes a message lifecycle — whereas a name in a test can be forgotten. On its first run the derivation picked up ValidateDeleteRequest, which the hand-written list had already missed. The exclusions are sourced the same way: the derived verbs are filtered through RouterTrafficRule itself, so HeartBeatEvent (which WithNodeOperationHandlers also registers) leaves automatically, and every derived name must resolve to a real type so a rename reddens the guard instead of silently shrinking it. One structural exclusion: a SELF-DIRECTED hub-lifecycle post (no target, or o.WithTarget(thatHub.Address)) is out, because the seam's remedy — issue it from a different hub — would misdeliver it. The node family is deliberately not excluded that way: a target-less CreateNodeRequest on the router is the prod wedge, and the seam does fix it. Measured over src/: 29 lifecycle post sites in the denominator (up from 23 node-CRUD-only), 4 self-directed hub-lifecycle posts excluded, and one allowed violation — the pre-existing #981 entry, unchanged, TotalBudget still 1. No new allowances. Controls (each armed, shown RED, disarmed, rebuilt, shown GREEN): * removing the seam at RecycleCore -> "NEW SITE src/MeshWeaver.Mesh.Operations/MeshOperations.cs (1) ... DisposeRequest posted on 'hub'" * breaking the hub-lifecycle derivation -> the ratchet went GREEN over 25 sites with DisposeRequest unguarded, and only the derivation test went red, which is exactly why they are two tests. #1140 is the same defect FAMILY and not the same defect: its evidence is receiver-side (so its type list is mostly the packed `RawJson`), its stated hypothesis was wrong, and its residue includes JsonSynchronizationStream's UnsubscribeRequest, which has no seam and whose origin hub is a correlation question rather than a routing one. It stays open; the doc page now says so. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

The guard resolves every derived verb name to a real Type so that a rename reds it rather than silently shrinking what it measures. That only holds while MeshWeaver.Messaging.Hub and MeshWeaver.Mesh.Contract are reachable, and until now they arrived only transitively. Naming them follows this project's own convention for a guard that references what it pins. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

…d denominator The 'five' and 'six' in the remarks were measured against the old node-CRUD-only scan and were stale the moment the denominator grew. Measured over the current tree: 33 matched sites, 4 self-directed (29 in the denominator), 7 that a construction-anchored scan would miss, 11 that a literal-seam-call receiver test would misclassify. A guard whose stated tolerances are stale reads as evidence and is not. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

Copilot

🟡 Changes recommended

Critical ordering and ratchet-coverage issues remain, with missing root-router regression tests.

Get a fresh assessment by requesting another Copilot review.

Pull request overview

Moves recycle teardown posts off the mesh router and derives lifecycle-origin coverage from production registrations.

Changes:

Routes recycle paths through NodeOperationIssuingHub().
Expands and validates lifecycle ratchet coverage.
Updates architecture documentation, allowlists, and release notes.

File summaries

File	Review summary
`test/RouterNodeOperationOriginSites.allow`	Updates the documented ratchet scope and retained allowance.
`test/MeshWeaver.Documentation.Test/RouterAsNodeOperationOriginRatchetGuard.cs`	Critical (1 vote): matcher misses target-typed and qualified constructions; extend matching or use syntax analysis with planted cases.
`src/MeshWeaver.Mesh.Operations/MeshOperations.cs`	Moderate (1 vote): add a root-router regression test covering teardown completion and absence of router traffic.
`src/MeshWeaver.Mesh.Contract/HubRecycleExtensions.cs`	Critical (2 votes): root-hub reads and teardown posts use independent hubs, allowing ordering races; use the same hub or an explicit barrier. Moderate (1 vote): add root-hub coverage with router-traffic capture.
`src/MeshWeaver.Documentation/Data/WhatsNew/2026-09-16-recycling-a-node-no-longer-competes-with-the-portals-routing.md`	Adds the user-facing release note.
`src/MeshWeaver.Documentation/Data/Architecture/RouterTrafficDetection.md`	Documents lifecycle derivation, exclusions, and remaining scope.

Review details

Suppressed comments (3)

src/MeshWeaver.Mesh.Contract/HubRecycleExtensions.cs:113

On a root-hub caller these two operations now use different issuing hubs: this post runs on NodeOperationIssuingHub() (portal/nodeops-*), while hub.GetMeshNode(...) internally switches a root caller to ReadIssuingHub() (portal/reads-*). Those action blocks are independent, so the read can overtake the teardown and return the still-live node, causing RecycleNode to emit before a fresh activation. Issue both operations through the same off-router read hub (or add an explicit disposal-completion barrier); the ordering comment is only valid for non-router callers.

                hub.NodeOperationIssuingHub().Post(
                    new DisposeRequest
                    {
                        Reason = reason
                                 ?? $"HubRecycleExtensions.RecycleNode: {hub.Address} asked for this "
                                    + "address to be recycled and is waiting for a fresh activation to "
                                    + "answer",
                    },
                    o => o.WithTarget(new Address(path)));

src/MeshWeaver.Mesh.Contract/HubRecycleExtensions.cs:106

The framework recycle tests explicitly invoke RecycleNode through RequestHub, whose non-mesh address makes this seam a no-op; they do not exercise the root-router branch introduced here. A regression to posting directly on hub would therefore keep the existing recycle tests green, so add a root-hub invocation with the router-traffic capture as the negative control.

                hub.NodeOperationIssuingHub().Post(
                    new DisposeRequest

src/MeshWeaver.Mesh.Operations/MeshOperations.cs:4104

The operator integration tests invoke MeshOperations through SessionHub, whose non-mesh address makes NodeOperationIssuingHub() the identity function, so they never execute the root-router branch changed here. Reverting this call to hub.Post would leave those tests green; add a root-hub recycle case that asserts the teardown still completes without origin/target router traffic, so this fix is runtime-pinned rather than only protected by the source ratchet.

                hub.NodeOperationIssuingHub()
                    .Post(

Files reviewed: 7/7 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 · 2026-09-16T07:48:00Z

+            NewRequest = new Regex(@"\bnew\s+(" + alternation + @")\b");
+            RequestLocal = new Regex(
+                @"\b([A-Za-z_][A-Za-z0-9_]*)\s*=\s*new\s+(" + alternation + @")\b");


Valid, and fixed in dc37098 — this was the same trap the PR is about, one level along again.

All four spellings are matched now, each with its own planted case so a regression names the construction rather than moving a count by one:

new MeshWeaver.Messaging.DisposeRequest { … } — the matcher now allows a dotted qualifier.

DisposeRequest request = new(); — a new rule keys on an identifier declared with a lifecycle type, wherever that declaration is. That also covers a message arriving as a parameter or a field, which had the same hole and which the comment did not name.

hub.Post<DisposeRequest>(new() { … }) — the message type read off the call.

…and the other half of that: Observe's type argument is the RESPONSE (Observe<CreateNodeResponse>(new CreateNodeRequest(…))), so it is deliberately not read as the message. There is a planted case pinning that too, because matching it would classify on the wrong side of the exchange.

Measured before applying it: over src/ the widened matcher adds zero sites — no such spelling exists in the tree today — and still finds exactly the one allowed violation. So this is pure hardening with no inventory change.

Control, because a planted case that cannot fail proves nothing: reverting the qualifier and declared-local rules reds the spelling cases while NoNewProductionSiteIssuesLifecycleWorkFromTheRouter stays green — which is exactly your point, and is why the two live in separate tests.

A syntax-tree scan would be stricter still, and is the right answer if this matcher needs a fifth case; the reason it is not that today is that SourceScan is shared by ~30 guards over .cs, .razor and .csx, and the last of those never reaches a compiler at all.

github-actions · 2026-09-16T07:38:46Z