From d42ceafde6d86ed7f322723c76b07149cab9990e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Roland=20B=C3=BCrgi?= <6334612+rbuergi@users.noreply.github.com> Date: Wed, 16 Sep 2026 09:26:53 +0200 Subject: [PATCH 1/4] fix(mesh): issue the recycle teardown off the router, and derive the guard's denominator MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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(...) in MessageHub's CONSTRUCTOR (DisposeRequest, ShutdownRequest, PingRequest, InitializeHubRequest) * node lifecycle <- every .WithHandler(...) 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) --- .../Architecture/RouterTrafficDetection.md | 96 +++- ...onger-competes-with-the-portals-routing.md | 42 ++ .../HubRecycleExtensions.cs | 12 +- .../MeshOperations.cs | 31 +- ...RouterAsNodeOperationOriginRatchetGuard.cs | 476 +++++++++++++++--- test/RouterNodeOperationOriginSites.allow | 23 +- 6 files changed, 576 insertions(+), 104 deletions(-) create mode 100644 src/MeshWeaver.Documentation/Data/WhatsNew/2026-09-16-recycling-a-node-no-longer-competes-with-the-portals-routing.md diff --git a/src/MeshWeaver.Documentation/Data/Architecture/RouterTrafficDetection.md b/src/MeshWeaver.Documentation/Data/Architecture/RouterTrafficDetection.md index 79f243bce3..1f7aeda8b0 100644 --- a/src/MeshWeaver.Documentation/Data/Architecture/RouterTrafficDetection.md +++ b/src/MeshWeaver.Documentation/Data/Architecture/RouterTrafficDetection.md @@ -2,7 +2,7 @@ nodeType: Markdown name: Router Traffic Detection category: Architecture -description: The ROUTER_TRAFFIC detector has two sites — the receiving hub, which names the two addresses, and the origin, which names the call site. Why the receiver-side line alone could not close an issue in four re-filings and 41,087 lines, what each site can and cannot see, and the seam a violating caller hops onto. +description: The ROUTER_TRAFFIC detector has two sites — the receiving hub, which names the two addresses, and the origin, which names the call site. Why the receiver-side line alone could not close an issue in four re-filings and 41,087 lines, what each site can and cannot see, the seam a violating caller hops onto, and why the ratchet that holds that seam derives its denominator from the framework's own handler registrations instead of listing message types. icon: /static/NodeTypeIcons/box.svg --- @@ -118,30 +118,97 @@ a **ratchet that may only shrink**, one per tree: | `src/` | `RouterAsNodeOperationOriginRatchetGuard` | `test/RouterNodeOperationOriginSites.allow` | 1 | | `test/` | `RouterAsTestRequestOriginRatchetGuard` | `test/RouterRequestOriginSites.allow` | 3 | -The `src/` guard matches an `.Observe(…)`/`.Post(…)` whose first argument is a node-lifecycle -request — `CreateNodeRequest`, `CreateNodesRequest`, `CreateOrUpdateNodeRequest`, -`DeleteNodeRequest`, `MoveNodeRequest`, `CopyNodeRequest` — built inline **or** hoisted into a local -first, and reads the receiver as an expression rather than as preceding text. Both tolerances were -measured: over `src/` the shape occurs at 23 sites, of which a construction-anchored scan misses the -five that are `MeshService`'s own verbs, and a literal `NodeOperationIssuingHub()` receiver test +The `src/` guard matches an `.Observe(…)`/`.Post(…)` whose first argument is a **lifecycle message**, +built inline **or** hoisted into a local first, and reads the receiver as an expression rather than +as preceding text. Both tolerances were measured: a construction-anchored scan misses the five sites +that are `MeshService`'s own verbs, and a literal `NodeOperationIssuingHub()` receiver test misclassifies the six that hoist the seam into a local or a cached property. +#### The denominator is DERIVED, and that is the point (#4463) + +> 🚨 **A list of message types is the same trap one level along.** The guard's first version carried +> a literal list of six node-CRUD names. `DisposeRequest` — a teardown, the most router-hostile +> lifecycle message there is — was not on it, so when `MeshOperations.RecycleCore` posted one +> straight off the DI-injected hub, **only the runtime ORIGIN detector saw it** +> ([#4463](https://github.com/Systemorph/MeshWeaver/issues/4463), measured on `memex` +> 2026-09-16 01:10:13Z). Adding one more name would have bought exactly one more message. + +So the guard now **reads its denominator out of production** instead of restating it. 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(…)` in **`MessageHub`'s constructor** — what a hub answers *because it is a hub* | `DisposeRequest`, `ShutdownRequest`, `PingRequest`, `InitializeHubRequest` | +| node lifecycle | every `.WithHandler(…)` in **`MeshExtensions.WithNodeOperationHandlers`** — what the off-router node-CRUD execution hub answers | `CreateNodeRequest`, `CreateNodesRequest`, `CreateOrUpdateNodeRequest`, `DeleteNodeRequest`, `ValidateDeleteRequest`, `MoveNodeRequest`, `CopyNodeRequest` | + +This is a *rule* rather than a *list* because **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 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**. That half is a separate test, and it fails on its +own: when the derivation was deliberately broken in rehearsal the ratchet went *green* over 25 sites +with `DisposeRequest` unguarded, and only the derivation test went red. + +**One structural exclusion: a self-directed hub-lifecycle post** — no target, or +`o.WithTarget(thatHub.Address)` — is out of the denominator, and not because the detector is quiet +about it. It is 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. `NodeTypeRebindWatcher`, the stale-build convergence, the overlay +self-heal and `LayoutAreaHost`'s own `InitializeHubRequest` are the four sites this covers. The NODE +family is deliberately *not* excluded this way: a target-less `CreateNodeRequest` on the router is +the literal prod 2026-06-11 wedge, and the seam does fix it by moving execution onto the +node-operation hub's block. + +Over `src/` this yields **29 lifecycle post sites in the denominator**, 4 self-directed ones +excluded, and one allowed violation (the `#981` entry below). + **Adopting the seam is never a behaviour change**, which is what lets this be a rule rather than a judgement call: `NodeOperationIssuingHub` returns the hub unchanged unless its address type is the mesh type. A site already off the router is byte-for-byte unaffected; a site that is not is corrected. -> 🚨 **What the ratchets still do not see.** They key on node-CRUD request TYPES, so the read seam -> (`ReadIssuingHub`, which has no request type of its own) is recognised where it is used but its -> absence is not reported — and the other message families in #1140's evidence (`ClickedEvent`, -> `UnsubscribeRequest`, `PatchDataChangeRequest`) have no single seam to hop onto at all. For those +> 🚨 **What the ratchets still do not see, and why #1140 is NOT closed by this.** They key on +> messages the framework registers a LIFECYCLE handler for, so the read seam (`ReadIssuingHub`, +> which has no request type of its own) is recognised where it is used but its absence is not +> reported — and the other message families in +> [#1140](https://github.com/Systemorph/MeshWeaver/issues/1140)'s evidence have no single seam to +> hop onto at all. Measured 2026-09-16, one of them is still live in `src/`: +> `JsonSynchronizationStream` posts `new UnsubscribeRequest(reduced.StreamId)` on its own `hub`, +> and hopping *that* is not a no-op — it would change which hub the unsubscribe originates from, +> which is a correlation question, not a routing one. +> +> #1140 is therefore the same defect **family** as #4463 and not the same **defect**. Its 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, fixed by keying the rule on the delivery's +> own ends. What is left of it is this residue plus whatever the origin line names next. For those > the origin line remains the instrument, and it is what makes a new one a five-minute question > instead of a month-long one. The one seeded `src/` entry is not debt: it is the #981 self-targeted inner create inside the `CreateOrUpdateNodeRequest` **handler**, posted on and handled by the hub whose turn loop already owns the upsert. A self-post is deliberately reported by neither detector site (above), so hopping -it would move the inner create off the hub that is mid-upsert and buy nothing. +it would move the inner create off the hub that is mid-upsert and buy nothing. It stays an explicit +allowance rather than joining the self-directed exclusion, because that exclusion is for HUB +lifecycle only — a target-less node CRUD post on the router is the wedge, not a benign self-post. + +### The two callers this rule was written from + +`MeshOperations.RecycleCore` (the Recycle tool / Compile button) and +`HubRecycleExtensions.RecycleNode` (the framework's one recycle surface) both post their +`DisposeRequest` through `hub.NodeOperationIssuingHub()`. The second had no production sighting; it +is hopped because 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*. The seam is +the identity function for every documented caller, so adopting it costs them nothing and removes the +undocumented one. ## Reading a report @@ -160,6 +227,11 @@ another process. - `RouterTrafficOnNodeCreateFromTheRootHubTest` — the create seam (`IMeshService.CreateNode` issued from the root hub keeps the router off both ends) plus the origin site's call-site attribution, each with its own positive control in the same fixture. +- `RouterAsNodeOperationOriginRatchetGuard` — the `src/` ratchet, in two tests that fail + independently: one asserts no new router-issued lifecycle site, the other asserts that the + denominator was actually DERIVED (both families non-empty, every derived name resolving to a real + type, `HeartBeatEvent` dropped by `RouterTrafficRule` rather than by a literal). The second exists + because a broken derivation makes the first go green over an unguarded tree. ## See also diff --git a/src/MeshWeaver.Documentation/Data/WhatsNew/2026-09-16-recycling-a-node-no-longer-competes-with-the-portals-routing.md b/src/MeshWeaver.Documentation/Data/WhatsNew/2026-09-16-recycling-a-node-no-longer-competes-with-the-portals-routing.md new file mode 100644 index 0000000000..4f5780ad9d --- /dev/null +++ b/src/MeshWeaver.Documentation/Data/WhatsNew/2026-09-16-recycling-a-node-no-longer-competes-with-the-portals-routing.md @@ -0,0 +1,42 @@ +--- +Name: Recycling a node no longer competes with the portal's routing +Category: Fix +Description: The Recycle tool and the Compile button used to tear a node's hub down from the mesh's router itself, so a burst of recycles queued behind — and ahead of — everyone else's page loads. Both recycle surfaces now issue the teardown off the router, and the guard that is supposed to catch this class was widened so it can no longer miss a message type. +Icon: ArrowSync +Order: -20260916 +--- + +# Recycling a node no longer competes with the portal's routing + +The mesh's root hub is its **router**: every message between two hubs passes through it, one at a +time. Work executed there does not merely run slowly — it takes turns away from routing, and a burst +of it starves the ordinary `SubscribeRequest` traffic that makes pages appear. That is what wedged a +production portal on 2026-06-11. + +`Recycle` — the MCP tool, and the **Compile** button that shares its code path — posted its teardown +straight from the router. So did `RecycleNode`, the framework's one recycle surface. Neither was +slow on its own; both put node-hub lifecycle on the one action block that must never queue. Both now +hand the teardown to the mesh's dedicated off-router hub first. For every caller that already held a +session, portal or per-node hub — which is nearly all of them — nothing changes at all: the hop is +the identity function for any hub that is not the router. + +## The half that mattered more + +A ratchet already existed to stop exactly this, and it did not see it. It matched a **literal list of +six node-CRUD request names**, and a teardown is not one of them — so the most router-hostile message +in the mesh was invisible to it, and the only thing that noticed was a runtime detector firing in +production ([#4463](https://github.com/Systemorph/MeshWeaver/issues/4463)). Adding "teardown" to the +list would have bought exactly one more message. + +The guard now **derives** what counts as lifecycle from the framework's own handler registrations — +the ones a hub makes for itself at construction, and the ones the node-operation hub makes — and +filters them through the same predicate the runtime detector evaluates. You cannot add a lifecycle +message without registering a handler for it, so a new one joins the guard automatically; a name in +a test can be forgotten, a handler cannot. On its first run the derivation picked up a verb the +hand-written list had already missed. + +What this does not change: the older, broader +[#1140](https://github.com/Systemorph/MeshWeaver/issues/1140) family stays open. Its remaining strand +is a stream unsubscribe, which has no equivalent seam and whose origin hub is a correlation question +rather than a routing one — see +[Router Traffic Detection](/Doc/Architecture/RouterTrafficDetection). diff --git a/src/MeshWeaver.Mesh.Contract/HubRecycleExtensions.cs b/src/MeshWeaver.Mesh.Contract/HubRecycleExtensions.cs index c2944e4ab2..edcf0cb455 100644 --- a/src/MeshWeaver.Mesh.Contract/HubRecycleExtensions.cs +++ b/src/MeshWeaver.Mesh.Contract/HubRecycleExtensions.cs @@ -92,7 +92,17 @@ public static class HubRecycleExtensions => hub.WhenNoInstallHoldsRoot(path) .SelectMany(_ => Observable.Defer(() => { - hub.Post( + // 🚨 OFF-ROUTER ISSUING (#4463). Every DOCUMENTED caller already holds a surviving + // non-router hub — that is this class's own rule — but this is a PUBLIC extension + // on `IMessageHub`, so nothing stops a mesh-singleton service from calling it with + // the DI-injected hub, which in the mesh's root container IS the ROUTER. The seam + // is the identity function for every hub whose address type is not the mesh type, + // so the documented callers are byte-for-byte unaffected and the undocumented one + // cannot make the router an end of a teardown delivery. The `hub.Address` in the + // fallback reason below is deliberately the CALLER's address, not the issuing + // hub's: it answers "who asked", which a reader of the target's [QUIESCE-START] + // needs, and `portal/nodeops-{meshId}` would answer nothing. + hub.NodeOperationIssuingHub().Post( new DisposeRequest { Reason = reason diff --git a/src/MeshWeaver.Mesh.Operations/MeshOperations.cs b/src/MeshWeaver.Mesh.Operations/MeshOperations.cs index 9082dac90f..2674c1430b 100644 --- a/src/MeshWeaver.Mesh.Operations/MeshOperations.cs +++ b/src/MeshWeaver.Mesh.Operations/MeshOperations.cs @@ -4088,15 +4088,28 @@ private IObservable RecycleCore(string resolvedPath) // call site as an ungated disposer for the same reason: it is the one teardown // that comes from OUTSIDE the framework's own lifecycle, so it is the one whose // attribution a log cannot reconstruct. - hub.Post( - new DisposeRequest - { - Reason = $"MeshOperations.Recycle: an operator asked for '{resolvedPath}' " - + "to be recycled (the Recycle tool / Compile button), which " - + "stamps a release request and then tears the hub down so the " - + "next access reactivates it", - }, - o => o.WithTarget(new Address(resolvedPath))); + // 🚨 OFF-ROUTER ISSUING (#4463), exactly as PackageInstaller.SettleRetypedRoot and + // the MoveNodeRequest above already do. `hub` here is whatever hub constructed this + // facade, and for the AI/agent surface that is the DI-injected IMessageHub — which + // in the mesh's ROOT container IS the ROUTER. Posting there stamps the teardown + // `Sender = mesh/{id}` and makes the router an END of a work delivery: measured in + // production 2026-09-16 01:10:13Z as `ROUTER_TRAFFIC ORIGIN: DisposeRequest was + // POSTED with the mesh hub as sender … target: Hosting/Triage/ci-failure/…`, with + // this exact frame on top. Node-hub lifecycle then competes with routing on the + // router's single-threaded action block, which is the 2026-06-11 portal wedge's + // shape. NodeOperationIssuingHub 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. + hub.NodeOperationIssuingHub() + .Post( + new DisposeRequest + { + Reason = $"MeshOperations.Recycle: an operator asked for '{resolvedPath}' " + + "to be recycled (the Recycle tool / Compile button), which " + + "stamps a release request and then tears the hub down so the " + + "next access reactivates it", + }, + o => o.WithTarget(new Address(resolvedPath))); return JsonSerializer.Serialize( new { diff --git a/test/MeshWeaver.Documentation.Test/RouterAsNodeOperationOriginRatchetGuard.cs b/test/MeshWeaver.Documentation.Test/RouterAsNodeOperationOriginRatchetGuard.cs index 5684fb627a..62e21e7817 100644 --- a/test/MeshWeaver.Documentation.Test/RouterAsNodeOperationOriginRatchetGuard.cs +++ b/test/MeshWeaver.Documentation.Test/RouterAsNodeOperationOriginRatchetGuard.cs @@ -1,16 +1,21 @@ using System; using System.Collections.Generic; +using System.Collections.Immutable; using System.IO; using System.Linq; +using System.Reflection; +using System.Runtime.CompilerServices; using System.Text.RegularExpressions; +using MeshWeaver.Mesh; +using MeshWeaver.Messaging; using Xunit; namespace MeshWeaver.Documentation.Test; /// -/// Governance ratchet for the router-as-node-CRUD-origin defect class (#1140): PRODUCTION code may -/// not issue a node-lifecycle request from a hub that can be the root mesh hub without hopping onto -/// MeshExtensions.NodeOperationIssuingHub(). +/// Governance ratchet for the router-as-lifecycle-origin defect class (#1140, #4463): PRODUCTION +/// code may not issue a node- or hub-LIFECYCLE message from a hub that can be the root mesh hub +/// without hopping onto MeshExtensions.NodeOperationIssuingHub(). /// /// Why this guard exists at all. The same finding was filed four times — #1113, #1121, /// #1136, #1140 — and ran to 41,087 [Error] lines on memex between 2026-08-10 and @@ -22,6 +27,14 @@ namespace MeshWeaver.Documentation.Test; /// posts node CRUD from its injected hub is a new #1140", i.e. that the class recurs by /// construction. This is the missing half. /// +/// 🚨 And the first version of it had the trap one level along. It carried a LITERAL +/// list of six node-CRUD request names, so DisposeRequest — a teardown, the most +/// router-hostile lifecycle message there is — was invisible to it. #4463 is the consequence: +/// MeshOperations.RecycleCore posted one straight off the DI-injected hub, only the runtime +/// ORIGIN detector saw it, and fixing that one line would have left every other lifecycle message +/// equally unguarded. Enumerating more names would buy exactly one more message. So the denominator +/// is now DERIVED FROM PRODUCTION instead of restated here — see below. +/// /// Why adopting the seam is never a behaviour change. /// NodeOperationIssuingHub returns the hub UNCHANGED unless its address type is the mesh /// type — it is the identity function for every portal, session, layout, import and per-node hub. @@ -30,13 +43,51 @@ namespace MeshWeaver.Documentation.Test; /// is why this ratchet can demand the seam without asking each author to prove which hub reaches /// their code. /// +/// THE DENOMINATOR, and why it is the right one. A message is lifecycle when the +/// FRAMEWORK ITSELF registers a lifecycle handler for it, and there are exactly two such +/// registrations. Both are read out of src/ at scan time; neither is spelled here. +/// +/// Hub lifecycle — every Register<T>(…) in MessageHub's +/// CONSTRUCTOR: the messages a hub answers because it is a hub (DisposeRequest, +/// ShutdownRequest, PingRequest, InitializeHubRequest). +/// Node lifecycle — every .WithHandler<T>(…) in +/// MeshExtensions.WithNodeOperationHandlers: the messages the mesh's off-router node-CRUD +/// EXECUTION hub answers, which is what makes ONE seam the answer for all of them. +/// +/// This is a rule rather than a list because 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 added to a test, can be forgotten; a handler cannot, +/// because without it the message does nothing. The derivation caught a verb the hand-written list +/// had already missed (ValidateDeleteRequest) on its first run. +/// +/// Sourced exclusions, also not restated. The derived verbs are filtered through +/// itself — the same pure predicate the two runtime detector sites +/// evaluate — so a message the production rule declares to be routing's OWN duty rather than work +/// (today: HeartBeatEvent, which WithNodeOperationHandlers also registers) 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 this guard +/// instead of silently shrinking what it measures. +/// /// What is matched. A .Observe(…) / .Post(…) call whose FIRST argument is -/// a node-lifecycle request — either constructed inline (new CreateNodeRequest(node)) or -/// hoisted into a local earlier in the file (var request = new DeleteNodeRequest(path); … +/// a lifecycle message — either constructed inline (new CreateNodeRequest(node)) or hoisted +/// into a local earlier in the file (var request = new DeleteNodeRequest(path); … /// IssuingHub.Observe(request, …), which is MeshService's own shape and which a /// constructor-anchored scan would have classified on the wrong statement). The receiver is then /// read as a primary expression and compared against the seam. /// +/// 🚨 One structural exclusion: a SELF-DIRECTED hub-lifecycle post. A hub telling +/// ITSELF to dispose, initialise or shut down — no target, or o.WithTarget(thatHub.Address) — +/// is out of the denominator, and NOT because the detector is quiet about it. It is 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. This is the shape +/// NodeTypeRebindWatcher, the stale-build convergence, the overlay self-heal and +/// LayoutAreaHost's own InitializeHubRequest all use. The NODE family is deliberately +/// NOT excluded this way: a target-less CreateNodeRequest on the router is the literal prod +/// 2026-06-11 wedge (CreateNodeRequest@mesh/<self> stale >60s), the seam DOES fix it +/// (the request then executes on the node-operation hub's block, not the router's), and the one +/// seeded allowance is exactly that shape. +/// /// Deliberately NOT matched, and why each is a separate job. /// /// The READ seam (ReadIssuingHub). A one-shot node read has no request type of its @@ -47,19 +98,19 @@ namespace MeshWeaver.Documentation.Test; /// A request handed to a HELPER rather than posted (EnsurePartitionBootstrap(hub, n, /// new CreateNodeRequest(n))). The post is in the helper, where this scan sees it with the /// helper's own receiver; counting the construction too would double-count one site. -/// Message types other than node CRUD. #1140's evidence also names ClickedEvent, -/// UnsubscribeRequest and PatchDataChangeRequest; those have no single seam to -/// hop onto, and a marker that fired on the router's own routing duties is the one that gets -/// muted — the argument RouterTrafficRule itself makes for keying on the delivery's -/// ends rather than the handling hub. +/// Messages the framework registers no lifecycle handler for. #1140's evidence also names +/// ClickedEvent, UnsubscribeRequest and PatchDataChangeRequest; those have +/// no single seam to hop onto, and a marker that fired on the router's own routing duties is +/// the one that gets muted — the argument itself makes for +/// keying on the delivery's ends rather than the handling hub. /// /// /// The tolerances were measured, not guessed. Over src/ at the time of writing -/// this finds 23 node-CRUD post sites. A scan that required the request to be constructed INSIDE -/// the call's argument list finds 18 of them — the five it misses are every verb of +/// this finds 29 lifecycle post sites, 4 of them self-directed hub lifecycle. A scan that required +/// the request to be constructed INSIDE the call's argument list misses every verb of /// MeshService, the reference implementation. A seam test that accepted only the literal -/// NodeOperationIssuingHub() text misclassifies six more, because the idiomatic spelling -/// hoists it once per operation (var issuingHub = hub.NodeOperationIssuingHub();, +/// NodeOperationIssuingHub() text misclassifies the sites that hoist it once per operation +/// (var issuingHub = hub.NodeOperationIssuingHub();, /// private IMessageHub IssuingHub => _issuingHub ??= hub.NodeOperationIssuingHub();). /// Hence the alias pass. Its known approximation: an identifier is treated as a seam alias for the /// WHOLE file once any assignment in that file derives it from a seam call, so a file that also @@ -88,17 +139,15 @@ public class RouterAsNodeOperationOriginRatchetGuard(ITestOutputHelper output) private static readonly string[] ScannedRoots = ["src"]; /// - /// The node-lifecycle verbs. These are the ones with a shared off-router execution hub behind - /// them (WithNodeOperationExecution), which is what makes one seam the answer for all of - /// them. + /// Where the HUB-lifecycle family is declared: the Register<T> calls in + /// MessageHub's constructor. Read, never restated — see the class remarks. /// - private static readonly string[] RequestTypes = - [ - "CreateNodeRequest", "CreateNodesRequest", "CreateOrUpdateNodeRequest", - "DeleteNodeRequest", "MoveNodeRequest", "CopyNodeRequest", - ]; + private const string HubLifecycleSourceFile = "src/MeshWeaver.Messaging.Hub/MessageHub.cs"; + + /// Where the NODE-lifecycle family is declared. + private const string NodeLifecycleSourceFile = "src/MeshWeaver.Mesh.Contract/MeshExtensions.cs"; - private static readonly string RequestAlternation = string.Join("|", RequestTypes); + private const string AllowFileName = "RouterNodeOperationOriginSites.allow"; /// /// The request/response and fire-and-forget entry points, tolerant of an explicit response type @@ -108,14 +157,6 @@ public class RouterAsNodeOperationOriginRatchetGuard(ITestOutputHelper output) private static readonly Regex CallMarker = new(@"\.\s*(?:Observe|Post)\s*(?:<[^<>()]*>\s*)?\(", RegexOptions.Compiled); - private static readonly Regex NewRequest = - new(@"\bnew\s+(?:" + RequestAlternation + @")\b", RegexOptions.Compiled); - - /// A local or field bound to a freshly constructed node-lifecycle request. - private static readonly Regex RequestLocal = - new(@"\b([A-Za-z_][A-Za-z0-9_]*)\s*=\s*new\s+(?:" + RequestAlternation + @")\b", - RegexOptions.Compiled); - /// A call to either seam. Both count: a site that hops for reads is off the router. private static readonly Regex SeamCall = new(@"(?:NodeOperationIssuingHub|ReadIssuingHub)\s*\(\s*\)", RegexOptions.Compiled); @@ -135,18 +176,77 @@ public class RouterAsNodeOperationOriginRatchetGuard(ITestOutputHelper output) private static readonly Regex BareIdentifier = new(@"^[A-Za-z_][A-Za-z0-9_]*$", RegexOptions.Compiled); - private const string AllowFileName = "RouterNodeOperationOriginSites.allow"; + /// The o.WithTarget(expr) inside a post's options lambda. + private static readonly Regex TargetOption = + new(@"\bWithTarget\s*\(", RegexOptions.Compiled); + + /// The hub's own constructor-time lifecycle registrations. + private static readonly Regex HubRegistration = + new(@"\bRegister<([A-Za-z_][A-Za-z0-9_]*)>\s*\(", RegexOptions.Compiled); + + /// The node-operation execution hub's handler registrations. + private static readonly Regex HandlerRegistration = + new(@"\.\s*WithHandler<([A-Za-z_][A-Za-z0-9_]*)>\s*\(", RegexOptions.Compiled); + + private static readonly Regex HubConstructorSignature = + new(@"\bpublic\s+MessageHub\s*\(", RegexOptions.Compiled); + + private static readonly Regex NodeOperationRegistrarSignature = + new(@"\bWithNodeOperationHandlers\s*\(\s*this\s+MessageHubConfiguration", RegexOptions.Compiled); + + /// + /// The two lifecycle families, as derived from production, plus the matchers built over them. + /// A hub-lifecycle post is excluded when it is self-directed (the seam's remedy would misdeliver + /// it); a node-lifecycle post never is. + /// + private sealed class LifecycleVerbs + { + public LifecycleVerbs(ImmutableHashSet hub, ImmutableHashSet node) + { + Hub = hub; + Node = node; + All = hub.Union(node); + var alternation = string.Join("|", All.Order(StringComparer.Ordinal)); + NewRequest = new Regex(@"\bnew\s+(" + alternation + @")\b"); + RequestLocal = new Regex( + @"\b([A-Za-z_][A-Za-z0-9_]*)\s*=\s*new\s+(" + alternation + @")\b"); + } + + public ImmutableHashSet Hub { get; } + + public ImmutableHashSet Node { get; } + + public ImmutableHashSet All { get; } - /// One matched call: where it is, and whether its receiver is a seam. - private readonly record struct Site(string File, int Line, bool OffRouter, string Receiver); + /// A lifecycle message constructed inline in a call's argument list. + public Regex NewRequest { get; } + + /// A local or field bound to a freshly constructed lifecycle message. + public Regex RequestLocal { get; } + } + + /// One matched call: where it is, which verb, and how it is addressed. + private readonly record struct Site( + string File, int Line, bool OffRouter, string Receiver, string Verb, bool SelfDirected); [Fact] - public void NoNewProductionSiteIssuesNodeCrudFromTheRouter() + public void NoNewProductionSiteIssuesLifecycleWorkFromTheRouter() { var root = SourceScan.FindRepoRoot(); var allowed = SourceScan.ReadAllowFile(Path.Combine(root, "test", AllowFileName), AllowFileName); + var verbs = DeriveVerbs(root); - var violations = Scan(root) + output.WriteLine( + $"Lifecycle verbs derived from src/: hub [{string.Join(", ", verbs.Hub.Order(StringComparer.Ordinal))}]" + + $" · node [{string.Join(", ", verbs.Node.Order(StringComparer.Ordinal))}]"); + + var sites = Scan(root, verbs); + var counted = sites.Where(s => InDenominator(s, verbs)).ToList(); + output.WriteLine( + $"{counted.Count} lifecycle post sites in the denominator ({sites.Count - counted.Count} " + + "self-directed hub-lifecycle posts excluded)."); + + var violations = counted .Where(s => !s.OffRouter) .GroupBy(s => s.File, StringComparer.Ordinal) .ToDictionary(g => g.Key, g => g.Count(), StringComparer.Ordinal); @@ -157,15 +257,15 @@ public void NoNewProductionSiteIssuesNodeCrudFromTheRouter() { if (!allowed.TryGetValue(file, out var budget)) failures.Add( - $" NEW SITE {file} ({count}) — issue the request from " + $" NEW SITE {file} ({count}) — issue the message from " + "hub.NodeOperationIssuingHub() (or hub.ReadIssuingHub() for a one-shot read). " + "It returns the hub unchanged unless that hub is the ROUTER, so this is a " + "no-op wherever the router is not reached. Do NOT add a line to " + AllowFileName + "."); else if (count > budget) failures.Add( - $" MORE {file} ({count} > {budget} allowed) — a router-issued node " - + "operation was ADDED to a file that already carries the shape."); + $" MORE {file} ({count} > {budget} allowed) — a router-issued lifecycle " + + "message was ADDED to a file that already carries the shape."); } var total = allowed.Values.Sum(); @@ -185,36 +285,75 @@ public void NoNewProductionSiteIssuesNodeCrudFromTheRouter() } Assert.True(failures.Count == 0, - "Production code that posts node CRUD from the root mesh hub puts the ROUTER on both " - + "ends of the delivery — and, when the request is target-less, EXECUTES the write on " - + "the routing action block. That is #1140, which was filed four times and ran to " - + "41,087 error lines before anyone could name a call site. Hop onto " - + "MeshExtensions.NodeOperationIssuingHub().\n" - + string.Join("\n", failures)); + "Production code that issues node- or hub-LIFECYCLE work from the root mesh hub puts the " + + "ROUTER on an end of the delivery — and, when the request is target-less, EXECUTES it " + + "on the routing action block. That is #1140, which was filed four times and ran to " + + "41,087 error lines before anyone could name a call site, and #4463, which slipped " + + "past the first version of this guard because DisposeRequest was not on a literal " + + "list. Hop onto MeshExtensions.NodeOperationIssuingHub().\n" + + string.Join("\n", failures) + + "\nOffending sites:\n" + + string.Join("\n", counted.Where(s => !s.OffRouter) + .Select(s => $" {s.File}:{s.Line} {s.Verb} posted on '{s.Receiver}'"))); } /// - /// Non-vacuity, in two halves that fail independently. + /// Non-vacuity, in three halves that fail independently. + /// + /// The first proves the DERIVATION reached both production registrations, that every name + /// it produced is a real type, and that the family it yields is the lifecycle family rather than + /// whatever a broken regex happened to capture. A guard whose denominator silently emptied would + /// report a clean tree while enforcing nothing — the exact shape of the defect it exists to + /// catch. /// - /// The first half proves the CLASSIFIER can reach both verdicts, by running the real - /// scanning functions over a planted snippet that contains one of each. Without it a seam test - /// that accidentally matched everything would report an empty violation set and the ratchet - /// above would pass having blessed the tree. + /// The second proves the CLASSIFIER can reach every verdict, by running the real scanning + /// functions over a planted snippet that contains one of each. Without it a seam test that + /// accidentally matched everything would report an empty violation set and the ratchet above + /// would pass having blessed the tree. /// - /// The second half proves the scan reached the PRODUCTION tree and recognised the shape - /// there. The two fail independently and only together mean anything — a planted-tree self-test + /// The third proves the scan reached the PRODUCTION tree and recognised the shape there. + /// The three fail independently and only together mean anything — a planted-tree self-test /// cannot prove the scanner found src/ (SourceScan's own remarks, #2844), and a /// production count cannot prove the classifier has a failing branch. /// [Fact] - public void TheScannerReachesBothVerdictsAndSeesTheProductionTree() + public void TheDerivationIsSourcedTheScannerReachesEveryVerdictAndSeesTheProductionTree() { + var root = SourceScan.FindRepoRoot(); + var verbs = DeriveVerbs(root); + + // --- 1. the derivation --------------------------------------------------------------- + Assert.True(verbs.Hub.Count > 0, + $"No Register(…) call was found in MessageHub's constructor ({HubLifecycleSourceFile}). " + + "The HUB-lifecycle half of this guard's denominator is EMPTY, so DisposeRequest — the " + + "message #4463 was about — is unguarded again while this test reports green. The " + + "constructor or the registration spelling moved; follow it, never relax the scan."); + Assert.True(verbs.Node.Count > 0, + $"No .WithHandler(…) call was found in WithNodeOperationHandlers ({NodeLifecycleSourceFile}). " + + "The NODE-lifecycle half of the denominator is EMPTY — every CreateNodeRequest in the " + + "tree is unguarded while this test reports green."); + Assert.Contains(nameof(DisposeRequest), verbs.Hub); + Assert.Contains(nameof(PingRequest), verbs.Hub); + Assert.Contains(nameof(CreateNodeRequest), verbs.Node); + Assert.Contains(nameof(DeleteNodeRequest), verbs.Node); + // Sourced from the production predicate, not restated: HeartBeatEvent IS registered by + // WithNodeOperationHandlers and RouterTrafficRule declares it routing's own liveness, so the + // filter must have dropped it. If RouterTrafficRule stops excluding it, this flips and the + // reader is told where the disagreement is rather than silently inheriting it. + Assert.DoesNotContain(nameof(HeartBeatEvent), verbs.All); + Assert.Null(RouterTrafficRule.RoleOf( + "portal", AddressExtensions.MeshType, new HeartBeatEvent(), isResponse: false)); + + // --- 2. the classifier --------------------------------------------------------------- const string planted = """ class Planted { IObservable Violating(IMessageHub hub) => hub.Observe(new CreateNodeRequest(node), o => o.WithTarget(t)).Select(_ => 1); + void ViolatingTeardown(IMessageHub hub) => + hub.Post(new DisposeRequest { Reason = r }, o => o.WithTarget(new Address(p))); + IObservable Compliant(IMessageHub hub) => hub.NodeOperationIssuingHub() .Observe(new DeleteNodeRequest(path), o => o.WithTarget(t)).Select(_ => 1); @@ -226,30 +365,54 @@ IObservable ViaAlias(IMessageHub hub) return issuing.Observe(request, o => o.WithTarget(t)).Select(_ => 1); } + void SelfDirectedTeardown(IMessageHub instanceHub) => + instanceHub.Post(new DisposeRequest { Reason = r }, + o => o.WithTarget(instanceHub.Address)); + + void SelfDirectedInit(IMessageHub streamHub) => + streamHub.Post(new InitializeHubRequest()); + IObservable NotARequest(IMessageHub hub) => hub.Observe(somethingElse, o => o.WithTarget(t)).Select(_ => 1); } """; - var plantedSites = SitesIn("planted.cs", planted); - - Assert.Equal(3, plantedSites.Count); - Assert.Single(plantedSites.Where(s => !s.OffRouter)); - Assert.Equal(2, plantedSites.Count(s => s.OffRouter)); - Assert.Equal("hub", Assert.Single(plantedSites.Where(s => !s.OffRouter)).Receiver); + var plantedSites = SitesIn("planted.cs", planted, verbs); + + Assert.Equal(6, plantedSites.Count); + var plantedCounted = plantedSites.Where(s => InDenominator(s, verbs)).ToList(); + Assert.Equal(4, plantedCounted.Count); + Assert.Equal(2, plantedCounted.Count(s => !s.OffRouter)); + Assert.Equal(2, plantedCounted.Count(s => s.OffRouter)); + Assert.Equal( + new[] { nameof(CreateNodeRequest), nameof(DisposeRequest) }, + plantedCounted.Where(s => !s.OffRouter).Select(s => s.Verb) + .Order(StringComparer.Ordinal).ToArray()); + // Both self-directed shapes leave the denominator, and BOTH spellings must: the explicit + // o.WithTarget(thatHub.Address) and the target-less post. + Assert.Equal(2, plantedSites.Count(s => s.SelfDirected)); + // …but a self-directed NODE-lifecycle post stays IN — it is the prod 2026-06-11 wedge, and + // the seam does fix it by moving execution off the router's block. + var plantedNodeSelfPost = SitesIn( + "planted-self-crud.cs", + "class P { void M(IMessageHub hub) => hub.Observe(new CreateNodeRequest(n), " + + "o => o.WithTarget(hub.Address)); }", + verbs); + Assert.True(InDenominator(Assert.Single(plantedNodeSelfPost), verbs)); // Comment masking, both sides. The same line is a site when it is code and nothing when it // is prose — which is the difference between measuring the tree and measuring the remarks // that describe it. This repo's remarks quote these shapes verbatim. const string asCode = "var x = hub.Observe(new CreateNodeRequest(n), o => o.WithTarget(t));"; - Assert.Single(SitesIn("code.cs", asCode)); - Assert.Empty(SitesIn("prose.cs", "// " + asCode)); - Assert.Empty(SitesIn("literal.cs", "var s = \"" + asCode.Replace("\"", "") + "\";")); + Assert.Single(SitesIn("code.cs", asCode, verbs)); + Assert.Empty(SitesIn("prose.cs", "// " + asCode, verbs)); + Assert.Empty(SitesIn("literal.cs", "var s = \"" + asCode.Replace("\"", "") + "\";", verbs)); - var found = Scan(SourceScan.FindRepoRoot()); + // --- 3. the production tree ------------------------------------------------------------ + var found = Scan(root, verbs); Assert.True(found.Count > 0, - "The scanner found NO node-lifecycle post site anywhere under " + "The scanner found NO lifecycle post site anywhere under " + string.Join(", ", ScannedRoots) + ". src/ cannot be free of node CRUD, so this is a BROKEN SCAN reporting as a clean " + "tree — the failure mode SourceScan's remarks describe (#2844). Fix the scan; never " @@ -257,11 +420,129 @@ IObservable NotARequest(IMessageHub hub) => Assert.True(found.Any(s => s.OffRouter), "The scanner classified EVERY production site as router-issued. MeshService alone hops " + "on all five of its verbs, so the seam test is broken and every count is unreliable."); + Assert.True(found.Any(s => verbs.Hub.Contains(s.Verb)), + "The scanner found node-CRUD sites but NO hub-lifecycle site (DisposeRequest, " + + "PingRequest, …). src/ posts both — PackageInstaller.SettleRetypedRoot alone posts a " + + "DisposeRequest and a PingRequest through the seam — so the half of the denominator " + + "#4463 added is measuring nothing."); + } + + /// + /// Whether a matched site is in the denominator: a self-directed HUB-lifecycle post is not, + /// because the seam's remedy — issue it from a different hub — would misdeliver it. + /// + private static bool InDenominator(Site site, LifecycleVerbs verbs) => + !(verbs.Hub.Contains(site.Verb) && site.SelfDirected); + + /// + /// The lifecycle families, read out of the two production registrations. Every derived name + /// must resolve to a loadable type, and any the production + /// declares to be routing's own duty is dropped. + /// + private static LifecycleVerbs DeriveVerbs(string root) + { + var hub = RegisteredIn( + Path.Combine(root, HubLifecycleSourceFile), HubConstructorSignature, HubRegistration); + var node = RegisteredIn( + Path.Combine(root, NodeLifecycleSourceFile), + NodeOperationRegistrarSignature, HandlerRegistration); + return new LifecycleVerbs(WorkMessages(hub), WorkMessages(node)); + } + + /// + /// The generic type arguments of every inside the body that + /// follows , brace-matched. Reading the BODY rather than the file + /// is what keeps the generic Register<TMessage> DECLARATIONS out of the family. + /// + private static ImmutableHashSet RegisteredIn( + string file, Regex signature, Regex registration) + { + Assert.True(File.Exists(file), + $"{file} is the source this guard DERIVES its denominator from, and it is not there. " + + "The file moved; point this guard at it. Never fall back to a literal list — that is " + + "the defect #4463 was."); + + var code = SourceScan.MaskCommentsAndStrings(File.ReadAllText(file)); + var match = signature.Match(code); + Assert.True(match.Success, + $"Could not find '{signature}' in {file}. The declaration this guard reads its " + + "denominator from was renamed or restructured — follow it. A guard whose derivation " + + "silently returns nothing reports a clean tree while enforcing nothing."); + + return registration.Matches(BodyAfter(code, match.Index + match.Length)) + .Select(m => m.Groups[1].Value) + .ToImmutableHashSet(StringComparer.Ordinal); + } + + /// The brace-matched block starting at the first { at or after . + private static string BodyAfter(string code, int from) + { + var start = code.IndexOf('{', from); + if (start < 0) return string.Empty; + var depth = 0; + for (var i = start; i < code.Length; i++) + { + if (code[i] == '{') depth++; + else if (code[i] == '}' && --depth == 0) return code[start..(i + 1)]; + } + + return code[start..]; } - private static IReadOnlyList Scan(string root) => + /// + /// The subset of that the production + /// treats as WORK rather than as routing's own duty — asked of the rule itself, so an exclusion + /// added there is inherited instead of copied. Every name must resolve to a type: a rename that + /// silently dropped a verb would shrink the denominator without reddening anything. + /// + private static ImmutableHashSet WorkMessages(ImmutableHashSet names) + { + var assemblies = new[] + { + typeof(RouterTrafficRule).Assembly, // MeshWeaver.Messaging.Contract + typeof(MessageHub).Assembly, // MeshWeaver.Messaging.Hub + typeof(MeshExtensions).Assembly, // MeshWeaver.Mesh.Contract + }; + + return names + .Where(name => + { + var type = assemblies.Select(a => Resolve(a, name)).FirstOrDefault(t => t is not null); + Assert.True(type is not null, + $"The derivation produced '{name}', but no type of that name exists in " + + string.Join(", ", assemblies.Select(a => a.GetName().Name)) + + ". Either the registration scan is capturing something that is not a message " + + "type, or the type moved to an assembly this guard does not anchor on. Both " + + "shrink the denominator silently, which is why this is a failure and not a " + + "filter."); + // A non-response delivery from the router to a non-router address: the shape every + // site in this scan produces. `null` means the rule calls it routing's own duty. + return RouterTrafficRule.RoleOf( + "portal", + AddressExtensions.MeshType, + RuntimeHelpers.GetUninitializedObject(type!), + isResponse: false) is not null; + }) + .ToImmutableHashSet(StringComparer.Ordinal); + } + + /// The single type named in , + /// internal ones included, or null. + private static Type? Resolve(Assembly assembly, string name) + { + try + { + return assembly.GetTypes().FirstOrDefault(t => string.Equals(t.Name, name, StringComparison.Ordinal)); + } + catch (ReflectionTypeLoadException e) + { + return e.Types.FirstOrDefault(t => t is not null && string.Equals(t.Name, name, StringComparison.Ordinal)); + } + } + + private static IReadOnlyList Scan(string root, LifecycleVerbs verbs) => SourceScan.SourceFiles(root, ScannedRoots) - .SelectMany(f => SitesIn(SourceScan.Relative(root, f), ReadOrEmpty(f))) + .SelectMany(f => SitesIn(SourceScan.Relative(root, f), ReadOrEmpty(f), verbs)) .ToList(); private static string ReadOrEmpty(string path) @@ -273,20 +554,21 @@ private static string ReadOrEmpty(string path) /// /// Every .Observe/.Post call in whose first argument is a - /// node-lifecycle request, classified by whether its receiver is one of the off-router seams. - /// Comments and string literals are masked first, so remarks quoting the shape are not counted. + /// lifecycle message, classified by whether its receiver is one of the off-router seams and by + /// whether the post is self-directed. Comments and string literals are masked first, so remarks + /// quoting the shape are not counted. /// - private static IReadOnlyList SitesIn(string file, string text) + private static IReadOnlyList SitesIn(string file, string text, LifecycleVerbs verbs) { - if (!RequestTypes.Any(t => text.Contains(t, StringComparison.Ordinal))) + if (!verbs.All.Any(t => text.Contains(t, StringComparison.Ordinal))) return []; var code = SourceScan.MaskCommentsAndStrings(text); - var requestLocals = new Dictionary(StringComparer.Ordinal); - foreach (Match m in RequestLocal.Matches(code)) + var requestLocals = new Dictionary(StringComparer.Ordinal); + foreach (Match m in verbs.RequestLocal.Matches(code)) if (!requestLocals.ContainsKey(m.Groups[1].Value)) - requestLocals[m.Groups[1].Value] = m.Index; + requestLocals[m.Groups[1].Value] = (m.Index, m.Groups[2].Value); var seamAliases = SeamAlias.Matches(code) .Select(m => m.Groups[1].Value) @@ -298,24 +580,64 @@ private static IReadOnlyList SitesIn(string file, string text) var openParen = call.Index + call.Length - 1; var argument = SourceScan.FirstArgument(code, openParen).Trim(); - if (!NewRequest.IsMatch(argument)) + string verb; + var constructed = verbs.NewRequest.Match(argument); + if (constructed.Success) + { + verb = constructed.Groups[1].Value; + } + else { // The hoisted spelling: the request was built into a local a few lines up. var head = argument.Split('.')[0].Trim(); if (!BareIdentifier.IsMatch(head) - || !requestLocals.TryGetValue(head, out var boundAt) - || boundAt >= call.Index) + || !requestLocals.TryGetValue(head, out var bound) + || bound.At >= call.Index) continue; + verb = bound.Verb; } var receiver = Receiver(code, call.Index); var offRouter = SeamCall.IsMatch(receiver) || IsSeamAlias(receiver, seamAliases); - sites.Add(new Site(file, LineOf(code, call.Index), offRouter, receiver)); + sites.Add(new Site( + file, LineOf(code, call.Index), offRouter, receiver, verb, + IsSelfDirected(code, openParen, receiver))); } return sites; } + /// + /// Whether the post names no target at all, or names only the RECEIVER's own address — the two + /// spellings of "this hub is telling itself", where the issuing seam has nothing to move and + /// adopting it would change which hub the message reaches. + /// + private static bool IsSelfDirected(string code, int openParen, string receiver) + { + var arguments = ArgumentList(code, openParen); + var targets = TargetOption.Matches(arguments) + .Select(m => Collapse(SourceScan.FirstArgument(arguments, m.Index + m.Length - 1))) + .ToList(); + return targets.Count == 0 + || targets.All(t => string.Equals(t, Collapse(receiver) + ".Address", StringComparison.Ordinal)); + } + + /// The whole argument list of the call whose open paren is at . + private static string ArgumentList(string code, int openParen) + { + var depth = 0; + for (var i = openParen; i < code.Length; i++) + { + if (code[i] is '(' or '[' or '{') depth++; + else if (code[i] is ')' or ']' or '}' && --depth == 0) return code[(openParen + 1)..i]; + } + + return code[(openParen + 1)..]; + } + + private static string Collapse(string expression) => + string.Concat(expression.Where(c => !char.IsWhiteSpace(c))); + private static int LineOf(string code, int index) => code.AsSpan(0, index).Count('\n') + 1; diff --git a/test/RouterNodeOperationOriginSites.allow b/test/RouterNodeOperationOriginSites.allow index 8c90da5560..b7a7d9005f 100644 --- a/test/RouterNodeOperationOriginSites.allow +++ b/test/RouterNodeOperationOriginSites.allow @@ -1,23 +1,36 @@ -# RouterNodeOperationOriginSites.allow — the src-side router-as-node-CRUD-origin ratchet (#1140). +# RouterNodeOperationOriginSites.allow — the src-side router-as-lifecycle-origin ratchet (#1140, #4463). # -# Each line is a PRODUCTION file that still issues a node-lifecycle request without hopping onto -# MeshExtensions.NodeOperationIssuingHub(): +# Each line is a PRODUCTION file that still issues a node- or hub-LIFECYCLE message without hopping +# onto MeshExtensions.NodeOperationIssuingHub(): # # hub.Observe(new CreateNodeRequest(node), …) hub.Post(new MoveNodeRequest(…), …) +# hub.Post(new DisposeRequest { … }, o => o.WithTarget(new Address(path))) # # For a caller holding the DI-injected IMessageHub — which in the mesh's ROOT container IS the -# ROUTER — that request leaves stamped `Sender = mesh/{id}` and its response is addressed straight +# ROUTER — that message leaves stamped `Sender = mesh/{id}` and its response is addressed straight # back at `mesh/{id}`; if it is target-less it also EXECUTES on the router's single-threaded action # block, starving the routing it exists to do (prod 2026-06-11). NodeOperationIssuingHub is the # IDENTITY FUNCTION for every hub that is not the router, so adopting it is a no-op everywhere # except where it matters. # +# 🚨 THE DENOMINATOR IS DERIVED, NOT LISTED. The guard reads the lifecycle families out of +# production — MessageHub's constructor registrations (DisposeRequest, ShutdownRequest, PingRequest, +# InitializeHubRequest) and MeshExtensions.WithNodeOperationHandlers (the node-CRUD verbs) — and +# filters them through RouterTrafficRule itself. #4463 is why: the first version of the guard +# carried a literal list of six CRUD names, DisposeRequest was not on it, and MeshOperations.Recycle +# posted one straight off the router in production without reddening anything. Do not re-introduce a +# list; if a lifecycle message is missing from the denominator, the registration it should be in is +# the thing to look at. +# # THE ENTRY BELOW IS NOT DEBT. It is the #981 self-targeted inner create inside the # CreateOrUpdateNodeRequest HANDLER: posted on, and handled by, the hub whose turn loop already owns # the upsert, with `o.WithTarget(hub.Address)` making that explicit. Hopping it would move the inner # create off the hub that is mid-upsert, and a self-post is deliberately reported by NEITHER # ROUTER_TRAFFIC site (see Doc/Architecture/RouterTrafficDetection, "What neither site sees"). -# Do not "fix" it; do not add a second line to make a new site pass. +# It is NOT covered by the guard's self-directed exclusion, and deliberately so: that exclusion is +# for HUB lifecycle only, because a target-less node CRUD post on the router is the prod 2026-06-11 +# wedge and the seam does fix it. Do not "fix" this line; do not add a second line to make a new +# site pass. # # Format: relative/path.cscount ('#' starts a comment) From c06f86bb013bb54af1acbfb9a4a7a9b0cf0c3e89 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Roland=20B=C3=BCrgi?= <6334612+rbuergi@users.noreply.github.com> Date: Wed, 16 Sep 2026 09:30:08 +0200 Subject: [PATCH 2/4] test(guard): name the two assemblies the lifecycle derivation reads 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) --- .../MeshWeaver.Documentation.Test.csproj | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/test/MeshWeaver.Documentation.Test/MeshWeaver.Documentation.Test.csproj b/test/MeshWeaver.Documentation.Test/MeshWeaver.Documentation.Test.csproj index c76910fba1..aaeebebb7b 100644 --- a/test/MeshWeaver.Documentation.Test/MeshWeaver.Documentation.Test.csproj +++ b/test/MeshWeaver.Documentation.Test/MeshWeaver.Documentation.Test.csproj @@ -15,6 +15,14 @@ reason as above: a guard that restates a key instead of referencing it goes green through the rename it exists to catch. --> + + + From 1de06bcf95bd1612b67f4eed93c293d5aa9b06e3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Roland=20B=C3=BCrgi?= <6334612+rbuergi@users.noreply.github.com> Date: Wed, 16 Sep 2026 09:32:17 +0200 Subject: [PATCH 3/4] docs(guard): re-measure the two scanner tolerances against the widened 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) --- .../Data/Architecture/RouterTrafficDetection.md | 8 +++++--- .../RouterAsNodeOperationOriginRatchetGuard.cs | 9 +++++---- 2 files changed, 10 insertions(+), 7 deletions(-) diff --git a/src/MeshWeaver.Documentation/Data/Architecture/RouterTrafficDetection.md b/src/MeshWeaver.Documentation/Data/Architecture/RouterTrafficDetection.md index 1f7aeda8b0..0dc1fb0ce2 100644 --- a/src/MeshWeaver.Documentation/Data/Architecture/RouterTrafficDetection.md +++ b/src/MeshWeaver.Documentation/Data/Architecture/RouterTrafficDetection.md @@ -120,9 +120,11 @@ a **ratchet that may only shrink**, one per tree: The `src/` guard matches an `.Observe(…)`/`.Post(…)` whose first argument is a **lifecycle message**, built inline **or** hoisted into a local first, and reads the receiver as an expression rather than -as preceding text. Both tolerances were measured: a construction-anchored scan misses the five sites -that are `MeshService`'s own verbs, and a literal `NodeOperationIssuingHub()` receiver test -misclassifies the six that hoist the seam into a local or a cached property. +as preceding text. Both tolerances were measured over the tree, not guessed: of the 33 sites it +matches, a construction-anchored scan would miss **7** (every verb of `MeshService`, the reference +implementation, plus the copy helper's hoisted request), and a receiver test that accepted only the +literal `NodeOperationIssuingHub()` text would misclassify the **11** that hoist the seam into a +local or a cached property. #### The denominator is DERIVED, and that is the point (#4463) diff --git a/test/MeshWeaver.Documentation.Test/RouterAsNodeOperationOriginRatchetGuard.cs b/test/MeshWeaver.Documentation.Test/RouterAsNodeOperationOriginRatchetGuard.cs index 62e21e7817..1cc48b52c2 100644 --- a/test/MeshWeaver.Documentation.Test/RouterAsNodeOperationOriginRatchetGuard.cs +++ b/test/MeshWeaver.Documentation.Test/RouterAsNodeOperationOriginRatchetGuard.cs @@ -106,10 +106,11 @@ namespace MeshWeaver.Documentation.Test; /// /// /// The tolerances were measured, not guessed. Over src/ at the time of writing -/// this finds 29 lifecycle post sites, 4 of them self-directed hub lifecycle. A scan that required -/// the request to be constructed INSIDE the call's argument list misses every verb of -/// MeshService, the reference implementation. A seam test that accepted only the literal -/// NodeOperationIssuingHub() text misclassifies the sites that hoist it once per operation +/// this matches 33 lifecycle post sites, 4 of them self-directed hub lifecycle, leaving 29 in the +/// denominator. A scan that required the request to be constructed INSIDE the call's argument list +/// would miss 7 of the 33 — every verb of MeshService, the reference implementation, plus the +/// copy helper's hoisted request. A seam test that accepted only the literal +/// NodeOperationIssuingHub() text would misclassify the 11 that hoist it once per operation /// (var issuingHub = hub.NodeOperationIssuingHub();, /// private IMessageHub IssuingHub => _issuingHub ??= hub.NodeOperationIssuingHub();). /// Hence the alias pass. Its known approximation: an identifier is treated as a seam alias for the From dc37098ad24e9ccef190c74f92902247cfc94dae Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Roland=20B=C3=BCrgi?= <6334612+rbuergi@users.noreply.github.com> Date: Wed, 16 Sep 2026 09:47:23 +0200 Subject: [PATCH 4/4] fix(guard): close the matcher's syntax holes and pin both recycle surfaces at RUNTIME MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Addresses all three findings from the automatic review on #4477. 1. THE MATCHER COULD BE EVADED BY A SYNTAX-ONLY REFACTOR (critical, valid). It keyed on the unqualified `new Foo(` spelling alone, so three legal constructions of the SAME lifecycle message produced no site at all — `new MeshWeaver.Messaging.DisposeRequest { … }`, a target-typed `DisposeRequest request = new();`, and `hub.Post(new() { … })` where the type is on the CALL. A fourth, a message arriving as a PARAMETER, was equally invisible. Each was a way to introduce an un-hopped teardown that both ratchet tests would have called clean — the defect this guard exists to prevent, one level along again. All four are matched now, with one planted case each so a regression names the spelling instead of moving a count by one. Observe's type argument is deliberately NOT read as the message: it is the RESPONSE (`Observe(new CreateNodeRequest(…))`), and there is a planted case pinning that too. Measured over src/: the widened matcher adds ZERO sites (no such spelling exists in the tree today) and still finds exactly the one allowed violation. Pure hardening, no inventory change. Control: reverting the qualifier and the declared-local rules reds the spelling cases while the main ratchet stays GREEN — which is precisely the review's point. 2. BOTH FIXES WERE PINNED ONLY BY SOURCE (moderate ×2, valid). Every existing test of both verbs drives them through a session or client hub, whose non-mesh address makes NodeOperationIssuingHub() the identity function — so none of them executes the branch #4463 is about, and a revert to `hub.Post` would leave the whole recycle suite green. Two runtime tests now drive MeshOperations.Recycle and HubRecycleExtensions.RecycleNode from the ROOT mesh hub and read what the ROUTER_TRAFFIC detector actually logged, in the fixture that already carries a proven-can-fail capture. Reverted in rehearsal, both reproduce the production line byte for byte, down to the frame: ROUTER_TRAFFIC ORIGIN: DisposeRequest was POSTED with the mesh hub as sender … at MeshWeaver.AI.MeshOperations+<>c__DisplayClass95_0 .b__3 Each asserts a positive anchor FIRST (status=Recycled; the node coming back), because the verb answers a refusal without posting anything at all — "no router traffic" would otherwise be trivially true of an operation that never ran. 3. THE ORDERING CLAIM ON RecycleNode (critical ×2). The review is right that a root-hub caller now has the dispose on portal/nodeops-* and the read on portal/reads-*, two independent action blocks — but wrong that the seam introduced it: BEFORE this change a root-hub caller posted the dispose from mesh/{id} and the read from portal/reads-*, two hubs then as well, with the router on an end of the teardown besides. For every caller this class documents, both seams are the identity function, so both deliveries still leave the same hub and the queue-order argument holds unchanged. Rather than paper over it or redesign the wait here, the precondition is now STATED on the code and in the doc, alongside this class's other named residual: closing it means an explicit disposal-completion barrier, which is a different change with its own design. Co-Authored-By: Claude Opus 5 (1M context) --- .../Architecture/RouterTrafficDetection.md | 22 ++- .../HubRecycleExtensions.cs | 18 +++ ...RouterAsNodeOperationOriginRatchetGuard.cs | 126 ++++++++++++++++-- .../MeshWeaver.Graph.Test.csproj | 4 + ...erTrafficOnNodeCreateFromTheRootHubTest.cs | 107 +++++++++++++++ 5 files changed, 262 insertions(+), 15 deletions(-) diff --git a/src/MeshWeaver.Documentation/Data/Architecture/RouterTrafficDetection.md b/src/MeshWeaver.Documentation/Data/Architecture/RouterTrafficDetection.md index 0dc1fb0ce2..409fb954b1 100644 --- a/src/MeshWeaver.Documentation/Data/Architecture/RouterTrafficDetection.md +++ b/src/MeshWeaver.Documentation/Data/Architecture/RouterTrafficDetection.md @@ -212,6 +212,18 @@ holds a hub that outlives the target" — constrains who *should* call it, not w the identity function for every documented caller, so adopting it costs them nothing and removes the undocumented one. +> 🚨 **A known residual on `RecycleNode`, stated rather than assumed.** Its wait works by queue +> order: the read is issued after the dispose, so it lands behind it *at the target*. That holds +> because both deliveries leave the **same** hub — for every documented caller both seams are the +> identity function. A **root-hub** caller, the one this class's rule already excludes, hops to two +> different off-router hubs (`portal/nodeops-{meshId}` and `portal/reads-{meshId}`) whose action +> blocks are independent, so the read can in principle be answered by the still-live hub and +> `RecycleNode` emits before a fresh activation. That is **not** a cost of adopting the seam: before +> it, a root-hub caller posted the dispose from `mesh/{id}` and the read from `portal/reads-{meshId}` +> — two hubs then as well, with the router on an end of the teardown besides. Closing it properly +> means an explicit disposal-completion barrier instead of relying on queue order, which is a +> different change with its own design. + ## Reading a report `ROUTER_TRAFFIC ORIGIN:` prints up to twelve frames, with `MessageHub`'s own plumbing dropped off the @@ -228,7 +240,15 @@ another process. and both genuine end roles must still fire. - `RouterTrafficOnNodeCreateFromTheRootHubTest` — the create seam (`IMeshService.CreateNode` issued from the root hub keeps the router off both ends) plus the origin site's call-site attribution, - each with its own positive control in the same fixture. + each with its own positive control in the same fixture. It also carries the two RECYCLE surfaces + driven from the ROOT hub — `MeshOperations.Recycle` and `HubRecycleExtensions.RecycleNode`. Those + exist because **every other test of both verbs drives them through a session or client hub, where + the seam is the identity function**, so a revert to `hub.Post` would leave the whole existing + recycle suite green. Reverted in rehearsal, they reproduce #4463's production line byte for byte, + down to the frame: `at MeshWeaver.AI.MeshOperations+<>c__DisplayClass95_0.b__3`. + Each asserts a positive anchor first (`status=Recycled`; the node coming back), because the verb + answers a refusal *without posting anything*, and "no router traffic" is trivially true of an + operation that never ran. - `RouterAsNodeOperationOriginRatchetGuard` — the `src/` ratchet, in two tests that fail independently: one asserts no new router-issued lifecycle site, the other asserts that the denominator was actually DERIVED (both families non-empty, every derived name resolving to a real diff --git a/src/MeshWeaver.Mesh.Contract/HubRecycleExtensions.cs b/src/MeshWeaver.Mesh.Contract/HubRecycleExtensions.cs index edcf0cb455..ebdd56c20a 100644 --- a/src/MeshWeaver.Mesh.Contract/HubRecycleExtensions.cs +++ b/src/MeshWeaver.Mesh.Contract/HubRecycleExtensions.cs @@ -115,6 +115,24 @@ public static class HubRecycleExtensions // and is answered by the reactivated hub (or NACKed ShuttingDown and re-probed until it // is). Deliberately NOT ReadTimeoutBehavior.EmitNull: "I could not tell" must reach the // caller as an error, because the caller's next act is to send a user somewhere. + // + // 🚨 THE PRECONDITION OF THAT ORDERING, stated rather than assumed. It holds because + // BOTH deliveries leave the SAME hub: for every caller this class documents — one + // holding a surviving non-router hub — `NodeOperationIssuingHub()` above and the + // `ReadIssuingHub()` inside `GetMeshNode` are each the identity function, so the + // dispose and the read are posted by the caller's own action block in that order. + // A ROOT-HUB caller is the exception, and it is the caller this class's own rule + // already excludes: there the two hop to DIFFERENT off-router hubs + // (`portal/nodeops-{meshId}` and `portal/reads-{meshId}`), whose action blocks are + // independent, so the read can in principle reach the target first and be answered + // by the still-live hub — whereupon this method emits before a fresh activation. + // 🚨 That is NOT introduced by the seam adoption above and must not be read as its + // cost: before it, a root-hub caller posted the dispose from `mesh/{id}` and the + // read from `portal/reads-{meshId}` — two hubs then as well, and with the router on + // an end of the teardown besides. Closing it properly means an explicit + // disposal-completion barrier rather than relying on queue order, which is a + // different change with its own design; it is named here, in the same register as + // the two-installs-one-root residual on this class, rather than papered over. return hub.GetMeshNode(path, budget ?? DefaultRecycleBudget); })); diff --git a/test/MeshWeaver.Documentation.Test/RouterAsNodeOperationOriginRatchetGuard.cs b/test/MeshWeaver.Documentation.Test/RouterAsNodeOperationOriginRatchetGuard.cs index 1cc48b52c2..51812d3f53 100644 --- a/test/MeshWeaver.Documentation.Test/RouterAsNodeOperationOriginRatchetGuard.cs +++ b/test/MeshWeaver.Documentation.Test/RouterAsNodeOperationOriginRatchetGuard.cs @@ -68,12 +68,22 @@ namespace MeshWeaver.Documentation.Test; /// Every derived name must also RESOLVE to a real type, so a rename or a move reddens this guard /// instead of silently shrinking what it measures. /// -/// What is matched. A .Observe(…) / .Post(…) call whose FIRST argument is -/// a lifecycle message — either constructed inline (new CreateNodeRequest(node)) or hoisted -/// into a local earlier in the file (var request = new DeleteNodeRequest(path); … -/// IssuingHub.Observe(request, …), which is MeshService's own shape and which a -/// constructor-anchored scan would have classified on the wrong statement). The receiver is then -/// read as a primary expression and compared against the seam. +/// What is matched. A .Observe(…) / .Post(…) call that names a lifecycle +/// message, in any of the spellings C# offers — because a matcher a SYNTAX-ONLY refactor can slip +/// past is a matcher that reports an un-hopped post as clean: +/// +/// constructed inline, qualified or not — new CreateNodeRequest(node), +/// new MeshWeaver.Messaging.DisposeRequest { … }; +/// hoisted into a local earlier in the file — var request = new DeleteNodeRequest(path); +/// … IssuingHub.Observe(request, …), which is MeshService's own shape and which a +/// constructor-anchored scan would have classified on the wrong statement; +/// DECLARED with the type and target-typed — DisposeRequest request = new(); — or +/// arriving as a parameter or field, where the type never appears next to a new at all; +/// named on the CALL — hub.Post<DisposeRequest>(new() { … }), whose argument +/// mentions no type. Only for Post: Observe's type argument is the RESPONSE. +/// +/// Each has its own planted case, so a regression names the spelling rather than moving a count. +/// The receiver is then read as a primary expression and compared against the seam. /// /// 🚨 One structural exclusion: a SELF-DIRECTED hub-lifecycle post. A hub telling /// ITSELF to dispose, initialise or shut down — no target, or o.WithTarget(thatHub.Address) — @@ -151,12 +161,18 @@ public class RouterAsNodeOperationOriginRatchetGuard(ITestOutputHelper output) private const string AllowFileName = "RouterNodeOperationOriginSites.allow"; /// - /// The request/response and fire-and-forget entry points, tolerant of an explicit response type - /// argument (Observe<CreateNodeResponse>() and of the line break C# style puts - /// between the receiver and the call. + /// The request/response and fire-and-forget entry points, tolerant of an explicit type argument + /// and of the line break C# style puts between the receiver and the call. + /// + /// Group 1 is set only for Post, and group 2 carries the explicit type argument, + /// because the two entry points mean OPPOSITE things by it: Post<TMessage> names the + /// message — so hub.Post<DisposeRequest>(new() { … }) is a lifecycle post whose + /// argument mentions no type at all — whereas Observe<TResponse> names the RESPONSE + /// (Observe<CreateNodeResponse>(new CreateNodeRequest(…))), and keying on that would + /// classify on the wrong half of the exchange. /// private static readonly Regex CallMarker = - new(@"\.\s*(?:Observe|Post)\s*(?:<[^<>()]*>\s*)?\(", RegexOptions.Compiled); + new(@"\.\s*(?:(Post)|Observe)\s*(?:<([^<>()]*)>\s*)?\(", RegexOptions.Compiled); /// A call to either seam. Both count: a site that hops for reads is off the router. private static readonly Regex SeamCall = @@ -208,9 +224,19 @@ public LifecycleVerbs(ImmutableHashSet hub, ImmutableHashSet nod Node = node; All = hub.Union(node); var alternation = string.Join("|", All.Order(StringComparer.Ordinal)); - NewRequest = new Regex(@"\bnew\s+(" + alternation + @")\b"); + // `new Foo(…)` and `new A.B.Foo(…)` alike: a namespace-qualified construction is the + // same site, and a matcher that only saw the unqualified spelling could be evaded by a + // pure syntax refactor (Copilot, #4477). + const string Qualifier = @"(?:[A-Za-z_][A-Za-z0-9_]*\s*\.\s*)*"; + NewRequest = new Regex(@"\bnew\s+" + Qualifier + "(" + alternation + @")\b"); RequestLocal = new Regex( - @"\b([A-Za-z_][A-Za-z0-9_]*)\s*=\s*new\s+(" + alternation + @")\b"); + @"\b([A-Za-z_][A-Za-z0-9_]*)\s*=\s*new\s+" + Qualifier + "(" + alternation + @")\b"); + // An identifier DECLARED with a lifecycle type, whatever it is later assigned: + // `DisposeRequest request = new();` (target-typed, so the type appears only on the + // left), a parameter `void Send(DisposeRequest request)`, a field, a `case Foo f:` + // pattern. Without this, target-typed `new()` is a hole the guard cannot see. + DeclaredLocal = new Regex( + @"\b(" + alternation + @")\s+([A-Za-z_][A-Za-z0-9_]*)\b"); } public ImmutableHashSet Hub { get; } @@ -224,6 +250,9 @@ public LifecycleVerbs(ImmutableHashSet hub, ImmutableHashSet nod /// A local or field bound to a freshly constructed lifecycle message. public Regex RequestLocal { get; } + + /// An identifier DECLARED with a lifecycle type — group 1 the type, group 2 the name. + public Regex DeclaredLocal { get; } } /// One matched call: where it is, which verb, and how it is addressed. @@ -401,6 +430,41 @@ IObservable NotARequest(IMessageHub hub) => verbs); Assert.True(InDenominator(Assert.Single(plantedNodeSelfPost), verbs)); + // 🚨 THE SPELLINGS A SYNTAX-ONLY REFACTOR COULD HIDE BEHIND (Copilot on #4477). Each of + // these is legal C# that constructs the SAME lifecycle message without the unqualified + // `new Foo(` the first matcher keyed on, so each was a way to introduce an un-hopped + // teardown that both ratchet tests would report as clean. One case per spelling, so a + // regression names WHICH one broke rather than moving a count by one. + AssertOneViolatingSite( + "qualified construction", + "class P { void M(IMessageHub hub) => hub.Post(new MeshWeaver.Messaging.DisposeRequest " + + "{ Reason = r }, o => o.WithTarget(new Address(p))); }", + verbs); + AssertOneViolatingSite( + "target-typed new() through a declared local", + "class P { void M(IMessageHub hub) { DisposeRequest request = new() { Reason = r }; " + + "hub.Post(request, o => o.WithTarget(new Address(p))); } }", + verbs); + AssertOneViolatingSite( + "the message type on the call: Post(new())", + "class P { void M(IMessageHub hub) => hub.Post(new() { Reason = r }, " + + "o => o.WithTarget(new Address(p))); }", + verbs); + AssertOneViolatingSite( + "a lifecycle message arriving as a PARAMETER", + "class P { void M(IMessageHub hub, DeleteNodeRequest request) => " + + "hub.Observe(request, o => o.WithTarget(new Address(p))); }", + verbs); + + // 🚨 …and the other half of that: Observe's type argument is the RESPONSE, so it must NOT + // be read as the message. Without this, `Observe(somethingElse)` would + // be classified as a node-lifecycle post on whatever receiver it happened to have. + Assert.Empty(SitesIn( + "planted-response-type-arg.cs", + "class P { void M(IMessageHub hub) => hub.Observe(somethingElse, " + + "o => o.WithTarget(t)); }", + verbs)); + // Comment masking, both sides. The same line is a site when it is code and nothing when it // is prose — which is the difference between measuring the tree and measuring the remarks // that describe it. This repo's remarks quote these shapes verbatim. @@ -428,6 +492,24 @@ IObservable NotARequest(IMessageHub hub) => + "#4463 added is measuring nothing."); } + /// + /// One planted snippet must yield exactly one site, counted, and NOT off-router — i.e. the + /// spelling is seen AND reported. Named, so a regression says which construction broke. + /// + private static void AssertOneViolatingSite(string spelling, string snippet, LifecycleVerbs verbs) + { + var sites = SitesIn("planted-" + spelling + ".cs", snippet, verbs); + Assert.True(sites.Count == 1, + $"the {spelling} spelling produced {sites.Count} sites, not 1 — a lifecycle message the " + + "matcher cannot SEE is a way to introduce an un-hopped post that this whole guard " + + $"reports as clean. Snippet: {snippet}"); + var site = sites[0]; + Assert.True(InDenominator(site, verbs), + $"the {spelling} spelling was matched but fell OUT of the denominator — {snippet}"); + Assert.False(site.OffRouter, + $"the {spelling} spelling was matched but read as already off-router — {snippet}"); + } + /// /// Whether a matched site is in the denominator: a self-directed HUB-lifecycle post is not, /// because the seam's remedy — issue it from a different hub — would misdeliver it. @@ -566,9 +648,16 @@ private static IReadOnlyList SitesIn(string file, string text, LifecycleVe var code = SourceScan.MaskCommentsAndStrings(text); + // Both ways a name comes to hold a lifecycle message: assigned from a construction + // (`var request = new DeleteNodeRequest(path)`) and DECLARED with the type + // (`DisposeRequest request = new();`, a parameter, a field). The earliest binding wins, so a + // call is attributed only to a binding that precedes it. var requestLocals = new Dictionary(StringComparer.Ordinal); + foreach (Match m in verbs.DeclaredLocal.Matches(code)) + if (!requestLocals.ContainsKey(m.Groups[2].Value)) + requestLocals[m.Groups[2].Value] = (m.Index, m.Groups[1].Value); foreach (Match m in verbs.RequestLocal.Matches(code)) - if (!requestLocals.ContainsKey(m.Groups[1].Value)) + if (!requestLocals.TryGetValue(m.Groups[1].Value, out var already) || m.Index < already.At) requestLocals[m.Groups[1].Value] = (m.Index, m.Groups[2].Value); var seamAliases = SeamAlias.Matches(code) @@ -583,13 +672,22 @@ private static IReadOnlyList SitesIn(string file, string text, LifecycleVe string verb; var constructed = verbs.NewRequest.Match(argument); + var typeArgument = call.Groups[2].Value.Trim(); if (constructed.Success) { verb = constructed.Groups[1].Value; } + else if (call.Groups[1].Success && verbs.All.Contains(typeArgument)) + { + // `hub.Post(new() { … })` — the message type is on the CALL, and the + // argument names no type at all. Only for Post: Observe's type argument is the + // RESPONSE (see CallMarker's remarks). + verb = typeArgument; + } else { - // The hoisted spelling: the request was built into a local a few lines up. + // The hoisted spelling: the request was built into, or declared as, a local a few + // lines up. var head = argument.Split('.')[0].Trim(); if (!BareIdentifier.IsMatch(head) || !requestLocals.TryGetValue(head, out var bound) diff --git a/test/MeshWeaver.Graph.Test/MeshWeaver.Graph.Test.csproj b/test/MeshWeaver.Graph.Test/MeshWeaver.Graph.Test.csproj index d5e710b424..f54728d5de 100644 --- a/test/MeshWeaver.Graph.Test/MeshWeaver.Graph.Test.csproj +++ b/test/MeshWeaver.Graph.Test/MeshWeaver.Graph.Test.csproj @@ -18,6 +18,10 @@ + + diff --git a/test/MeshWeaver.Graph.Test/RouterTrafficOnNodeCreateFromTheRootHubTest.cs b/test/MeshWeaver.Graph.Test/RouterTrafficOnNodeCreateFromTheRootHubTest.cs index 1b86d2f117..0ccda4cad8 100644 --- a/test/MeshWeaver.Graph.Test/RouterTrafficOnNodeCreateFromTheRootHubTest.cs +++ b/test/MeshWeaver.Graph.Test/RouterTrafficOnNodeCreateFromTheRootHubTest.cs @@ -3,7 +3,9 @@ using System.Collections.Generic; using System.Linq; using System.Reactive.Linq; +using System.Text.Json; using System.Threading.Tasks; +using MeshWeaver.AI; using MeshWeaver.Fixture; using MeshWeaver.Hosting.Monolith.TestBase; using MeshWeaver.Mesh; @@ -138,6 +140,111 @@ public async Task ACreateIssuedFromTheRootMeshHub_NeverPutsTheRouterOnEitherEnd( + "sees a violating post even when the receiving hub's own report is muted"); } + /// + /// 🚨 The RUNTIME pin for #4463 + /// — the operator recycle (the Recycle tool / Compile button). + /// + /// Why the source ratchet is not enough on its own. Every other test of this verb + /// drives it through a SessionHubFactory hub, whose non-mesh address makes + /// NodeOperationIssuingHub() the IDENTITY FUNCTION — so none of them executes the branch + /// #4463 is about, and reverting RecycleCore to hub.Post would leave them all + /// green. RouterAsNodeOperationOriginRatchetGuard would catch the revert, but it reads + /// SOURCE; this reads what the detector actually logged, which is the artefact production + /// produced. + /// + /// The shape is production's. #4463's line was posted from + /// MeshWeaver.AI.MeshOperations+<>c__DisplayClass95_0.<RecycleCore>b__3 with + /// sender: mesh/… — an agent-surface MeshOperations built over the DI-injected + /// hub, which in the mesh's root container IS the router. new MeshOperations(Mesh) is + /// that, verbatim. + /// + /// The positive anchor is not decoration. The verb REFUSES rather than posting when + /// the release-request stamp is denied, and a refusal emits no DisposeRequest at all — so + /// "no router traffic" would be trivially true of a recycle that never ran. Asserting + /// status=Recycled first is what makes the silence below mean something. + /// + [Fact(Timeout = 120_000)] + public async Task AnOperatorRecycleIssuedFromTheRootMeshHub_NeverPostsTheTeardownAsTheRouter() + { + var path = await SeedNode("RouterTrafficOperatorRecycleProbe"); + + var answer = await new MeshOperations(Mesh).Recycle(path) + .FirstAsync() + .Await(TestContext.Current.CancellationToken); + + using var envelope = JsonDocument.Parse(answer); + envelope.RootElement.GetProperty("status").GetString().Should().Be("Recycled", + "the verb has to have RUN — it answers a refusal without posting any DisposeRequest at " + + "all, and 'no router traffic' is trivially true of an operation that never happened"); + + DumpReports(); + TeardownOrigins().Should().BeEmpty( + "#4463: the teardown was POSTED with the mesh hub as sender, from this exact frame. " + + "RecycleCore must issue it on NodeOperationIssuingHub(), which is the identity " + + "function for every non-router caller and the off-router execution hub for this one"); + TeardownReports().Should().BeEmpty( + "and the receiving hub must not see the router at an end of the teardown either"); + } + + /// + /// 🚨 The same runtime pin for the framework's ONE recycle surface, + /// HubRecycleExtensions.RecycleNode. Its own tests deliberately drive it through + /// RequestHub — again a non-mesh address, again the seam as identity — so a regression to + /// hub.Post would keep them green. This is the root-hub branch, with the router-traffic + /// capture as the instrument. + /// + /// The positive anchor here is the emission itself: RecycleNode answers only once + /// the address has served a read again, so a node coming back proves the teardown was posted, + /// executed, and the hub re-activated. + /// + [Fact(Timeout = 120_000)] + public async Task ARecycleNodeIssuedFromTheRootMeshHub_NeverPostsTheTeardownAsTheRouter() + { + var path = await SeedNode("RouterTrafficRecycleNodeProbe"); + + var node = await Mesh.RecycleNode(path) + .FirstAsync() + .Await(TestContext.Current.CancellationToken); + + node.Should().NotBeNull( + "RecycleNode answers only once the recycled address serves a read again, so this " + + "emission is the proof that the teardown was actually posted and executed"); + + DumpReports(); + TeardownOrigins().Should().BeEmpty( + "a teardown issued from the root hub makes the ROUTER the sender of a work delivery — " + + "the #4463 shape, one call frame further out"); + TeardownReports().Should().BeEmpty( + "and the receiving hub must not see the router at an end of the teardown either"); + } + + /// The node the recycle targets has to exist before it can be torn down. + private async Task SeedNode(string id) + { + var created = await Mesh.ServiceProvider.GetRequiredService() + .CreateNode(new MeshNode(id, TestPartition) { Name = id, NodeType = "Markdown" }) + .FirstAsync() + .Await(TestContext.Current.CancellationToken); + created.Path.Should().Be($"{TestPartition}/{id}", + "a recycle of a node that was never created would tear nothing down and emit no " + + "traffic, making the assertions that follow vacuous"); + return created.Path; + } + + /// + /// 🚨 Filtered on the TEARDOWN specifically, not on "any record". The origin site always carries + /// the real CLR type, so this is exact there; the receiver side reports a routed payload as + /// RawJson, so the companion filter below also counts a record whose ends match the + /// teardown's. Keeping the filter narrow is deliberate: these two tests pin ONE defect, and a + /// blanket "no router traffic anywhere" assertion would red on any unrelated pre-existing line + /// and teach the next reader to widen it rather than read it. + /// + private RouterTrafficRecord[] TeardownOrigins() => + Origins().Where(r => r.MessageType == nameof(DisposeRequest)).ToArray(); + + private RouterTrafficRecord[] TeardownReports() => + Reports().Where(r => r.MessageType == nameof(DisposeRequest)).ToArray(); + /// /// 🚨 THE NEGATIVE CONTROL, and it is not optional. The measurement above reads "no records" as /// "the seam held" — a reading that is valid only if this fixture's capture provider is really