Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
@@ -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.
51 changes: 41 additions & 10 deletions src/MeshWeaver.PluginCatalog/ModuleActivation.cs
Original file line number Diff line number Diff line change
Expand Up @@ -504,20 +504,22 @@ public static ImmutableList<EffectiveModule> ComputeEffectiveModuleEntries(
var effective = ImmutableList.CreateBuilder<EffectiveModule>();
var seen = new HashSet<string>(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<string, ModuleActivationEntry>(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
Comment on lines 519 to +522

if (platformGate(module.MinMeshVersion) is { } reason)
{
Expand All @@ -534,6 +536,35 @@ public static ImmutableList<EffectiveModule> 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.
Comment on lines +542 to +544
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));
}

Expand Down
18 changes: 13 additions & 5 deletions src/MeshWeaver.PluginCatalog/ModuleLoadReport.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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/<Name>/ 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/<Name>/ 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.";
Comment on lines +147 to +151

/// <summary>
/// The newest copy of <paramref name="name"/> under <c>{moduleRoot}/modules/</c> that is neither
Expand Down
81 changes: 76 additions & 5 deletions test/MeshWeaver.PluginCatalog.Test/ModuleActivationBootTest.cs
Original file line number Diff line number Diff line change
Expand Up @@ -44,18 +44,89 @@ public void EffectiveSet_IsBaselineUnionEnabledPersisted_InOrder()
.Should().Equal("Acme.Widgets", "Acme.Reports");
}

/// <summary>
/// 🚨 #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.
///
/// <para>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.</para>
///
/// <para>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.</para>
/// </summary>
[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");
}

/// <summary>
/// 🚨 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.
/// </summary>
[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");
}

/// <summary>
/// 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.
/// </summary>
[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();
}

/// <summary>
/// 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.
/// </summary>
[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]
Expand Down
Loading
Loading