Repository navigation
A hub activated on the wrong NodeType now lets go of it - #1143
Merged
Merged
Conversation
A per-node hub binds its HubConfiguration from the node's NodeType exactly once, at activation, then is pinned by address in HostedHubsCollection. Nothing re-reads the NodeType, so a node that acquires (or changes) its type while its hub is alive keeps serving the configuration it was born with -- the mesh DEFAULT when the node had no type at activation. The test reproduces the mechanism with neither the installer nor Roslyn: create a node with no NodeType, activate its hub, retype it to a static NodeType that declares a distinctive area, then render that area through the layout client. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Fixes the mechanism behind #1104. A per-node hub binds its HubConfiguration from its node's NodeType exactly once, during activation, and is then pinned by address in HostedHubsCollection -- routing short-circuits on GetHostedHub for an already- hosted address, so the hub never resolves its path again and nothing re-reads the NodeType. A node that acquires or changes its type while its hub is alive keeps serving the configuration it was born with: the mesh DEFAULT when it had no type. Not cached is not not-pinned. #1055 stopped PathResolutionService from CACHING the fabricated partition-root placeholder, and the installer posts a DisposeRequest after its retype -- both necessary, neither sufficient. Repairing resolution cannot help a hub a bad resolution already activated, and the installer's recycle is fire-and-forget, conditional on the placeholder dance, and available to no other writer. The guarantee belongs in the framework, at the hub. Every activation now arms NodeTypeRebindWatcher on the instance hub (the same WithInitialization + self-DisposeRequest idiom as ArmOverlaySelfHeal): it watches IMeshChangeFeed for its own path and recycles once, the first time an event reports a different NodeType. The change feed -- not the hub's own stream -- because StorageAdapterChangeFeedExtensions publishes strictly POST-COMMIT, so the re-activation's routing read (which goes through storage) sees the new type; recycling off the in-memory commit would race the debounced persist and resurrect the pre-retype node. The feed is also a hot subject with no replay, so a freshly armed watcher structurally cannot fire on the state it was armed on. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…write The change-feed handler runs synchronously on the PUBLISHER's thread -- inside the storage write's post-commit Do -- so a throw from the recycle would propagate into that write and fail an unrelated caller's save. Guard it and log instead, the same best-effort contract ArmOverlaySelfHeal declares. Also records the negative control: with the wiring reverted the two integration tests fail in 120.9s with 'No renderer is registered for area RebindMarkerArea', listing exactly ConfigureDefaultNodeHub's area set; with it restored both pass in 5.0s. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
One subscription per live hub means every published change walks every armed watcher. Say so, and say why that is preferable to a shared path-keyed registry (nothing to keep in step with hub lifetimes, nothing to leak) -- the same trade PathResolutionService.OnMeshChange already documents. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…tivates Both activation sites (MonolithRoutingService.CreateHub and MessageHubGrain) branch on enriched.HubConfiguration == null to activate the fail-fast NACK-fallback hub, whose UnhandledMessageNack answers every message with a typed DeliveryFailure naming the node type and which DeactivateOnIdle's so the next access retries. Wrapping unconditionally made it non-null always, killing that branch and swapping fail-fast for a bare hub that Ignores typed requests -- the park class, where senders wait out their whole budget instead of getting a diagnostic. This is the same rule NodeTypeEnrichmentHelpers.ApplyStreamResult already states for its own wrap. Nothing is lost: a hub with no configuration at all already retries on the next access, so it was never pinned. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Fixes a framework-level routing/activation defect where a per-node hub would permanently keep the HubConfiguration derived from the node’s NodeType at activation time, even if the node’s NodeType later arrives or changes while the hub remains hosted (pinned by address). The fix makes already-hosted hubs “let go” of the wrong NodeType by self-recycling on the first post-commit change-feed event that indicates a NodeType change for their own path.
Changes:
- Introduces
NodeTypeRebindWatcherand wires it intoMeshNodeHubFactory.ResolveHubConfigurationso every instance hub arms a one-shot change-feed watcher that posts a self-DisposeRequestwhen its node’s NodeType changes. - Adds deterministic integration + unit coverage for the rebind/recycle contract (
NodeTypeRebindTest,NodeTypeRebindWatcherTest). - Updates comment ledgers and adds a WhatsNew entry describing the user-visible behavior change.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| test/MeshWeaver.Hosting.Monolith.Test/NodeTypeRebindTest.cs | Integration repro asserting an already-activated hub rebinds after the node’s NodeType arrives/changes. |
| test/MeshWeaver.Graph.Test/NodeTypeRebindWatcherTest.cs | Unit contract tests for the watcher predicate and “post Dispose exactly once” behavior. |
| src/MeshWeaver.PluginCatalog/PackageInstaller.cs | Updates installer commentary to reflect the recycle post is now a fast path, not the guarantee. |
| src/MeshWeaver.Hosting/PathResolutionService.cs | Updates the resolution ledger to document the second-half guarantee being the rebind watcher. |
| src/MeshWeaver.Graph/Configuration/NodeTypeRebindWatcher.cs | Implements the change-feed-driven, one-shot self-recycle watcher. |
| src/MeshWeaver.Graph/Configuration/MeshNodeHubFactory.cs | Hooks rebind arming into the shared activation funnel. |
| src/MeshWeaver.Documentation/Data/WhatsNew/2026-08-10-a-page-picks-up-its-type-without-a-restart.md | WhatsNew entry describing the behavior change for users/operators. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Two more unit tests: a null HubConfiguration passes through untouched (the fail-fast NACK-fallback branch keys on it), and a non-null one is COMPOSED rather than replaced so the type's own areas survive the wrap. 8/8 green. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Contributor
Contributor
Contributor
Test Results (shard 1)1 193 tests - 36 1 193 ✅ - 36 6m 1s ⏱️ -26s Results for commit 716f74f. ± Comparison against base commit 83db1d7. This pull request removes 102 and adds 66 tests. Note that renamed tests count towards both. |
Contributor
Contributor
Contributor
Test Results (shard 2)2 016 tests +46 2 014 ✅ +46 6m 36s ⏱️ +4s Results for commit 716f74f. ± Comparison against base commit 83db1d7. This pull request removes 64 and adds 110 tests. Note that renamed tests count towards both. |
Contributor
rbuergi
added a commit
that referenced
this pull request
Aug 10, 2026
Three conflicts, all in files #1143/#1102 also touched. PathResolutionService + PackageInstaller (the two comment ledgers): merged, not picked. Main's account is the correct spine — #1143 resolved the residual my version posed as open, so "check THREE things … is the live hub the one activated inside the provisioning window?" is deleted in favour of NodeTypeRebindWatcher being the un-pin. What mine contributed and main did not is the FINGERPRINT: the available-areas list being exactly ConfigureDefaultNodeHub's set (AddDefaultLayoutAreas + Invite) and not one area of the node's real type, plus the gate run (31361446933) it was observed on. That is what lets someone recognise this defect instead of re-deriving it, so it is folded into main's paragraphs. PluginGateRunner: main's design wins outright. #1102 deleted the shipped-instance preference — the probe host is ALWAYS a freshly created GateProbe, which binds the just-verified release by construction. My ResolveTestsHost and its two-branch "shipped vs probe" description are obsolete and are gone with it; the branch they described no longer exists. Kept on top of it, because neither is about branch selection: • the host still goes into the report (TestsHost). With one branch it no longer discriminates a cause, but "No renderer … on hub X" is still unreadable without knowing which node X was — that ambiguity is what made the original RED get read as "the probe landed on a type path". • the probe creation is still BOUNDED. CreateNode is a round-trip with no budget of its own; unbounded, a wedge there spends the whole 12-min job timeout and reports nothing. Also drops a stale duplicate <summary> on CreateTestsProbe left by #1102 — the old "Prefers an instance the package ships" text survived above the new one, contradicting it, with two <summary> elements on one member. Verified after the merge: 33/33 tester tests, including all ten GateSummaryTest cases — the two-sided ones (TestsFailure_NamesTests_AndNeverCompile, and the same with the allowlist) confirmed present in the trx, so the misattribution cannot have crept back. mw-plugin-test run end-to-end still emits the CI-facing line: GATE FAILED — fatal: …. Release -warnaserror clean for MeshWeaver.PluginTester (+ Hosting and PluginCatalog, built transitively).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #1104
The defect, pinned
A per-node hub binds its
HubConfigurationfrom its node's NodeType exactly once, duringactivation, and is then pinned by address in
HostedHubsCollection.RoutingServiceBase.RouteInMeshshort-circuits on
Mesh.GetHostedHub(address, Never)for an already-hosted address, so a live hubnever resolves its path again and nothing re-reads its NodeType. A node that acquires — or
changes — its type while its hub is alive keeps serving the configuration it was born with, which
for a type-less node is the mesh
DefaultNodeHubConfiguration.The repro (
NodeTypeRebindTest,test/MeshWeaver.Hosting.Monolith.Test) reproduces the issue'sdecisive evidence deterministically, with neither the installer nor Roslyn: create a node,
activate its hub with
PingRequest, retype the node, render an area the new type declares. With thefix reverted:
— item for item
ConfigureDefaultNodeHub's set, with not one area of the type the node carries.That is the plugin gate's
Store/CatalogRED, in a unit-of-work you can run in five seconds. Bothvariants fail (type-less → typed, and type A → type B), so the pin is general, not install-specific.
Why #1055 did not close it
Not cached is not not-pinned. #1055 stopped
PathResolutionServicefrom caching thefabricated partition-root placeholder, and
PackageInstallerposts aDisposeRequestafter itsretype. Both are necessary; neither is sufficient. Repairing resolution can only help the next
activation — it does nothing for a hub a bad resolution has already activated. And the
installer's recycle is fire-and-forget, conditional on the placeholder dance having run, and
available to no other writer, while any writer can retype a node (an import, a repair migration, a
user).
The design, and the alternatives weighed
The un-pin belongs at the hub, driven by an event:
GetHostedHub. It is address-keyed and knows nothing about nodes; making itNodeType-aware would mean a path resolution per lookup — reinstating exactly the hot-path cost
the
RouteInMeshshort-circuit exists to avoid (Monitor.Enter_Slowpath ← GetHub ← RouteStreamMessage, two prod incidents).post-Fix: a partition root permanently bound to the default hub configuration (plugin-gate Store/Catalog RED) #1055 that is not a cached fabrication. The problem is not the first read; it is that there
is no later activation.
framework without the guarantee.
So:
MeshNodeHubFactory.ResolveHubConfiguration— the single funnel every activation path (Monolithrouting and
MessageHubGrain) goes through — wraps the resolved configuration with aWithInitializationthat armsNodeTypeRebindWatcheron the instance hub. It is the sameWithInitialization+ self-DisposeRequestidiom asNodeTypeEnrichmentHelpers.ArmOverlaySelfHealand
RecycleLayoutArea.The signal is
IMeshChangeFeed, not the hub's own node stream — the load-bearing choice. Bothsee the retype, but only the feed sees it post-commit:
StorageAdapterChangeFeedExtensionspublishes Created/Updated strictly after the storage adapter's write emits. Recycling off the hub's
in-memory commit would race its debounced persist, and since re-activation reads the node back
through routing (i.e. through storage), the fresh hub would resurrect the pre-retype node and
bind the wrong type again — silently, with the stale row now authoritative. That is the hazard
PackageInstaller.RootRetypePersistedalready exists for. Using the feed makes the ordering airtightfor free: it is the very event that sweeps
PathResolutionService's resolution cache, so by the timewe recycle, the resolver the re-activation will consult has already been invalidated.
Blast radius
Disposing a live hub is not free — in-flight subscriptions are torn down and clients re-subscribe —
so the watcher is deliberately narrow:
overwhelming majority) republish the node with its type unchanged and are no-ops; so are writes to
satellites, children and siblings.
Take(1)). After the recycle the new hub's baseline is the new type, so thepredicate cannot re-fire — there is no recycle loop, and a flapping writer cannot make one.
Deletednever fires (the delete path tears the hub down itself and the event carries noauthoritative type); a case-only difference never fires (enrichment resolves type paths
case-insensitively, so it would be churn onto the identical configuration).
on the state it was armed on — the replay-and-recycle hot loop that forced the overlay watcher's
version gate cannot occur here.
stream, no poll, no timer.
Do), so the recycle is wrapped: a fault there could otherwise fail an unrelated caller's save.DisposeRequestis[SystemMessage]/[CanBeIgnored], so it needs noAccessContexton a threadthat carries none.
PackageInstaller'sDisposeRequeststays — it recycles immediately rather than on the feed hop —but it is no longer load-bearing, and its comment ledger now says so.
Verification
NodeTypeRebindTest(2 integration tests), fix revertedNodeTypeRebindTest, fix restoredNodeTypeRebindWatcherTest(8 unit tests, firing contract)MeshWeaver.Graph.Test(full)MeshWeaver.PluginCatalog.Test(full)MeshWeaver.Hosting.Monolith.Test(full)One review catch worth calling out, because it is the kind of wrap that has bitten this codebase
before: the watcher must leave a null
HubConfigurationnull. Both activation sites branch on itto activate the fail-fast NACK-fallback hub (a typed
DeliveryFailurenaming the node type, plusDeactivateOnIdle); wrapping unconditionally would have killed that branch and swapped fail-fast fora bare hub that Ignores typed requests — the park class.
NodeTypeEnrichmentHelpers.ApplyStreamResultstates the same rule for its own wrap. Pinned by
NullHubConfiguration_IsLeftNull.Comment ledgers updated at both sites the issue names:
PathResolutionService.SynthesizePartitionRoot(not pinning a fabrication is one half; letting go of one is the other) and
PackageInstaller(whyits Post is now a fast path and not the guarantee).
🤖 Generated with Claude Code