Skip to content

Router-traffic detector reported every routing HOP as a violation - #1069

Merged
rbuergi merged 10 commits into
mainfrom
fix/router-traffic-end-role
Aug 10, 2026
Merged

rbuergi merged 10 commits into
mainfrom
fix/router-traffic-end-role

Conversation

@rbuergi

@rbuergi rbuergi commented Aug 10, 2026

Copy link
Copy Markdown
Contributor

The defect

MessageHub.ReportRouterTraffic asked the rule about Address.Type — THIS hub, the one receiving the delivery:

RouterTrafficRule.RoleOf(Address.Type, delivery.Sender?.Type, delivery.Message)

But RouterTrafficRule's own contract is whether the mesh hub is an END of the delivery. Those differ on every routed message: HierarchicalRouting.cs:218 sends a hosted hub's non-local delivery UP via parentHub.DeliverMessage(delivery), and for essentially every hub in the process that parent is the root mesh hub. So mesh.DeliverMessage runs on nearly all cross-address traffic, and the detector named the router's own job — forwarding — a violation.

A monolith repro of merely "validate a token + read a node" emitted 5 ERROR lines, all of them hops. A detector that cries wolf on the mesh's actual job is the one that gets muted, taking its true positives with it — and one such defect has previously produced ~50 red-log incidents (StructuralIdentityTest documents exactly that).

The log line was untruthful the same way: it passed Address as {Target}, so a report named the hop the delivery was passing through, not where it was addressed.

The fix

-        var role = RouterTrafficRule.RoleOf(Address.Type, delivery.Sender?.Type, delivery.Message);
+        // Target = where the delivery is ADDRESSED, not where it is currently being handled.
+        var role = RouterTrafficRule.RoleOf(delivery.Target?.Type, delivery.Sender?.Type, delivery.Message);
...
-            messageType, role, delivery.Sender?.ToString() ?? "(none)", Address);
+            messageType, role, delivery.Sender?.ToString() ?? "(none)", delivery.Target?.ToString() ?? "(none)");

Plus the XML docs on both sides, which now state that "an END" means an end of the delivery.

This is NOT muting the detector

It narrows nothing real. Both genuine end roles still fire, and both are pinned by tests that pass with the fix reverted:

Shape Before After
client → host (HOP through the mesh) ❌ ERROR "target", target: mesh/1 ✅ silent
request addressed AT mesh/{id} ERROR "target" ERROR "target", target: mesh/1
mesh hub as SENDER (NodeOperationTarget) ERROR "sender" ERROR "sender", truthful target

Per-(role, messageType) dedup and the HeartBeatEvent exclusion are untouched.

Tests

RouterTrafficRuleTest pins the predicate, but it cannot see which values the call site feeds it — which is exactly where the defect lived. New RouterTrafficDetectorTest drives all three shapes through a real hub hierarchy (HubTestBase: mesh hosting host + client) and reads the ERROR the detector actually logged, asserting on the structured values (role/sender/target) rather than the prose.

Verified failing before the change — reverting the one-line call-site edit:

MeshWeaver.Messaging.Hub.Test.RouterTrafficDetectorTest.ADeliveryMerelyRoutedThroughTheMeshHub_IsNotReported [FAIL]
  AssertionException : Expected collection to be empty because the mesh hub is neither end of a
  client→host delivery — it only FORWARDS it. ..., but found 1 item(s).

  ROUTER_TRAFFIC captured: { MessageType = HopPing, Role = target, Sender = client/fbe360ccac4a, Target = mesh/1 }
  ROUTER_TRAFFIC captured: { MessageType = Pong,    Role = target, Sender = host/1,               Target = mesh/1 }

Both legs of a client → host round trip reported against the hub that merely forwarded them — and Target = mesh/1 shows the untruthful field in the same output. The other two tests (genuine target end, mesh as sender) passed with the fix reverted, which is the proof the change silences neither.

  • dotnet test test/MeshWeaver.Messaging.Hub.Test — 171/171 passed
  • DocumentationLinkIntegrityTest — passed
  • dotnet build -c Release -warnaserror on MeshWeaver.Messaging.Contract, MeshWeaver.Messaging.Hub, MeshWeaver.Messaging.Hub.Test — 0 warnings, 0 errors

Notes

🤖 Generated with Claude Code

rbuergi and others added 2 commits August 10, 2026 07:42
`MessageHub.ReportRouterTraffic` asked `RouterTrafficRule.RoleOf` about
`Address.Type` — THIS hub, the one receiving the delivery — while the rule's
contract is whether the mesh hub is an END of the delivery. Those differ on
every routed message: `HierarchicalRouting` sends a hosted hub's non-local
delivery UP via `parentHub.DeliverMessage(delivery)`, and for essentially
every hub in the process that parent is the root mesh hub. So
`mesh.DeliverMessage` runs on nearly all cross-address traffic and the
detector named the router's own job — forwarding — a violation. A monolith
repro of merely "validate a token + read a node" emitted 5 ERROR lines, all
hops; one such defect has previously produced ~50 red-log incidents.

The log line was untruthful in the same way: it passed `Address` as
`{Target}`, so a report named the hop the delivery was passing through
rather than where it was addressed.

Fix: pass the DELIVERY's own ends — `delivery.Target?.Type` and
`delivery.Sender?.Type` — and log `delivery.Target`. This narrows nothing
real: a request addressed AT `mesh/{id}` still reports "target", and the
mesh hub as SENDER (the genuine NodeOperationTarget violation and the real
volume driver) still reports "sender". Dedup per (role, messageType) and the
HeartBeatEvent exclusion are untouched.

