Repository navigation
Background services ran their work on the mesh router — issue it off-router - #1167
Conversation
…router Issue #1113: 840 ROUTER_TRAFFIC errors in ~8h across 6 memex pods — deliveries with the mesh ROUTER hub as sender or target. Live-log triage (24h, current image) put ~97% of the volume on the node-CRUD fallback already fixed by the nodeops retarget (eea2dd2) and the transport token validation fixed by 50bfb0f. Two producer classes remained on main; both are fixed here at their roots. 1. Mesh-singleton services hold the DI-injected root hub — the ROUTER — and issued request/response work on it: one-shot GetMeshNode reads (GetDataResponse / DeliveryFailure addressed straight back at mesh/{id}: "sender: Hosting/_Access/Public_Access", "sender: Plugins/_DefaultInstallLedger"), content imports (ImportContentResponse → mesh, "sender: Agent"), and target-less CreateOrUpdateNodeRequests — which EXECUTE on the posting hub, so the plugin default-install's bulk upserts ran on the router's action block. Fix: NodeOperationIssuingHub — the shared off-router issuing seam, extracted from MeshService.IssuingHub — applied at the helper roots (GetMeshNode, the content import/sync builders) and at every direct target-less CreateOrUpdate post (LogIncidentIngestService, PackageInstaller, ModuleDiscoveryService, InstanceSyncWorker, GitSync services, MeshDocumentSink). A non-router hub gets itself back, so portal/session/import callers are byte-for-byte unchanged. 2. The routing layer's own undeliverable-mail NACK (RoutingServiceBase.PostNotFound / NackRouteFailure) posts its DeliveryFailure from the mesh hub via ResponseFor — the router's own duty, reported as a violation ("RawJson has the mesh hub as sender" for every bounced read of a missing node; the payload is RawJson-packed at the detector, so the request-id correlation is the structural signal). Fix: RouterTrafficRule excludes a response the router posts at another hub; a response ADDRESSED AT the router still reports (it proves the router issued a request), as does any non-response send from it. Pinned by NodeOperationIssuingHubTest (the read seam, the missing-node bounce, the write seam, and the detector's own capture staying silent — the bounce line was caught by this very test before fix 2) and the extended RouterTrafficRuleTest NACK cases. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UYLgoPK1qLtyxZ2ixdFtj5
There was a problem hiding this comment.
Pull request overview
This PR addresses issue #1113 by ensuring background/mesh-singleton services and helper paths no longer issue request/response work on the root mesh hub (the router). It introduces a shared “issuing hub” seam to route node operations off-router, and refines the router-traffic detector to avoid flagging the router’s own undeliverable-message NACK responses.
Changes:
- Introduce and apply
NodeOperationIssuingHub()so reads/imports/upserts issued from the DI root mesh hub run on the dedicated off-router execution hub. - Refine
ROUTER_TRAFFICdetection to exclude router-originated responses (NACKs) while still reporting real router-as-end violations. - Add/extend tests to pin both the off-router issuing behavior and the tightened router-traffic rule behavior.
Reviewed changes
Copilot reviewed 18 out of 18 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| test/MeshWeaver.Messaging.Hub.Test/RouterTrafficRuleTest.cs | Extends rule coverage for router NACK response exclusion and non-widening cases. |
| test/MeshWeaver.Hosting.Monolith.Test/NodeOperationIssuingHubTest.cs | New integration test pinning off-router issuing behavior and asserting detector silence. |
| src/MeshWeaver.PluginCatalog/PackageInstaller.cs | Routes target-less upserts through NodeOperationIssuingHub() to keep work off the router. |
| src/MeshWeaver.PluginCatalog/ModuleDiscoveryService.cs | Uses NodeOperationIssuingHub() for hosted-service writes that previously executed on the router. |
| src/MeshWeaver.Observability/LogIncidentIngestService.cs | Issues log-incident node upserts off-router to avoid self-triggering router-traffic errors. |
| src/MeshWeaver.Messaging.Hub/MessageHub.cs | Passes response-correlation signal into RouterTrafficRule for correct NACK classification. |
| src/MeshWeaver.Messaging.Contract/RouterTrafficRule.cs | Adds response-aware exclusion logic for router-posted NACK responses. |
| src/MeshWeaver.Mesh.Contract/MeshNodeStreamExtensions.cs | Issues GetMeshNode reads via NodeOperationIssuingHub() and aligns diagnostics with the issuing hub. |
| src/MeshWeaver.Mesh.Contract/MeshExtensions.cs | Introduces NodeOperationIssuingHub() extension method as the shared off-router seam. |
| src/MeshWeaver.InstanceSync/InstanceSyncWorker.cs | Routes target-less sync upserts through NodeOperationIssuingHub(). |
| src/MeshWeaver.Hosting/MeshService.cs | Reuses the shared NodeOperationIssuingHub() seam for CRUD issuing behavior. |
| src/MeshWeaver.GitSync/IssueService.cs | Issues GitHub issue node upserts off-router. |
| src/MeshWeaver.GitSync/GitHubWebhookProcessor.cs | Routes webhook-driven upserts off-router. |
| src/MeshWeaver.GitSync/GitHubSyncService.cs | Uses NodeOperationIssuingHub() for config writes to avoid router involvement. |
| src/MeshWeaver.GitSync/GitHubCredentialService.cs | Issues credential writes off-router. |
| src/MeshWeaver.Documentation/Data/WhatsNew/2026-08-10-router-stays-a-router.md | Adds a “What’s New” entry documenting the operational fix. |
| src/MeshWeaver.ContentCollections/ContentImportExtensions.cs | Ensures import request/response is issued off-router for mesh-singleton callers. |
| src/MeshWeaver.ContentCollections.Indexing.Graph/MeshDocumentSink.cs | Routes indexing sink upserts off-router. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Test Results (shard 1)1 252 tests +58 1 252 ✅ +58 6m 49s ⏱️ +25s Results for commit 35211f0. ± Comparison against base commit b4920dc. This pull request removes 76 and adds 134 tests. Note that renamed tests count towards both.♻️ This comment has been updated with latest results. |
Test Results (shard 2)1 968 tests - 53 1 966 ✅ - 53 6m 23s ⏱️ -30s Results for commit 35211f0. ± Comparison against base commit b4920dc. This pull request removes 129 and adds 76 tests. Note that renamed tests count towards both.♻️ This comment has been updated with latest results. |
…t surface Copilot review: adding the optional isResponse parameter was a binary breaking change for assemblies compiled against the 3-arg signature. Reintroduce the original overload delegating to the explicit 4-arg form (default removed). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UYLgoPK1qLtyxZ2ixdFtj5
|
(re-posted 2026-10-11; originally by @rbuergi on 2026-08-10) Independent root-cause of #1113 from a parallel investigation converged on the same mechanism as this PR — the seam here is the right one. Three things from that investigation that may be useful, one of which is a gap. 1. The guard is a detector, not a guard — the issue body's impact claim is false. 2. What "801 occurrences" actually measured. The dedupe key is Also worth knowing: the 3. Measured effect of what has already merged (read-only across the three AKS portals — a natural A/B, since they run different images):
~97% already gone. The issue's window (01:08–08:59Z) predates all of it. 4. 🔎 Two producers this PR does not cover. The seam fixes the helper roots, but a direct cross-hub
Both match the I also checked the |
Fixes #1113
No traffic was ever dropped.
ReportRouterTrafficis a detector, not a guard — it logs and returns before routing, so every flagged delivery routed exactly as it would have without it. The issue body's "potential functional impact" does not exist; the cost was log volume and router action-block contention.What was producing the 840 ROUTER_TRAFFIC errors
Live-log triage (24 h of the current memex image, aggregated by type+role) — corroborated by the read-only A/B across the three AKS portals (see the review comment below) — put the volume here:
RawJsonsender=mesh (+CreateNodeRequest/CreateOrUpdateNodeRequesttarget=mesh)ValidateTokenRequest/ResponseGetDataResponse/DeliveryFailure/ImportContentResponse/CreateNodeResponsetarget=meshRawJsonsender=mesh at the issuing hub (bounce of a missing-node read)ClickedEvent/UnsubscribeRequest/PatchDataChangeRequest953ab9245detector, which keyed on the receiving hub'sAddressand reported every routing hop953ab9245emit noneProducer 1 — mesh-singleton services issue work on the DI root hub (= the router)
InstanceAutoRegistrationService(default-install seed + ledger),ModuleDiscoveryService,PackageInstaller,RegistryTokenResolver,LogIncidentIngestService, GitSync services,InstanceSyncWorkerandMeshDocumentSinkall hold the DI-injectedIMessageHub— the root mesh hub — and issued request/response work on it:GetMeshNodereads →GetDataResponse/DeliveryFailureaddressed straight back atmesh/{id}(production lines:sender: Hosting/_Access/Public_Access,sender: Plugins/_DefaultInstallLedger),ImportContentResponse→ mesh (sender: Agent),CreateOrUpdateNodeRequests — which execute on the posting hub, so the plugin default-install's bulk upserts and log-incident creates ran on the router's single-threaded action block (the starvation shape the detector exists for).Fix:
MeshExtensions.NodeOperationIssuingHub— the off-router issuing seam extracted fromMeshService.IssuingHub— applied at the helper roots (GetMeshNode, theImportContent/SyncContentFilesbuilders) so every mesh-singleton caller of those helpers is covered, plus every direct target-lessCreateOrUpdatepost. A non-router hub gets itself back, so portal/session/import callers are byte-for-byte unchanged.Known survivors (direct cross-hub
Observeon the root hub that bypasses the helpers — named in the review comment):PackageInstaller.csPingRequestprobe loop andContentIndexingObserver.cs's cross-hubGetDataRequest. A separate follow-up PR againstmaincovers those (deliberately not stacked and without a closing keyword).Producer 2 — the guard misclassified the router's own undeliverable-mail NACK
RoutingServiceBase.PostNotFound/NackRouteFailurepost theirDeliveryFailurefrom the mesh hub viaResponseFor— explicitly "routing infrastructure's OWN NotFound NACK", i.e. the router doing its job, but the detector reported it as work (RawJson has the mesh hub as senderon every bounced read of a missing node; the payload isRawJson-packed at the detector —RawJsonmeans "opaque here", not a real message type — so the request-id correlation is the structural signal). Fix:RouterTrafficRuleexcludes a response the router posts at another hub. Coverage is preserved: a response addressed at the router still reports (it proves the router issued a request), as does any non-response send from it.NodeOperationIssuingHubTestcaught this arm red before the classification fix — the missing-node read's bounce produced exactly the production line. Per Copilot review, the pre-existing 3-argRoleOfoverload is kept for binary compatibility (shipped Contract package).Evidence
NodeOperationIssuingHubTest(new, MonolithMeshTestBase): seam shape, read seam, missing-node bounce, write seam, and the detector's own captured ERROR stream must stay empty. Red before producer-2's fix (caught the bounce), green after.RouterTrafficRuleTestextended with the NACK cases (exclusion + its three non-widening counter-cases);RouterTrafficDetectorTest16/16.StaleStampRootBindingTest, the pre-existing main flake fixed by The installer started a teardown and handed back without waiting for it #1153 — 3/3 green after merging main), Observability 105/105, GitSync 157/157, ContentCollections 29/29, InstanceSync 24/24, Indexing.Graph 31/31, What's New integrity 1/1.-c Release -warnaserror.🤖 Generated with Claude Code
https://claude.ai/code/session_01UYLgoPK1qLtyxZ2ixdFtj5