From 87a5be03ae7c3376f2fe9419c6288aa70698dd06 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Roland=20B=C3=BCrgi?= Date: Thu, 27 Aug 2026 23:46:00 +0200 Subject: [PATCH 1/2] Modules: a usable registry entry wins over a same-named baseline (#2548) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `db552ffbf` removed the Blazor view packs' `MeshModuleClosure` rows and their `Modules:Assemblies` entries together. Removal of the baseline entries was the operative, deliberate half β€” a baseline entry shadowed the registry-landed copy by name, so the installed pack was unreachable. But the closure rows went with it and no host re-added them, leaving the packs in NEITHER place: every layout-area control on a fresh mesh rendered as `Controls.Html(HtmlEncode(instance.ToString()))`, and because `Modules:Required` names neither pack the portal booted healthy while serving escaped JSON. The maintainer's call was registry-wins rather than putting the image floor back, which keeps #2169 Phase B2 and removes the shadow at its source instead of trading it back. `ComputeEffectiveModuleEntries` is now three passes: 1. work out which enabled persisted entries are USABLE β€” they, and only they, may override; 2. walk the baseline in ITS OWN ORDER, substituting a usable override in place; 3. append usable persisted entries that have no baseline counterpart. Substituting rather than appending is the half this most easily gets wrong: the module must still load exactly once, and with nothing overriding, the emitted list is what it was before this change β€” same entries, same order. An UNUSABLE persisted entry must not remove the baseline. A landed DLL that never arrived or a platform gate that refuses the entry would otherwise turn a rejected upgrade into a MISSING module, which is strictly worse than running the older copy. A DISABLED entry does not override either β€” disabling a module returns it to the baseline rather than deleting it. The gates now run for an entry that shadows a baseline name, which they did not before: the old code deduped such an entry away before reaching them, so a store-installed module that could not load was indistinguishable from one never installed. Reporting it is the point. 🚨 `ModuleLoadReport`'s baseline remediation was about to become actively harmful. It said "delist it from Modules:Assemblies to let the landed generation win", true only while a baseline shadowed the store entry by name. Reaching that branch now means no usable store entry exists β€” so delisting removes the only copy that loads. Rewritten to say re-install, and to contradict the old advice explicitly rather than merely omit it. Two existing tests encoded the old contract and are inverted, not patched: `PersistedDuplicateOfBaseline_IsDeduplicatedByName` asserted the baseline wins, and `A_newer_landed_generation_that_loses_to_the_image_copy_is_reported_as_stale` asserted the image copy still wins. Both were correct descriptions of the behaviour being changed. Verified red: reverting to baseline-wins fails SIX tests, including both halves that matter β€” the override itself and the unusable-entry fallback. 42/42 green with the change. 🚨 This does NOT fix `memex-local`. A mesh with no registry has no bundle source at all, so a laptop keeps rendering raw JSON until the local landing lane exists β€” MeshWeaver#2417. The two are the same defect from opposite ends and #2417 stays open. Fixes #2548 --- .../ModuleActivation.cs | 51 ++++++++-- .../ModuleLoadReport.cs | 18 +++- .../ModuleActivationBootTest.cs | 81 +++++++++++++++- .../ModuleLoadReportTest.cs | 93 ++++++++++++++----- 4 files changed, 198 insertions(+), 45 deletions(-) diff --git a/src/MeshWeaver.PluginCatalog/ModuleActivation.cs b/src/MeshWeaver.PluginCatalog/ModuleActivation.cs index 8d966939a6..fc287d1bcf 100644 --- a/src/MeshWeaver.PluginCatalog/ModuleActivation.cs +++ b/src/MeshWeaver.PluginCatalog/ModuleActivation.cs @@ -504,20 +504,22 @@ public static ImmutableList ComputeEffectiveModuleEntries( var effective = ImmutableList.CreateBuilder(); var seen = new HashSet(StringComparer.OrdinalIgnoreCase); - foreach (var entry in baselineEntries ?? []) - { - if (string.IsNullOrWhiteSpace(entry)) - continue; - effective.Add(new EffectiveModule(entry, Landed: null)); - seen.Add(Path.GetFileNameWithoutExtension(entry)); - } - + // ── Pass 1: which enabled persisted entries are USABLE ────────────────────────────────── + // A usable one OVERRIDES a same-named baseline entry (#2548); an unusable one must not, + // because removing the baseline would turn a rejected upgrade into a missing module β€” a + // strictly worse outcome than running the older copy. + // + // 🚨 The gates now run for an entry that shadows a baseline name, which they did not before: + // the old code deduped such an entry away BEFORE reaching them, so a store-installed module + // that could not load was silently indistinguishable from one that was never installed. + // Reporting it is the point β€” the operator needs to know the registry copy was refused. + var overrides = new Dictionary(StringComparer.OrdinalIgnoreCase); foreach (var module in persisted?.Entries ?? []) { if (!module.Enabled || string.IsNullOrWhiteSpace(module.Name)) continue; - if (!seen.Add(module.Name)) - continue; // already activated (baseline or an earlier entry) β€” dedupe by name + if (overrides.ContainsKey(module.Name)) + continue; // first ENABLED entry of a name wins, as it always has if (platformGate(module.MinMeshVersion) is { } reason) { @@ -534,6 +536,35 @@ public static ImmutableList ComputeEffectiveModuleEntries( continue; } + overrides[module.Name] = module; + } + + // ── Pass 2: the baseline, in its own order, with a usable override substituted IN PLACE ── + // Order is preserved deliberately: when nothing overrides, the emitted list is byte-for-byte + // what it was before this change, so the only behaviour that moves is the one #2548 is about. + foreach (var entry in baselineEntries ?? []) + { + if (string.IsNullOrWhiteSpace(entry)) + continue; + var name = Path.GetFileNameWithoutExtension(entry); + if (!seen.Add(name)) + continue; // a baseline that repeats a name is still deduped + + effective.Add(overrides.TryGetValue(name, out var winner) + ? new EffectiveModule(winner.Name + ".dll", winner) + : new EffectiveModule(entry, Landed: null)); + } + + // ── Pass 3: usable persisted entries with no baseline counterpart, in persisted order ──── + foreach (var module in persisted?.Entries ?? []) + { + if (!module.Enabled || string.IsNullOrWhiteSpace(module.Name)) + continue; + if (!overrides.TryGetValue(module.Name, out var winner) || !ReferenceEquals(winner, module)) + continue; // not the winning entry for this name, or not usable at all + if (!seen.Add(module.Name)) + continue; // already emitted in pass 2, substituted into the baseline's slot + effective.Add(new EffectiveModule(module.Name + ".dll", module)); } diff --git a/src/MeshWeaver.PluginCatalog/ModuleLoadReport.cs b/src/MeshWeaver.PluginCatalog/ModuleLoadReport.cs index 2fa7296093..da274a098a 100644 --- a/src/MeshWeaver.PluginCatalog/ModuleLoadReport.cs +++ b/src/MeshWeaver.PluginCatalog/ModuleLoadReport.cs @@ -136,11 +136,19 @@ private static string Remediation(string source) => + "Modules:Assemblies β€” decides which generation loads, and it still names the older " + "one. Re-install the module so activation records the newer generation." // The baseline lane: ResolveModulePath's landed probe looks in the fixed - // modules// folder, which generation landing never writes, so the image copy wins - // β€” and the sidecar entry that WOULD have named the generation was deduped away by name. - : "A baseline Modules:Assemblies entry resolves to the image copy and dedupes the store " - + "entry away by name; delist it from Modules:Assemblies to let the landed generation " - + "win."; + // modules// folder, which generation landing never writes, so the image copy wins. + // Before #2548 the sidecar entry that WOULD have named the generation was also deduped + // away by name; it no longer is, so reaching this branch means no usable store entry + // exists β€” which changes the advice completely. + // 🚨 NOT "delist it". Since #2548 a USABLE store entry overrides a same-named baseline, + // so a baseline entry that is still winning means there is no usable store entry to + // take over β€” and delisting would remove the only copy that loads rather than promoting + // a newer one. The baseline is the floor, and the floor is what you keep. + : "A baseline Modules:Assemblies entry is loading the image copy because no usable " + + "store-installed entry claims this name β€” an enabled entry whose landed DLL exists " + + "would override it. Re-install the module so activation records a landed generation " + + "this instance can load; do NOT delist the baseline, which is the fallback that " + + "keeps the module loading at all."; /// /// The newest copy of under {moduleRoot}/modules/ that is neither diff --git a/test/MeshWeaver.PluginCatalog.Test/ModuleActivationBootTest.cs b/test/MeshWeaver.PluginCatalog.Test/ModuleActivationBootTest.cs index b267d822a7..444443c5f2 100644 --- a/test/MeshWeaver.PluginCatalog.Test/ModuleActivationBootTest.cs +++ b/test/MeshWeaver.PluginCatalog.Test/ModuleActivationBootTest.cs @@ -44,18 +44,89 @@ public void EffectiveSet_IsBaselineUnionEnabledPersisted_InOrder() .Should().Equal("Acme.Widgets", "Acme.Reports"); } + /// + /// 🚨 #2548 INVERTED this. A usable store entry now OVERRIDES a same-named baseline entry + /// instead of being deduped away by it β€” case-insensitively, and taking the baseline's SLOT so + /// load order is unchanged. + /// + /// Before: the baseline won, and the registry-landed copy the operator had installed was + /// unreachable. That is what left the two Blazor view packs served from nowhere once they left + /// the image, so a fresh mesh rendered every control as escaped JSON while booting healthy. + /// + /// The module is still loaded exactly ONCE β€” the override substitutes, it does not + /// append, which is the half a "registry wins" change most easily gets wrong. + /// [Fact] - public void PersistedDuplicateOfBaseline_IsDeduplicatedByName_CaseInsensitive() + public void UsablePersistedEntry_OverridesTheSameNamedBaseline_CaseInsensitive() { var effective = ModuleActivationBoot.ComputeEffectiveModuleEntries( ["MeshWeaver.OgCard.dll"], List(Store("meshweaver.ogcard"), Store("Acme.Widgets"), Store("ACME.WIDGETS")), AcceptAll, AllDllsExist); - // A store install of an already-baseline module (any casing) must not double-load it. - effective.Select(m => m.Entry).Should().Equal("MeshWeaver.OgCard.dll", "Acme.Widgets.dll"); - effective[0].Landed.Should().BeNull("the BASELINE entry won the dedupe, so it stays baseline " - + "β€” resolving it through the shadowed sidecar entry's folder would be a different file"); + effective.Select(m => m.Entry).Should().Equal("meshweaver.ogcard.dll", "Acme.Widgets.dll"); + effective[0].Landed.Should().NotBeNull( + "the store entry now wins, so the module resolves through its LANDED generation β€” the " + + "whole point of #2548; a null here means the baseline shadowed it again"); + effective[0].Landed!.Name.Should().Be("meshweaver.ogcard"); + effective.Should().HaveCount(2, "an override SUBSTITUTES into the baseline's slot; it must " + + "never append a second entry for the same module"); + } + + /// + /// 🚨 The half that makes the override safe, and the one worth breaking the build over: an + /// UNUSABLE store entry must NOT remove the baseline. Otherwise a rejected upgrade β€” a landed + /// DLL that never arrived, a platform gate that refuses it β€” turns into a MISSING module, which + /// is strictly worse than running the older copy. + /// + [Fact] + public void UnusablePersistedEntry_LeavesTheBaselineLoading_AndSaysWhy() + { + var skips = new List<(string Module, string Reason)>(); + + var effective = ModuleActivationBoot.ComputeEffectiveModuleEntries( + ["MeshWeaver.OgCard.dll"], + List(Store("MeshWeaver.OgCard")), + AcceptAll, + _ => false, // its landed DLL is not there + (m, r) => skips.Add((m, r))); + + effective.Select(m => m.Entry).Should().Equal("MeshWeaver.OgCard.dll"); + effective[0].Landed.Should().BeNull("the baseline is the FLOOR β€” an unusable store entry " + + "must fall back to it, never delete it"); + skips.Should().ContainSingle().Which.Module.Should().Be("MeshWeaver.OgCard"); + } + + /// + /// A DISABLED store entry is not an override either β€” disabling a module must return it to the + /// baseline, not remove it. Distinct from the unusable case above: disabling is a deliberate + /// operator action and is not reported as a skip. + /// + [Fact] + public void DisabledPersistedEntry_DoesNotOverrideTheBaseline() + { + var effective = ModuleActivationBoot.ComputeEffectiveModuleEntries( + ["MeshWeaver.OgCard.dll"], + List(Store("MeshWeaver.OgCard", enabled: false)), + AcceptAll, AllDllsExist); + + effective.Select(m => m.Entry).Should().Equal("MeshWeaver.OgCard.dll"); + effective[0].Landed.Should().BeNull(); + } + + /// + /// With nothing overriding, the emitted list must be exactly what it was before #2548 β€” same + /// entries, same ORDER. The override substitutes in place precisely so that this holds. + /// + [Fact] + public void WithNoOverride_TheBaselineOrderIsUnchanged() + { + var effective = ModuleActivationBoot.ComputeEffectiveModuleEntries( + ["A.dll", "B.dll", "C.dll"], + List(Store("D.Widgets")), + AcceptAll, AllDllsExist); + + effective.Select(m => m.Entry).Should().Equal("A.dll", "B.dll", "C.dll", "D.Widgets.dll"); } [Fact] diff --git a/test/MeshWeaver.PluginCatalog.Test/ModuleLoadReportTest.cs b/test/MeshWeaver.PluginCatalog.Test/ModuleLoadReportTest.cs index 97d4073326..5aa423bb3c 100644 --- a/test/MeshWeaver.PluginCatalog.Test/ModuleLoadReportTest.cs +++ b/test/MeshWeaver.PluginCatalog.Test/ModuleLoadReportTest.cs @@ -69,49 +69,85 @@ private string PlacePack(string root, string directory, string sourceDll, DateTi } /// - /// 🚨 THE deliverable. The exact prod topology: a baseline Modules:Assemblies entry, an - /// older image copy, and a NEWER landed generation carrying different bytes. The image copy - /// still wins (that resolution is unchanged and deliberate) β€” but the report now NAMES it and - /// says out loud that a newer, different copy was passed over. + /// 🚨 #2548 CHANGED WHO WINS HERE, and this test now pins the new answer. The topology is the + /// prod one: a baseline Modules:Assemblies entry, an older image copy, and a NEWER + /// landed generation carrying different bytes. + /// + /// Before: the baseline claimed the name, the sidecar entry was deduped away, and the + /// image copy loaded β€” the report's whole job was to say out loud that a newer copy had been + /// passed over. Now the usable store entry OVERRIDES the baseline, so the landed generation is + /// what loads and there is nothing shadowed to report. + /// + /// The report itself is unchanged and still needed: it fires whenever a baseline entry is + /// genuinely the one loading β€” which, after #2548, means no usable store entry exists (covered + /// by ). /// [Fact] - public void A_newer_landed_generation_that_loses_to_the_image_copy_is_reported_as_stale() + public void A_usable_landed_generation_now_WINS_over_the_image_copy() { var image = PlacePack(imageRoot, PackName, OldBytes, new DateTime(2026, 8, 25, 10, 35, 0, DateTimeKind.Utc)); var landed = PlacePack(storeRoot, $"{PackName}@12d2c7c2", NewBytes, new DateTime(2026, 8, 25, 13, 53, 0, DateTimeKind.Utc)); - // The baseline claims the name, so the sidecar entry for the SAME module is deduped away β€” - // silently, by design (ModuleActivationBootTest.PersistedDuplicateOfBaseline_…). That is - // exactly how the landed generation stops being consulted. var resolved = ResolveAsBootDoes( [PackName + ".dll"], new ModuleActivationList { Entries = [new ModuleActivationEntry { Name = PackName, Directory = $"{PackName}@12d2c7c2" }], }); - // The store's landed probe looks in modules//, which a generation landing never - // writes, so resolution falls through to the image copy. Pin it β€” if this ever changes, the - // report below is describing a different world. + resolved.Should().ContainSingle(); - resolved[0].Path.Should().NotBe(landed); + resolved[0].Path.Should().Be(landed, + "#2548: a usable store entry overrides the same-named baseline, so the LANDED " + + "generation is what the loader is handed β€” not the older image copy it used to fall " + + "through to"); + resolved[0].Module.Landed.Should().NotBeNull( + "the winning entry must carry its landed pointer, or resolution has no generation to " + + "aim at and the override is cosmetic"); + + // Nothing was passed over, so there is nothing to warn about. + var lines = ModuleLoadReport.Describe(storeRoot, [(resolved[0].Module, resolved[0].Path)]); + var line = lines.Should().ContainSingle().Subject; + line.Name.Should().Be(PackName); + line.Source.Should().Be(ModuleActivationSources.Store); + line.Path.Should().Be(landed); + line.Shadowed.Should().BeNull( + "the newer copy is the one loading now; reporting it as shadowed would name a problem " + + "that no longer exists"); - // The report describes the path the loader was handed β€” here, forced to the image copy the - // prod pod actually mapped. + Render(lines).Warnings.Should().BeEmpty(); + image.Should().NotBe(landed, "the fixture must actually have two distinct copies, or this " + + "test would pass without discriminating anything"); + } + + /// + /// The report's remaining job after #2548, and the case its remediation text now describes: a + /// baseline entry that is loading because NO usable store entry claims the name, while a newer + /// generation sits on disk. The warning still has to fire β€” and it must no longer tell the + /// operator to delist the baseline, which is now the only copy that loads. + /// + [Fact] + public void A_baseline_that_wins_because_no_usable_store_entry_exists_is_still_reported() + { + var image = PlacePack(imageRoot, PackName, OldBytes, new DateTime(2026, 8, 25, 10, 35, 0, DateTimeKind.Utc)); + var landed = PlacePack(storeRoot, $"{PackName}@12d2c7c2", NewBytes, new DateTime(2026, 8, 25, 13, 53, 0, DateTimeKind.Utc)); + + // No sidecar entry at all β€” nothing can override, so the baseline legitimately wins. + var resolved = ResolveAsBootDoes([PackName + ".dll"], new ModuleActivationList()); var lines = ModuleLoadReport.Describe(storeRoot, [(resolved[0].Module, image)]); var line = lines.Should().ContainSingle().Subject; - line.Name.Should().Be(PackName); line.Source.Should().Be(ModuleActivationSources.AppSettings); - line.Path.Should().Be(image); - line.Mvid.Should().NotBeNull("the line must carry the identity of the bytes, not just a path"); - line.Shadowed.Should().NotBeNull( - "a newer, different copy in the module store is #2223 β€” nothing said so, and the fix " - + "shipped end-to-end without ever running"); + line.Shadowed.Should().NotBeNull("a newer, different copy is sitting unused β€” #2223"); line.Shadowed!.Path.Should().Be(landed); - line.Shadowed.Mvid.Should().NotBe(line.Mvid); - var warnings = Render(lines).Warnings; - warnings.Should().ContainSingle().Which.Should().Contain(PackName).And.Contain(landed); + var warning = Render(lines).Warnings.Should().ContainSingle().Subject; + warning.Should().Contain(PackName).And.Contain(landed); + warning.Should().NotContain("delist it from Modules:Assemblies", + "since #2548 that instruction would remove the only copy that loads rather than promote " + + "the newer one β€” a warning that names the wrong fix is worse than one that names none"); + warning.Should().Contain("do NOT delist the baseline", + "the replacement must say so explicitly: an operator who read the old advice, or who " + + "reasons the old way, needs the instruction contradicted, not merely absent"); } /// @@ -248,11 +284,18 @@ public void The_remediation_names_the_lane_that_actually_decides_which_generatio storeWarning.Should().NotContain("delist it from Modules:Assemblies", "a store-installed module has no Modules:Assemblies line to delist"); - // …and the BASELINE lane still gets the remediation that is right for it. + // …and the BASELINE lane still gets a DIFFERENT remediation β€” but no longer the old one. + // 🚨 Until #2548 this said "delist it from Modules:Assemblies to let the landed generation + // win", which was true only while a baseline entry shadowed the store entry by name. Now a + // usable store entry overrides the baseline, so reaching this branch means no usable store + // entry exists β€” and delisting would remove the only copy that loads. var baselineLines = ModuleLoadReport.Describe( storeRoot, [(new EffectiveModule(PackName + ".dll", null), older)]); var baselineWarning = Render(baselineLines).Warnings.Should().ContainSingle().Subject; - baselineWarning.Should().Contain("delist it from Modules:Assemblies"); + baselineWarning.Should().NotContain("delist it from Modules:Assemblies", + "that instruction removes the fallback instead of promoting the newer copy (#2548)"); + baselineWarning.Should().Contain("do NOT delist the baseline"); + baselineWarning.Should().Contain("Re-install the module"); baselineWarning.Should().NotContain("activation.json"); } From 4b771c6395f5dc77029d8c24cd226bef71165818 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Roland=20B=C3=BCrgi?= Date: Thu, 27 Aug 2026 23:46:44 +0200 Subject: [PATCH 2/2] What's New: an installed pack is what loads (#2548) --- ...026-08-27-installed-packs-are-what-load.md | 24 +++++++++++++++++++ 1 file changed, 24 insertions(+) create mode 100644 src/MeshWeaver.Documentation/Data/WhatsNew/2026-08-27-installed-packs-are-what-load.md diff --git a/src/MeshWeaver.Documentation/Data/WhatsNew/2026-08-27-installed-packs-are-what-load.md b/src/MeshWeaver.Documentation/Data/WhatsNew/2026-08-27-installed-packs-are-what-load.md new file mode 100644 index 0000000000..92dcd4fcbd --- /dev/null +++ b/src/MeshWeaver.Documentation/Data/WhatsNew/2026-08-27-installed-packs-are-what-load.md @@ -0,0 +1,24 @@ +--- +Name: An installed pack is what loads, even when the platform ships one too +Category: Fix +Description: A module you installed from the registry now wins over a same-named copy that ships with the platform β€” and if the installed one cannot load, the platform copy still keeps the feature working. +Icon: Bug +Order: -20260827 +--- + +# An installed pack is what loads, even when the platform ships one too + +When a module was both listed by the platform and installed from the registry, the platform's copy won and the installed one was unreachable. Installing or upgrading such a module appeared to succeed and changed nothing. + +On a **brand-new mesh** the same rule had a much louder effect. Two of the view packs that draw the interface had recently moved out of the platform image and into the registry, so on a fresh instance nothing supplied them β€” and every view fell back to printing its own description instead of rendering. The instance reported itself healthy while showing raw text where the interface should be. Existing installs were unaffected: they already had the packs from before the move. + +Now the installed copy is the one that loads. + +**Two things it deliberately does not do:** + +- **A copy that cannot load does not take the feature down with it.** If the installed module is missing its files, or is built for a newer platform than this instance runs, the platform's own copy keeps working and the reason is reported. A refused upgrade should never turn into a missing feature. +- **Disabling a module returns it to the platform's copy** rather than removing it. + +Nothing changes for an instance where only one copy exists β€” the same modules load, in the same order. + +**Still open:** an instance with no registry configured at all β€” a local development mesh β€” has nowhere to get these packs from, and is unaffected by this fix. That is tracked separately.