`RouterTrafficRuleTest` pins the predicate; it cannot see which values the
call site feeds it, which is where the defect lived. `RouterTrafficDetectorTest`
drives all three shapes through a real hub hierarchy and reads the ERROR the
detector actually logged. Reverting the one-line call-site change fails it
with:

  Expected collection to be empty ..., but found 1 item(s).
  ROUTER_TRAFFIC: HopPing has the mesh hub as target
    (sender: client/fbe360ccac4a, target: mesh/1)

— a client→host delivery, reported against the hub that merely forwarded it.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Copilot AI lite review requested due to automatic review settings August 10, 2026 05:43

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Fixes a false-positive in the router-traffic detector where every routed hop through the root mesh hub was reported as a violation, by correctly evaluating the delivery’s addressed target (and logging the truthful sender/target pair).

Changes:

  • MessageHub.ReportRouterTraffic now evaluates delivery.Target (delivery end) instead of the receiving hub’s Address, and logs the delivery’s actual target.
  • Clarifies the “end of delivery” contract in RouterTrafficRule XML docs and the existing predicate tests.
  • Adds RouterTrafficDetectorTest to exercise a real hub hierarchy and assert the detector’s structured log values for: routed hop (silent), mesh-as-target (reported), mesh-as-sender (reported).

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated no comments.

Show a summary per file
File Description
test/MeshWeaver.Messaging.Hub.Test/RouterTrafficRuleTest.cs Documents the predicate contract and adds a unit test that distinguishes routed-through traffic from true mesh-ended deliveries.
test/MeshWeaver.Messaging.Hub.Test/RouterTrafficDetectorTest.cs New integration-style test capturing ROUTER_TRAFFIC logs to pin correct behavior for hop vs. true end roles.
src/MeshWeaver.Messaging.Hub/MessageHub.cs Fixes the detector call-site to use delivery.Target and logs the delivery’s actual target instead of the current hop hub.
src/MeshWeaver.Messaging.Contract/RouterTrafficRule.cs Updates XML docs to explicitly define “end” in terms of delivery endpoints (target/sender).
src/MeshWeaver.Documentation/Data/WhatsNew/2026-08-10-router-alerts-name-only-real-problems.md Adds a What’s New entry explaining the user-visible reduction in false alerts.

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

@github-actions

github-actions Bot commented Aug 10, 2026 •

Copy link
Copy Markdown
Contributor

Test Results (shard 0)

701 tests  ±0   701 ✅ ±0   13m 59s ⏱️ + 2m 33s
 10 suites ±0     0 💤 ±0 
 10 files   ±0     0 ❌ ±0 

Results for commit 9b1c7a5. ± Comparison against base commit f2b9cce.

♻️ This comment has been updated with latest results.

@github-actions

github-actions Bot commented Aug 10, 2026 •

Copy link
Copy Markdown
Contributor

Test Results (shard 1)

1 202 tests  +4   1 202 ✅ +4   6m 13s ⏱️ +12s
   11 suites ±0       0 💤 ±0 
   11 files   ±0       0 ❌ ±0 

Results for commit 9b1c7a5. ± Comparison against base commit f2b9cce.

♻️ This comment has been updated with latest results.

@github-actions

github-actions Bot commented Aug 10, 2026 •

Copy link
Copy Markdown
Contributor

Test Results (shard 4)

1 316 tests  ±0   1 311 ✅ ±0   6m 8s ⏱️ -54s
   11 suites ±0       5 💤 ±0 
   11 files   ±0       0 ❌ ±0 

Results for commit 9b1c7a5. ± Comparison against base commit f2b9cce.

♻️ This comment has been updated with latest results.

@github-actions

github-actions Bot commented Aug 10, 2026 •

Copy link
Copy Markdown
Contributor

Test Results (shard 2)

1 979 tests  ±0   1 977 ✅ ±0   6m 38s ⏱️ +7s
   11 suites ±0       2 💤 ±0 
   11 files   ±0       0 ❌ ±0 

Results for commit 9b1c7a5. ± Comparison against base commit f2b9cce.

♻️ This comment has been updated with latest results.

@github-actions

github-actions Bot commented Aug 10, 2026 •

Copy link
Copy Markdown
Contributor

Test Results (shard 5)

1 756 tests  ±0   1 564 ✅ ±0   5m 53s ⏱️ +12s
   11 suites ±0     192 💤 ±0 
   11 files   ±0       0 ❌ ±0 

Results for commit 9b1c7a5. ± Comparison against base commit f2b9cce.

♻️ This comment has been updated with latest results.

@github-actions

github-actions Bot commented Aug 10, 2026 •

Copy link
Copy Markdown
Contributor

Test Results (shard 3)

   11 files  ±0     11 suites  ±0   6m 0s ⏱️ + 1m 3s
1 977 tests ±0  1 869 ✅ ±0  108 💤 ±0  0 ❌ ±0 
2 139 runs  +1  2 031 ✅ +1  108 💤 ±0  0 ❌ ±0 

Results for commit 9b1c7a5. ± Comparison against base commit f2b9cce.

♻️ This comment has been updated with latest results.

@github-actions

github-actions Bot commented Aug 10, 2026 •

Copy link
Copy Markdown
Contributor

Test Results

   65 files  ±0     65 suites  ±0   44m 54s ⏱️ + 3m 14s
8 931 tests +4  8 624 ✅ +4  307 💤 ±0  0 ❌ ±0 
9 093 runs  +5  8 786 ✅ +5  307 💤 ±0  0 ❌ ±0 

Results for commit 9b1c7a5. ± Comparison against base commit f2b9cce.

♻️ This comment has been updated with latest results.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants