Repository navigation
Modules: a usable registry entry wins over a same-named baseline (#2548) - #2558
Merged
Merged
Conversation
`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
Contributor
There was a problem hiding this comment.
Pull request overview
Updates module activation resolution so a usable registry-installed (persisted) module entry overrides a same-named baseline Modules:Assemblies entry (fixing the “installed pack unreachable” scenario behind #2548), while preserving baseline fallback behavior when the persisted entry is unusable or disabled.
Changes:
- Reworks
ComputeEffectiveModuleEntriesinto a 3-pass algorithm (usable persisted overrides → baseline substitution-in-place → append non-baseline persisted). - Updates
ModuleLoadReportremediation messaging and adjusts/extends tests to pin the new resolution and reporting contract. - Adds a What’s New entry documenting the behavior change and its intentional limits.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| test/MeshWeaver.PluginCatalog.Test/ModuleLoadReportTest.cs | Updates assertions to reflect persisted override winning when usable; adds coverage for baseline-winning report/remediation. |
| test/MeshWeaver.PluginCatalog.Test/ModuleActivationBootTest.cs | Replaces “baseline wins” expectations with “usable persisted overrides”, plus new safety/ordering tests. |
| src/MeshWeaver.PluginCatalog/ModuleLoadReport.cs | Updates baseline-lane remediation text to match the new “registry wins when usable” contract. |
| src/MeshWeaver.PluginCatalog/ModuleActivation.cs | Implements the 3-pass effective-module computation with explicit usability gating and in-place substitution. |
| src/MeshWeaver.Documentation/Data/WhatsNew/2026-08-27-installed-packs-are-what-load.md | Documents the fix and clarifies remaining known gap (no-registry local instance). |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+542
to
+544
| // ── 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
+147
to
+151
| : "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
519
to
+522
| 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 |
Contributor
Test Results (shard 2)1 088 tests 896 ✅ 5m 24s ⏱️ Results for commit 4b771c6. |
Contributor
Test Results (shard 3)1 113 tests 1 113 ✅ 18m 15s ⏱️ Results for commit 4b771c6. |
Contributor
Test Results (shard 0)1 316 tests 1 316 ✅ 6m 6s ⏱️ Results for commit 4b771c6. |
Contributor
Test Results (shard 5)1 606 tests 1 605 ✅ 9m 43s ⏱️ Results for commit 4b771c6. |
Contributor
Test Results (shard 4) 8 files 8 suites 11m 54s ⏱️ Results for commit 4b771c6. |
Contributor
Test Results (shard 1)739 tests 735 ✅ 7m 12s ⏱️ Results for commit 4b771c6. |
Contributor
Test Results 44 files 44 suites 58m 35s ⏱️ Results for commit 4b771c6. |
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 #2548. Maintainer's call: registry wins over baseline, not putting the image floor back.
What was broken
db552ffbfremoved two things together: the view packs'MeshModuleClosurerows and theirModules:Assembliesentries. Removing 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 then rendered as
Controls.Html(HtmlEncode(instance.ToString()))— and becauseModules:Requirednames neither pack,the portal booted healthy while serving escaped JSON. Existing prod portals were unaffected only
because they had installed the packs before the flip, which is exactly why it went unnoticed.
The change
ComputeEffectiveModuleEntriesbecomes three passes: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 byte-for-byte what it was before —
same entries, same order. That is asserted directly (
WithNoOverride_TheBaselineOrderIsUnchanged).The two safety halves, both pinned
A consequence worth stating: the gates now run for an entry that shadows a baseline name, which they
did not before — the old code deduped it away before reaching them, so a store-installed module
that could not load was indistinguishable from one never installed. Reporting it is the point.
🚨 The remediation text was about to become actively harmful
ModuleLoadReporttold operators:True only while a baseline shadowed the store entry. After this change, reaching that branch 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 (
do NOT delist the baseline) rather than merely omit it: an operator who remembers the old instruction needs itcountered, not absent. Asserted both ways.
Two existing tests inverted, not patched
PersistedDuplicateOfBaseline_IsDeduplicatedByNameasserted the baseline wins the dedupe.A_newer_landed_generation_that_loses_to_the_image_copy_is_reported_as_staleasserted the image copystill wins. Both were correct descriptions of the behaviour being changed, so they became the new
contract rather than being adjusted around.
The report's remaining job gets its own case:
A_baseline_that_wins_because_no_usable_store_entry_exists_is_still_reported.Controls
dotnet build -c Release -warnaserror: 0 Warning(s), 0 Error(s).🚨 What this deliberately does NOT fix
memex-local. A mesh with no registry has no bundle source at all, so a laptop keeps rendering rawJSON until a local landing lane exists — that is #2417, and the maintainer chose this option with
that gap understood. #2417 and #2548 are the same defect from opposite ends; #2417 stays open and must
not be read as covered by this.
Also unchanged:
Modules:Requirednaming neither view pack, which is why the portal boots healthywhile serving raw JSON. Whether that list should include them is a separate decision, raised on #2548
rather than folded in here.
What's New
Category: Fix— a fresh mesh renders its UI instead of escaped JSON.