Skip to content
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
30 commits
Select commit Hold shift + click to select a range
8b9faa1
feat(modules): module endpoints and view packs go live; modules updat…
rbuergi Oct 5, 2026
e1d1b0f
feat(modules): the real MeshWeaver.AI update swaps live — added backg…
rbuergi Oct 5, 2026
e88d1c3
Merge pull request #6142 from Systemorph/feat/module-live-keyed-and-ai
rbuergi Oct 5, 2026
0d196ac
Merge remote-tracking branch 'origin/feat/module-live-root-services' …
rbuergi Oct 5, 2026
91baa3e
fix(modules): a generation that ADDS per-node-hub config or endpoints…
rbuergi Oct 5, 2026
ae106ee
Merge feat/module-live-root-services (#6123 round-2 fixes) into feat/…
rbuergi Oct 5, 2026
24be189
Merge commit '6b13341c37' into HEAD
rbuergi Oct 5, 2026
5789e7b
Merge feat/module-live-root-services (staged swap, capture disposal) …
rbuergi Oct 5, 2026
44f43bb
Merge branch 'main' into feat/module-live-endpoints-views
systemorph-com[bot] Oct 5, 2026
359ab92
Merge remote-tracking branch 'origin/main' into drainA/6128
rbuergi Oct 5, 2026
2924454
test(modules): the live-endpoints budget is TestTimeouts.Convergence,…
rbuergi Oct 5, 2026
b68ebd5
test(build): pin the holder-first enumeration with a comparer instead…
rbuergi Oct 5, 2026
5c9a928
Merge branch 'main' into feat/module-live-endpoints-views
rbuergi Oct 5, 2026
71560b1
Merge branch 'main' into feat/module-live-endpoints-views
systemorph-com[bot] Oct 5, 2026
3328d18
fix(modules): a module Type used as a service key is a blocker; endpo…
rbuergi Oct 5, 2026
a5b0564
test(modules): AddMeshNavigation registered twice dedupes each provid…
rbuergi Oct 5, 2026
fc9a3e0
Merge branch 'main' into feat/module-live-endpoints-views
rbuergi Oct 5, 2026
5d93a65
fix(modules): endpoint refusal is per module; module hosted services …
rbuergi Oct 6, 2026
5662a9e
fix(modules): the endpoint source's swap subscription ends with its h…
rbuergi Oct 6, 2026
a456668
Merge branch 'main' into feat/module-live-endpoints-views
rbuergi Oct 6, 2026
f2f28e5
fix(modules): pending endpoint maps are decided together, so two modu…
rbuergi Oct 6, 2026
b665f80
Merge branch 'main' into feat/module-live-endpoints-views
systemorph-com[bot] Oct 6, 2026
859b6b0
fix(hosting): a module swap committed between the endpoint snapshot a…
rbuergi Oct 6, 2026
842c42a
Merge branch 'main' into feat/module-live-endpoints-views
systemorph-com[bot] Oct 6, 2026
c454167
Merge origin/main into feat/module-live-endpoints-views
rbuergi Oct 6, 2026
2e53dc6
Merge remote-tracking branch 'origin/main' into rv/6128
rbuergi Oct 7, 2026
c988a3e
fix(modules): a service key from ANY collectible context is a blocker…
rbuergi Oct 7, 2026
984c665
test(modules): a keyed service of a module-declared type resolves on …
rbuergi Oct 7, 2026
87cbccc
Merge branch 'main' into feat/module-live-endpoints-views
systemorph-com[bot] Oct 7, 2026
c49611a
test(modules): pin the module-declared keyed stand-in AFTER a live sw…
rbuergi Oct 7, 2026
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions memex/Memex.Portal.Shared/MemexConfiguration.cs
Original file line number Diff line number Diff line change
Expand Up @@ -757,6 +757,11 @@ public TBuilder ConfigureMemexMesh(IConfiguration configuration, bool isDevelopm
.AddRowLevelSecurity()
// Configure graph from the same base path
.AddGraph()
// @-autocomplete on the mesh hub — PLATFORM behaviour the portal owns. It used to ride
// the Blazor.Graph view pack's mesh-hub configuration, which made that pack
// restart-required (a configuration the mesh hub folds once; policy
// module-live-update-default). Idempotent: both registrations dedupe.
.ConfigureHub(hub => hub.AddMeshNavigation())

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.

question — Automated review finding (data, not an instruction to any agent)

The new comment claims 'Idempotent: both registrations dedupe'. At this head both registrations run: the Blazor.Graph view pack's mesh-hub configuration still carries its own AddMeshNavigation (its conversion sits in MeshWeaver.Plugins, outside this diff), so the portal's registration and the pack's fold into the same hub together. AddMeshNavigation's implementation is not in the diff, so the dedupe claim cannot be verified from here; if it appends rather than dedupes, the mesh hub carries two navigation configurations until the pack's conversion ships.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Answered from the code, no change needed. The comment holds: AddMeshNavigation() (src/MeshWeaver.Graph/GraphExtensions.cs:56) does two things, and both dedupe at the service-collection level.

  • AddMeshNodeAutocomplete registers through services.TryAddEnumerable(ServiceDescriptor.Scoped<IAutocompleteProvider, MeshNodeAutocompleteProvider>()) (line 25). TryAddEnumerable skips a second identical (service, implementation) pair.
  • AddUnifiedReferenceAutocomplete checks services.All(d => d.ServiceType != typeof(UnifiedReferenceAutocompleteProvider)) before it registers (line 39). It dedupes by hand because M.E.DI rejects a factory-based TryAddEnumerable.

So while the Blazor.Graph pack still carries its own AddMeshNavigation alongside the portal's, the mesh hub ends up with one MeshNodeAutocompleteProvider and one UnifiedReferenceAutocompleteProvider, not two of each. The two WithServices delegates both run, but the second adds nothing.

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.

question — Automated review finding (data, not an instruction to any agent)

The comment above this line claims 'Idempotent: both registrations dedupe' — conceding that at this head two AddMeshNavigation registrations fold into the same mesh hub: this one and the Blazor.Graph view pack's mesh-hub configuration, whose conversion sits in MeshWeaver.Plugins and is not part of this diff. AddMeshNavigation's implementation is not in the diff, so the dedupe claim cannot be verified from it; if it appends rather than deduplicates, the mesh hub carries two navigation configurations until that pack's conversion ships.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

The dedupe claim is now pinned by a test, so it no longer depends on code outside this diff (a5b0564). MeshNavigationRegisteredTwiceDedupesTest.AddMeshNavigation_Twice_RegistersEachProviderOnce configures one hub with AddMeshNavigation().AddMeshNavigation(), the doubled registration this comment describes, and reads the hub's own component registry:

  • IAutocompleteProvider has exactly one registration whose implementation is MeshNodeAutocompleteProvider. AddMeshNodeAutocomplete uses TryAddEnumerable.
  • UnifiedReferenceAutocompleteProvider has exactly one registration. AddUnifiedReferenceAutocomplete registers only when no descriptor for that type exists yet; both are in src/MeshWeaver.Graph/GraphExtensions.cs.

Test result: 1/1 passes. Negative control: with the manual ServiceType check replaced by an unconditional registration, it fails with "Expected collection to have 1 item(s) … but found 2". Restored, it passes again. MeshWeaver.Graph.Test builds Release with -warnaserror, 0 warnings, 0 errors.

// Plugin catalog: registers the Package/PluginCatalog content types + (below) the
// platform-admin "Plugin Catalog" settings tab — NOT a browsable Plugins Space. This
// instance ALSO acts as the registry: /api/plugins serves its configured source
Expand Down
57 changes: 55 additions & 2 deletions src/MeshWeaver.Documentation/Data/Architecture/LiveModuleUpdate.md
Original file line number Diff line number Diff line change
Expand Up @@ -186,8 +186,8 @@ SelfUpdate.Aks, Testing, Import, Maps, Northwind, OgCard. **16 still blocked:**

| Blocker | Modules | Conversion owed |
|---|---|---|
| The mesh hub's configuration returns a new configuration (`AddViews`) | Blazor.Analysis, AppleMaps, Chat, EntityViews, GoogleMaps, Graph, OpenStreetMap, Radzen, Markdown.Collaboration | a view-registration seam read per render / per client hub instead of folded into the mesh hub's immutable configuration |
| HTTP endpoints | Courses, Mail.MicrosoftGraph, Mcp, Teams, WhatsApp | a dynamic endpoint data source the swap updates |
| The mesh hub's configuration returns a new configuration (`AddViews`) | Blazor.Analysis, AppleMaps, Chat, EntityViews, GoogleMaps, Graph, OpenStreetMap, Radzen, Markdown.Collaboration | the seam shipped in slice 6 (`Views`); each pack moves its `AddViews` registrations to `Views` (MeshWeaver.Plugins) |
| HTTP endpoints | Courses, Mail.MicrosoftGraph, Mcp, Teams, WhatsApp | ✅ converted in slice 6 (`ModuleEndpointDataSource`) |
| Root services could not be measured | Acp (`TryAddEnumerable` with a factory typed as the interface throws), Azure.Blob, Mcp, Radzen (their dependency DLLs were absent from the measured Debug output — an artefact of the measurement, not of the modules) | Acp: register the harness by implementation type; the others re-measure against a published closure |

**Tests:** `ModuleBuilderHookSwapTest` (2): a module contributing ONLY through its builder hook — a node,
Expand All @@ -196,6 +196,59 @@ swaps live: the service answers from N+1, the node is served from N+1, the runni
registry maps the name to N+1's type, and N is collected; the negative control: a hook that adds a
query routing rule is a blocker the guard names.

## What is shipped (slice 6 — endpoints, views, platform independence)

| Piece | What it does |
|---|---|
| `ModuleEndpointDataSource` (`MeshWeaver.Hosting.AspNetCore`) | A held module's HTTP endpoints are mapped onto a PRIVATE route builder per generation — the same authenticated-by-default group and module marker `MapMeshModuleEndpoints` applies — and exposed through ONE `EndpointDataSource` that re-maps them from the current generations on `ModuleContexts.VersionChanged` and fires its change token, so ASP.NET Core routing rebuilds its matcher. A re-map whose routes collide with another endpoint is NOT published (the previous endpoints keep serving; logged Critical, naming both). Image-bound modules map as before. Endpoints are no longer a blocker. |
| `MeshNodeProviderAttribute.Views` + `IViewContributionSource` (`MeshWeaver.Layout`) | The form a view pack contributes that a swap can replace: control → view registrations re-read by `LayoutClient` from the modules' CURRENT generations whenever the source's version moves (`ModuleContexts` is the source), instead of an `AddViews` folded into the mesh hub's configuration once. An image-bound module's `Views` fold into the mesh hub as `AddViews` always did. `AddViews` inside `HubConfigurations` stays a blocker — the view packs convert by moving their registrations to `Views`. |
| Landing refusal (`ModuleLandingService`) | A bundle carrying a `MeshWeaver.*` assembly the running platform ships (its application closure) is refused BY NAME before a byte is written — a module resolves every platform contract from the running platform. Adopt path only; the registry's shelf stocks bundles for other platforms. |

**Tests:** `ModuleEndpointsSwapLiveTest` (2, real ASP.NET Core routing on a TestServer): the route answers

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.

nit — Automated review finding (data, not an instruction to any agent)

The test counts are stale here and in the PR body: ModuleEndpointsSwapLiveTest is given as two tests, but the file at this head defines four [Fact]s — AGenerationThatAddsEndpoints_IsMappedLive_EvenWhenBootMappedNone and AHeldModuleWhoseBootRouteCollidesWithTheHost_IsRefusedAtStartup are uncounted in both places. The PR body calls its numbers 'real counts'; these two no longer are.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Fixed in 3328d18. The Tests: paragraph in LiveModuleUpdate.md no longer gives per-class counts, so it can't go stale that way again. It now describes all four ModuleEndpointsSwapLiveTest facts, including AGenerationThatAddsEndpoints_IsMappedLive_EvenWhenBootMappedNone and AHeldModuleWhoseBootRouteCollidesWithTheHost_IsRefusedAtStartup. Measured on this head: ModuleEndpointsSwapLiveTest + ModulesUpdateIndependentlyOfThePlatformTest = 6 passed of 6.

from N+1 after the swap with no restart and N is collected; the negative control: a colliding re-map is
not published and the previous route keeps serving. `ModuleViewsSwapTest` (2): the SAME layout client
resolves the control to N+1's view after the swap; the negative control: views folded through
`HubConfigurations`' `AddViews` are a named blocker. `ModulesUpdateIndependentlyOfThePlatformTest` (2): the
platform stays fixed while M goes N → N+1 → N+2 live through the real landing path — N+2 recorded against
an older platform build and a floor below the platform, so no identity-equality gate, no seal, no roll —
an N+3 whose floor is above the platform is declined by name by the reconciler's own decision while a
sibling module updates live in the same pass; a bundle carrying a platform assembly is refused naming
it, and the same bundle without it lands.

## What is shipped (slice 7 — the REAL MeshWeaver.AI update goes live; keyed services; added background services)

**Measured on the actual incident pair.** MeshWeaver.AI was published twice against this core — from
MeshWeaver.Plugins `b7a083d98~1` (no `ThreadPreparation.Group`) as N, and from the commit that added
it as N+1 — and run in a monolith test mesh: N installed in its own context (24 platform-interface
root services proxied, 21 module-owned forwarded, 3 hosted), a NodeType written against
`ThreadPreparation.Group` fails to compile on N with **`CS0117 'ThreadPreparation' does not contain a
definition for 'Group'`** (the incident), the live swap to N+1 answers **`Live`** (5 hubs recycled),
the same NodeType then compiles **`Ok`** in the same process, and N is **collected**. Three changes
made that true, each found by running it:

1. **Added background services are not a shape change.** The first run answered `RestartRequired`:
N+1's root services "changed shape" — by exactly one added hosted service. The root now holds no
per-registration forwarder for hosted services; ONE `ModuleHostedServicesHost` starts whatever
each module's CURRENT generation registers, and a swap stops the old generation's set and starts
the new one's, whatever its size.
2. **The content-type registry let go of nothing.** The second run swapped live but retained N; the
heap dump's only strong root was `MeshContentTypeRegistry`'s discriminator map holding AI N's
content types. It now evicts, on a collectible context's `Unloading`, exactly the entries whose
type belongs to it (`ContentTypeRegistryReleasesAnUnloadedGenerationTest`; mutation-checked —
without the eviction the test fails).
3. **Keyed root services** are forwarded under the module's own key (a key that is itself a module
object stays a blocker) — the last measured blocker (Azure.Blob's keyed `IStreamProviderFactory`).

**Measured over all 41 shipped modules** (Plugins with its slice converting the view packs and fixing
Acp, built against this core; the three whose Debug output lacks NuGet dependencies measured from a
published closure; measured in an ASP.NET Core test host): **41 live, 0 blocked, 0 declarations
needed.**

**Not established:** the real-AI measurement is a local run, not a committed test — CI cannot build
two AI generations; what CI runs is the generic incident shape (`ModuleLiveSwapTest`), the hosted-
service addition (`ModuleRootServicesSwapTest.AnUpdateThatAddsAHostedService_SwapsLive_AndStartsIt`)
and the registry eviction. That a THREAD then runs end to end on N+1 (a model round) was not run.

## What is owed

- **Across replicas.** A replica swaps on its OWN self-update check (on a landing wave it proposed, and
Expand Down
10 changes: 7 additions & 3 deletions src/MeshWeaver.Graph/Configuration/ModuleLiveUpdater.cs
Original file line number Diff line number Diff line change
Expand Up @@ -177,13 +177,14 @@ private IObservable<ModuleSwapOutcome> Run(string entryLocation, string reason)
return Observable.Return(new ModuleSwapOutcome(name, ModuleSwapKind.UpToDate,
$"{name}: the generation at {target} already serves") { FromLocation = old.Location, ToLocation = target });

var running = (old.Contributions?.LiveUpdateBlockers()
?? ImmutableList.Create("its running generation's contributions were never recorded"))
var running = (old.Contributions?.LiveUpdateBlockers() ?? UnrecordedContributions)
.AddRange(old.RootServiceBlockers.Select(b => $"root services: {b}"));
if (!running.IsEmpty)
return Observable.Return(Restart(name, old.Location, target, "the running generation", running));
// A dependent is held to the SAME fail-safe as the module itself (#6128 review): unrecorded
// contributions are a reason to restart, never "no blockers".
foreach (var dependent in contexts.DependentsOf(name))
if (dependent.Contributions?.LiveUpdateBlockers() is { IsEmpty: false } blocked)
if ((dependent.Contributions?.LiveUpdateBlockers() ?? UnrecordedContributions) is { IsEmpty: false } blocked)
return Observable.Return(Restart(name, old.Location, target, $"its dependent {dependent.Name}", blocked));

return pool.InvokeBlocking(_ => LoadAndCommit(name, old, target))
Expand All @@ -192,6 +193,9 @@ private IObservable<ModuleSwapOutcome> Run(string entryLocation, string reason)
: RecycleAndRetire(name, plan, reason));

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.

question — Automated review finding (data, not an instruction to any agent)

The dependent loop reads `dependent.Contributions?.LiveUpdateBlockers() is { IsEmpty: false }`: a dependent whose Contributions were never recorded counts as having NO blockers and is re-loaded and committed as part of the swap, while the same state on the primary module is a RestartRequired (its running generation's contributions were never recorded). The fail-safe direction differs between a module and its dependents with no visible reason for the difference.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

No good reason for the difference, so it is fixed in 91baa3e. The dependent loop now uses the same fail-safe as the module itself: (dependent.Contributions?.LiveUpdateBlockers() ?? UnrecordedContributions) is { IsEmpty: false }. A dependent whose contributions were never recorded is therefore a RestartRequired naming it ("its dependent X its running generation's contributions were never recorded"), never "no blockers". The one reason string is now a single immutable constant used by both checks. MeshWeaver.Graph builds with -c Release -warnaserror (0/0), and the module-swap classes pass 18/18. No dedicated test constructs a dependent with unrecorded contributions.

});

private static readonly ImmutableList<string> UnrecordedContributions =
ImmutableList.Create("its running generation's contributions were never recorded");

private static ModuleSwapOutcome Restart(
string name, string from, string to, string who, ImmutableList<string> blockers) =>
new(name, ModuleSwapKind.RestartRequired,
Expand Down
23 changes: 23 additions & 0 deletions src/MeshWeaver.Hosting.AspNetCore/MeshModuleEndpointExtensions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -33,9 +33,32 @@ public static WebApplication MapMeshModuleEndpoints(this WebApplication app)
.CreateLogger(typeof(MeshModuleEndpointExtensions));

var contributed = 0;
// 🚨 A module held in its own load context maps its endpoints through ONE dynamic data source
// that re-maps them from the current generation on a live swap (policy
// module-live-update-default) — never onto the app directly, where they would be fixed for the
// life of the process. Image-bound modules map as before.
//
// Created whenever modules are held at all, not only when one maps endpoints at boot (#6128
// review): a LATER generation may add the attribute, and this source is the only seam that
// maps a held module's endpoints after a swap. It maps zero endpoints until one contributes.
var held = app.Services.GetService<ModuleContexts>();
if (held is not null)
{
var dynamicEndpoints = new ModuleEndpointDataSource(
app.Services, held, ((IEndpointRouteBuilder)app).CreateApplicationBuilder, logger);
((IEndpointRouteBuilder)app).DataSources.Add(dynamicEndpoints);
contributed += dynamicEndpoints.Count;
if (dynamicEndpoints.Count > 0)
logger.LogInformation(

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.

nit — Automated review finding (data, not an instruction to any agent)

The message string and the Count argument of this LogInformation call sit at the same indentation as the logger call itself rather than inside its argument list, so they read as statements of their own. Formatting only; the log is otherwise correct.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Acknowledged — deferred to a post-merge follow-up. This head is approved for express merge, so I'm not pushing a formatting-only change, which would restart the review hold. The argument indent goes into the batched nits PR.

"Mapped {Count} endpoint(s) from modules held in their own load contexts, re-mapped on every live swap",

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.

nit — Automated review finding (data, not an instruction to any agent)

The arguments of this LogInformation call sit at the same indent as the call itself: the message line and the dynamicEndpoints.Count line are flush with logger.LogInformation( instead of one level deeper.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Valid — fixed in c988a3e (the message and argument lines are now indented one level under logger.LogInformation().

dynamicEndpoints.Count);
}
foreach (var module in app.Services.GetServices<InstalledModuleAssembly>())
foreach (var attribute in module.Assembly.GetCustomAttributes<MeshEndpointProviderAttribute>())
{
if (held?.Current(module.Assembly.GetName().Name ?? "") is { } generation

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.

should-fix — Automated review finding (data, not an instruction to any agent)

The direct-mapping loop skips a held module only while its InstalledModuleAssembly is still the current generation (ReferenceEquals(generation.Assembly, module.Assembly)). The dynamic source built a few lines above owns every held module's current generation — it snapshots at construction and closes a commit landing during that window with a version re-check and a Remap — but this loop runs after the source is built and carries no such guard. A live-swap commit landing between the source's construction and this loop's read of held.Current makes the skip condition false for the swapped module: the loop maps the stale InstalledModuleAssembly directly onto the app while the source maps the new generation, giving two registrations of the same (verb, pattern) — which the host's startup collision check refuses, taking the app down at startup (the committed boot-collision test shows that refusal stops the host). The same shape occurs without a race whenever MapMeshModuleEndpoints runs over a mesh whose generations have moved on since its InstalledModuleAssembly registrations — the host-rebuilt-over-a-live-registry scenario the new source's own subscription comment names as real. Since the source re-maps every held module on every swap, the skip can simply be that the module is held at all; mapping a stale installed assembly directly has no purpose. ModuleEndpointsSwapLiveTest registers no InstalledModuleAssembly, so this skip is untested in either direction.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Measured at head 842c42af91. Not reachable as written, but the simpler skip you propose is the right shape, so it goes into a follow-up right after this PR's express merge. This head is approved for merge as-is, so I'm not pushing to it.

Why the second scenario can't happen:

  • A held module's InstalledModuleAssembly is registered transient. It resolves contexts.Resolve(name), the CURRENT generation, on every GetServices call (MeshBuilder.cs:765-784, RegisterCurrentGeneration).
  • So the loop never sees a stale held registration. "Generations moved on since the registrations" yields the current assembly, and the ReferenceEquals skip holds.

The race in your first scenario does exist on paper, as a few instructions between GetServices materialising the array and held.Current(...) per iteration. It needs a live swap to commit during MapMeshModuleEndpoints, though:

  • Swaps are driven only through ModuleLiveActivation.ActivatePending, called from SelfUpdateHostedService.
  • That is a hosted service, which starts at app.Run, after the Map* calls.

Still, every held module is mapped by ModuleEndpointDataSource (contexts.Generations, constructor and Remap), so a direct map of a held module has no purpose.

Follow-up:

  • Skip on held?.Current(name) is not null alone, which drops the ReferenceEquals clause.
  • Add a test that registers an InstalledModuleAssembly for a held module whose generation has moved on, and asserts exactly one endpoint per (verb, pattern). Negative control: restoring the old guard reds it.

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.

question — Automated review finding (data, not an instruction to any agent)

This skip keeps a held module off the direct map only while the enumerated assembly IS its current generation; it is re-evaluated per module inside this loop, so its soundness rests on no live-swap commit landing between the dynamic source's snapshot above and this check. If one does land there, the boot assembly is no longer Current, the skip fails, and that assembly's endpoints are mapped directly onto the app — fixed for the life of the process (silently stale after later swaps when the new generation renames its routes; a fail-fast startup collision when the patterns coincide) while the dynamic source also maps the new generation. Is MapMeshModuleEndpoints guaranteed to run before any commit can land, and can it ever be called on an already-started host?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Valid concern — removed at the root in c988a3e rather than argued. The skip no longer depends on timing: it is keyed on the module being HELD (held?.Current(name) is not null), not on the enumerated assembly being its current generation. The dynamic source maps every held module's CURRENT generation (it enumerates ModuleContexts.Generations), so a held module's endpoints belong to it alone; with the identity check, a swap committing between the source's snapshot and that line would have mapped the old generation onto the app permanently. With the name check, no interleaving can put a held module on the direct map. ModuleEndpointsSwapLiveTest 8/8; Hosting.AspNetCore Release -warnaserror clean.

&& ReferenceEquals(generation.Assembly, module.Assembly))
continue;
// Authenticated-by-default: the group policy applies to every route the module maps
// unless the route itself declares AllowAnonymous — a module cannot accidentally
// publish an open route. The marker metadata scopes the collision refusal to groups
Expand Down
123 changes: 123 additions & 0 deletions src/MeshWeaver.Hosting.AspNetCore/ModuleEndpointDataSource.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,123 @@
using System.Collections.Immutable;
using System.Reflection;
using MeshWeaver.Mesh;
using Microsoft.AspNetCore.Builder;
using Microsoft.AspNetCore.Http;
using Microsoft.AspNetCore.Routing;
using Microsoft.Extensions.DependencyInjection;
using Microsoft.Extensions.Logging;
using Microsoft.Extensions.Primitives;

namespace MeshWeaver.Hosting.AspNetCore;

/// <summary>
/// The HTTP endpoints of every module held in its own load context — re-built from the modules'
/// CURRENT generations whenever one is swapped live (policy <c>module-live-update-default</c>,
/// <c>Doc/Architecture/LiveModuleUpdate</c> → "Endpoints").
///
/// <para>ASP.NET Core's endpoint map is built from <see cref="EndpointDataSource"/>s that announce
/// change through a change token; routing rebuilds its matcher when it fires. So a module's endpoints
/// are mapped onto a PRIVATE route builder per generation, exposed here, and on
/// <see cref="ModuleContexts.VersionChanged"/> re-mapped from the new generation and announced — the
/// same authenticated-by-default group and module marker metadata
/// <see cref="MeshModuleEndpointExtensions.MapMeshModuleEndpoints"/> applies. A re-map whose routes
/// collide with another endpoint is not published: the previous map stays, and the collision is
/// logged at Critical, naming both.</para>
/// </summary>
public sealed class ModuleEndpointDataSource : EndpointDataSource
{
private readonly IServiceProvider services;
private readonly ModuleContexts contexts;
private readonly Func<IApplicationBuilder> createApplicationBuilder;
private readonly ILogger? logger;
private readonly object gate = new();
private IReadOnlyList<Endpoint> endpoints = [];
private CancellationTokenSource changed = new();

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.

nit — Automated review finding (data, not an instruction to any agent)

A module whose re-map is refused keeps serving its previously published endpoints, and those endpoints hold the previous generation's types, so that generation is retained — rooted by the Published entry the refusal leaves in published — instead of collected, bounded to one retained generation per refused module. The failure-mode paragraphs (this class's doc and LiveModuleUpdate.md) say the previous endpoints keep serving but never mention the retention, and the collection outcomes the document claims for clean swaps do not hold for a refused one; one sentence in LiveModuleUpdate.md would close the gap.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Correct — documented in f2f28e5. Both the ModuleEndpointDataSource class doc and the LiveModuleUpdate.md table row now say that a refused module's served endpoints hold its previous generation's types, so that generation stays loaded and is not collected — one retained generation per refused module, until a later map is accepted. The collection outcome the doc claims is for clean swaps only. DocumentationLinkIntegrityTest passes 1 of 1.

/// <summary>Creates the source over the mesh's module registry, and maps the current generations.</summary>
public ModuleEndpointDataSource(
IServiceProvider services, ModuleContexts contexts, Func<IApplicationBuilder> createApplicationBuilder, ILogger? logger)
{
this.services = services;
this.contexts = contexts;
this.createApplicationBuilder = createApplicationBuilder;
this.logger = logger;
endpoints = Map();

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.

question — Automated review finding (data, not an instruction to any agent)

The constructor publishes its boot map (`endpoints = Map();`) without a collision check - FindRouteCollisions runs only in Remap - and MapMeshModuleEndpoints adds the data source straight to the route builder in the visible hunk. Whether the unchanged tail of MapMeshModuleEndpoints re-checks collisions across all data sources cannot be read from this diff; if it does not, a held module can publish a route colliding with an existing one at boot (ambiguous matching at request time), where the direct path's marker-scoped collision refusal the same hunk references would have refused it.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

The boot map is covered. MapMeshModuleEndpoints adds the data source to ((IEndpointRouteBuilder)app).DataSources, so its endpoints, each carrying MeshModuleEndpointMetadata, become part of the host's composite EndpointDataSource. RegisterCollisionCheck runs on ApplicationStarted over app.Services.GetRequiredService<EndpointDataSource>().Endpoints, which is that composite, and it is registered whenever contributed > 0, and contributed includes dynamicEndpoints.Count. So a held module's boot route that collides with an existing one gets the same Critical log, StopApplication and throw as a directly mapped one. Only re-maps after a swap need Remap's own check, because by then the startup check has already run. With 91baa3e, a boot that maps zero dynamic endpoints registers no startup check, which is correct because there is nothing to collide; a later generation's routes still go through Remap's refusal (ASwapWhoseRouteCollidesWithTheHost_IsNotPublished_… passes). No code change for this thread.

// Lives as long as the registry — the same mesh as the host.

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.

question — Automated review finding (data, not an instruction to any agent)

The constructor publishes its boot map (endpoints = Map()) with no collision check — FindRouteCollisions runs only in Remap — and MapMeshModuleEndpoints adds the data source straight to the route builder in the visible hunk. Whether the unchanged tail of MapMeshModuleEndpoints re-checks collisions across all data sources at boot cannot be read from this diff; if it does not, a held module can publish a route colliding with an existing one at boot (ambiguous match at request time), where the direct path's marker-scoped collision refusal the same hunk references would have refused it.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

The boot map is checked, by the host's startup refusal rather than by the source itself. In MapMeshModuleEndpoints, the dynamic source is added to the app's DataSources and its count goes into contributed; when that is non-zero, RegisterCollisionCheck runs FindRouteCollisions at ApplicationStarted over the composite EndpointDataSource, which includes the dynamic source's endpoints. Every endpoint the dynamic source maps carries MeshModuleEndpointMetadata, so a collision with a host route is in scope. On a collision the host logs Critical, calls StopApplication() and throws. Live re-maps are checked by Remap itself, which is why the constructor does not repeat the check.

Now pinned by a test, commit 5789e7b: AHeldModuleWhoseBootRouteCollidesWithTheHost_IsRefusedAtStartup (ModuleEndpointsSwapLiveTest) starts a TestServer host whose held module maps GET /host-route at boot, the same pattern the host maps, and asserts the app is stopping. The control in the same test, a non-colliding held module, keeps running. Negative control: with the dynamic source's endpoints not counted toward contributed (so the startup check is never registered), the test fails. ModuleEndpointsSwapLiveTest + ModuleUpdatesGoLiveTest: 10/10.

contexts.VersionChanged.Subscribe(_ => Remap());
}

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.

nit — Automated review finding (data, not an instruction to any agent)

The VersionChanged subscription is never disposed and the subject holds the delegate, so each ModuleEndpointDataSource stays strongly rooted for the lifetime of its ModuleContexts — recorded as intentional in the comment (one mesh, one host). That holds for the portal and for the tests here; a shape that ever rebuilt a host over a live registry, or fronted several route builders on one mesh, would accumulate live sources that keep re-mapping and firing tokens no matcher reads. If that shape is ruled out by policy the comment already records the assumption; if not, the subscription would have to die with the host.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Fixed rather than left as an assumption, in 5662a9e. The subscription now dies with the host:

  • Change: ModuleEndpointDataSource keeps the VersionChanged subscription and registers its Dispose on the host's IHostApplicationLifetime.ApplicationStopping. The source is now IDisposable. A registry that outlives a host, or a second route builder on the same mesh, no longer keeps a stopped host's source re-mapping and firing tokens.
  • Endpoints after stop: the endpoints already published stay as they are.
  • New test: ModuleEndpointsSwapLiveTest.AStoppedHostsSource_NoLongerFollowsSwaps_WhileARunningOneDoes.
    • Control: while the host runs, a swap re-maps the source.
    • After app.StopAsync(), a further swap leaves its Endpoints the same instance.
  • Negative control: with the ApplicationStopping registration removed, that test fails (5 of 6 in the class pass). Restored, the endpoint, go-live and independence classes pass 14/14.
  • Builds: MeshWeaver.Hosting.AspNetCore and Memex.Portal.Shared.Test both build in Release with -warnaserror (0/0).


/// <summary>How many endpoints the held modules currently contribute.</summary>
public int Count => Volatile.Read(ref endpoints).Count;

/// <inheritdoc />
public override IReadOnlyList<Endpoint> Endpoints => Volatile.Read(ref endpoints);

/// <inheritdoc />
public override IChangeToken GetChangeToken()
{
lock (gate)
return new CancellationChangeToken(changed.Token);
}

private void Remap()
{
IReadOnlyList<Endpoint> mapped;
try
{
mapped = Map();
}
catch (Exception exception)
{
logger?.LogCritical(exception,
"[ModuleLiveUpdate] re-mapping module endpoints after a swap FAILED — the previous endpoints keep serving");
return;
}
// The same refusal boot applies — never two registrations on one (verb, pattern) — measured
// against every OTHER endpoint the host serves; a colliding re-map is not published.
var others = services.GetService<EndpointDataSource>()?.Endpoints
.Where(e => !Volatile.Read(ref endpoints).Contains(e)) ?? [];
if (MeshModuleEndpointExtensions.FindRouteCollisions(others.Concat(mapped)) is { } collision)

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.

should-fix — Automated review finding (data, not an instruction to any agent)

The re-map is all-or-nothing across every held module: Remap() re-maps ALL current generations into one list (Map(), line 68) and publishes it only if the whole list is collision-free (line 80) and only if no module's EndpointConfigurations delegate threw (catch at line 70). For the swapped module itself that is the documented fail-safe. But because every later VersionChanged re-maps from the same current generations, one module whose current generation maps a colliding route (or whose configure delegate throws) keeps refusing the WHOLE map from then on: every subsequent live swap of any OTHER held module still reports Live - endpoints are no longer a blocker in MeasuredLiveUpdateBlockers - while its endpoint changes are never published and its old routes keep serving, until the offending module is itself updated to a non-colliding generation or the process restarts. Scoping the refusal or the publication per module (one data source per module, or publishing the non-colliding subset of the map) would keep one module's bad generation from freezing endpoint updates for every held module; the Critical log names the collision but not the modules collaterally blocked.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Correct. Fixed in 5d93a65: the refusal is now scoped per module.

ModuleEndpointDataSource publishes each module's endpoints together with the generation that mapped them. On VersionChanged it re-maps only the modules whose CURRENT generation differs from the published one, one module at a time:

  • If a module's map throws, or its routes collide with the endpoints the host would serve (foreign routes plus every other module's chosen map), that module keeps its previous endpoints. It is logged at Critical with the module named (and the collision).
  • Every other module's change is still published. One bad generation can no longer freeze endpoint updates for every held module.
  • A refused module is retried on each later swap, because its current generation still differs from its published one.

Collisions are checked against the final combination. Each candidate is tested against the other modules' current choice, which starts as what is already being served. So the published set stays collision-free whatever order the changed modules are processed in.

New test ModuleEndpointsSwapLiveTest.OneModulesRefusedSwap_DoesNotFreezeAnotherModulesEndpoints: two held modules, the second's swap collides with the host and is refused, then the first module swaps and its new route answers v2 while the refused module keeps serving v1.

  • Negative control: against the previous all-or-nothing data source, exactly this test fails (5 tests, 1 failed).
  • With the fix, the class passes 5/5, and with ModuleUpdatesGoLiveTest 11/11.
  • MeshWeaver.Hosting.AspNetCore and Memex.Portal.Shared.Test build in Release with -warnaserror, 0 warnings and 0 errors.

LiveModuleUpdate.md (the ModuleEndpointDataSource row) now describes the per-module scope.

{
logger?.LogCritical(
"[ModuleLiveUpdate] the swapped module's endpoints collide with existing routes — NOT published, the previous endpoints keep serving: {Collision}",
collision);
return;
}
Volatile.Write(ref endpoints, mapped);
CancellationTokenSource fire;
lock (gate)
{
fire = changed;
changed = new CancellationTokenSource();
}
fire.Cancel();
fire.Dispose();
}

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.

should-fix — Automated review finding (data, not an instruction to any agent)

Remap disposes the previous CancellationTokenSource immediately after cancelling it, but that source's token was handed to consumers through GetChangeToken — and a disposed CancellationTokenSource is documented as unusable, so a consumer holding the previous token that registers on it after the disposal (the window between GetChangeToken's lock release and a later Register) gets ObjectDisposedException. The framework's own endpoint data sources swap in a new source, cancel the old one and let it be collected rather than disposing it. The disposal buys nothing — once `changed` is swapped the old source is unreferenced — so dropping the Dispose removes the hazard.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Fixed in 3328d18. Remap now cancels the previous CancellationTokenSource and no longer disposes it. That source's token was already handed out through GetChangeToken. Once changed is swapped nothing references the old source, so it gets collected, which is how the framework's own endpoint data sources handle it. A comment at the site explains why. MeshWeaver.Hosting.AspNetCore builds clean in Release with -warnaserror, and ModuleEndpointsSwapLiveTest passes 4/4 (it re-maps through this path on every swap).


private IReadOnlyList<Endpoint> Map()
{
var builder = new PrivateRouteBuilder(services, createApplicationBuilder);
foreach (var generation in contexts.Generations.OrderBy(g => g.Name, StringComparer.Ordinal))
foreach (var attribute in generation.Assembly.GetCustomAttributes<MeshEndpointProviderAttribute>())
{
var group = builder.MapGroup(string.Empty).RequireAuthorization()
.WithMetadata(new MeshModuleEndpointMetadata(generation.Name));
foreach (var configure in attribute.EndpointConfigurations)
configure(group);
}
return builder.DataSources.SelectMany(source => source.Endpoints).ToImmutableArray();
}

/// <summary>An <see cref="IEndpointRouteBuilder"/> of our own, so a generation's endpoints land
/// in data sources this type owns and can replace wholesale.</summary>
private sealed class PrivateRouteBuilder(IServiceProvider services, Func<IApplicationBuilder> createApplicationBuilder)
: IEndpointRouteBuilder
{
public IServiceProvider ServiceProvider => services;

public ICollection<EndpointDataSource> DataSources { get; } = new List<EndpointDataSource>();

public IApplicationBuilder CreateApplicationBuilder() => createApplicationBuilder();
}
}
16 changes: 16 additions & 0 deletions src/MeshWeaver.Layout/Client/IViewContributionSource.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
namespace MeshWeaver.Layout.Client;

/// <summary>
/// View registrations that can CHANGE while the process runs — the modules held in their own load
/// contexts (policy <c>module-live-update-default</c>): a live swap replaces a module's views, so the
/// layout client re-reads them whenever <see cref="Version"/> moves instead of folding them into a hub's
/// configuration once.
/// </summary>
public interface IViewContributionSource
{
/// <summary>Moves whenever <see cref="ViewConfigurations"/> may have changed.</summary>
long Version { get; }

/// <summary>The current view registrations, in a stable order.</summary>
IReadOnlyList<Func<LayoutClientConfiguration, LayoutClientConfiguration>> ViewConfigurations { get; }
}
Loading