Skip to content

fix(install): an explicit package set narrows the baseline on a bound portal too - #6769

Merged
rbuergi merged 3 commits into
mainfrom
fix/preinstalled-baseline-binder
Oct 14, 2026
Merged

rbuergi merged 3 commits into
mainfrom
fix/preinstalled-baseline-binder

Conversation

@rbuergi

@rbuergi rbuergi commented Oct 14, 2026

Copy link
Copy Markdown
Contributor

What

Follow-up to #6737 (policy declared-package-set-is-exact): the explicit-set rule now holds on a portal whose options are bound from configuration.

PluginCatalogOptions.InstallPreInstalledPackages keeps its bool surface and default. A new bool? InstallPreInstalledPackagesConfigured, bound from the same key ([ConfigurationKeyName]), stays null when the key is absent; EffectiveInstallPreInstalledPackages is what the default install decides on (configured value, else false when code set the bool to false, else unset).

Why (measured)

After #6737 shipped (3.1.10808), a fresh control instance still selected 48 packages. ConfigurationBinder reads each bindable property's current value and writes it back through the setter even when the key is absent (it skips only null). The #6737 bool read true while unset, so binding always "set" it to true, and the rule never fired outside tests that built options by hand.

Tests

PreInstalledBaselineSelectionTest.BoundFromConfiguration_AnExplicitSet_FollowsTheKeyOnly (absent → baseline off, true → on, false → off) and BoundFromConfiguration_NoPatterns_KeepsTheBaseline, binding through ConfigurationBuilder().GetSection("PluginCatalog").Get<PluginCatalogOptions>() exactly as MemexConfiguration does.

Negative control: with the decision reading the bound bool, BoundFromConfiguration_AnExplicitSet_FollowsTheKeyOnly(key: null, expectedBaseline: False) FAILS. Restored: 29/29 across the bootstrap + default-install classes; 830/830 MeshWeaver.Documentation.Test. -c Release -warnaserror: 0 errors.

Implementers: none. Pairs-with: none — additive (InstallPreInstalledPackagesConfigured becomes settable, EffectiveInstallPreInstalledPackages added); the bool property is unchanged.

🤖 Generated with Claude Code

rbuergi and others added 2 commits October 14, 2026 15:44
… portal too

ConfigurationBinder writes every property's current value back through its setter, so the bool
backed by a nullable field (#6737) read 'configured: true' on every deployed portal and the
explicit-set rule never fired (a fresh control instance on the fixed build still selected 48).
InstallPreInstalledPackages keeps its bool surface; a bool? InstallPreInstalledPackagesConfigured is
bound from the same key and stays null when the key is absent. Pinned by binding tests that fail on
the previous shape.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Copilot AI balanced review requested due to automatic review settings October 14, 2026 13:44
@rbuergi
rbuergi enabled auto-merge October 14, 2026 13:44

Copilot AI left a comment

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.

🟡 Changes recommended

Explicit programmatic assignment of the existing public bool to true now silently loses its documented effect.

1 open finding
What changed in this PR

Fixes configuration-bound package selection so explicit package sets correctly suppress the preinstalled baseline when the key is absent.

Changes:

  • Adds a nullable configuration-bound switch and effective baseline value.
  • Uses the effective value during default installation.
  • Adds configuration-binding regression tests.
File Description
PluginCatalogOptions.cs Distinguishes configured and default switch values.
InstanceAutoRegistrationService.cs Uses the effective baseline setting.
StoreBootstrapLandsModulesFirstTest.cs Tests bound configuration scenarios.

🧠 Review effort: Balanced


💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread src/MeshWeaver.PluginCatalog/PluginCatalogOptions.cs
…h InstallPreInstalledPackagesConfigured

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Copilot AI balanced review requested due to automatic review settings October 14, 2026 14:08
@meshweaver-cloud
meshweaver-cloud Bot disabled auto-merge October 14, 2026 14:09
@rbuergi
rbuergi enabled auto-merge October 14, 2026 14:09

Copilot AI left a comment

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.

🔵 Needs a closer look

The BaselineApplies documentation still contradicts the newly intentional programmatic-true behavior.

0 open findings

1 resolved since last review
Previously missed (1)

In code that hasn't changed since last review

Low severity Correct BaselineApplies documentation for pre-installed package configuration

src/​MeshWeaver.PluginCatalog/​InstanceAutoRegistrationService.cs:847

The XML documentation for BaselineApplies below still says an explicit PluginCatalogOptions.InstallPreInstalledPackages = true wins. This call now intentionally routes through EffectiveInstallPreInstalledPackages, where a programmatic true is treated as unset and only the configured key/nullable property opts in. Please update that documentation so callers are not given the opposite guidance (for example, refer to the configured key or InstallPreInstalledPackagesConfigured).

🧠 Review effort: Balanced

@github-actions

github-actions Bot commented Oct 14, 2026 •

Copy link
Copy Markdown
Contributor

Test Results

   10 files   -    15     10 suites   - 15   30m 46s ⏱️ - 26m 6s
7 404 tests  - 5 103  7 404 ✅  - 4 910  0 💤  - 193  0 ❌ ±0 
7 413 runs   - 5 109  7 413 ✅  - 4 916  0 💤  - 193  0 ❌ ±0 

Results for commit 6f2344d. ± Comparison against base commit 54ef925.

This pull request removes 5142 and adds 13 tests. Note that renamed tests count towards both.

   --- End of inner exception stack trace ---
   --- End of inner exception stack trace ---)
   --- End of inner exception stack trace ---, expected: True)
   --- End of inner exception stack trace ---, isDenial: True)
 ---> (Inner Exception #1) MeshWeaver.Mesh.QueryProviderStalledException: Query provider(s) [pg] did not emit an Initial within the query fan-in's 16s bound for query 'nodeType:NodeType' (user 'system'). The merged Initial gates on EVERY provider, so this query has NO snapshot to answer with — it is reported as unavailable (retryable) rather than left hanging with no error. This is an availability failure, never a permission verdict: a consumer deciding access must fail CLOSED and say it could not establish the answer. Fix the stalled provider; never bump the consumer's timeout.<---
 ---> (Inner Exception #1) MeshWeaver.Messaging.Hub.Test.InfrastructureFaultTest+ProviderException (0x80004005): Failed to connect to 10.42.18.4:5432<---
 ---> (Inner Exception #1) System.ArgumentException: Value does not fall within the expected range.<---
 ---> (Inner Exception #1) System.InvalidOperationException: boom<---
 ---> (Inner Exception #1) System.InvalidOperationException: source B is misconfigured<---
…
Memex.Portal.Shared.Test.InstanceIdRulesMatchTheRegistryTest ‑ TheSetupHostAgreesWithTheRegistry(candidate: "ace850b2-10d8-452b-b7e7-b0fe0bf4067d")
Memex.Portal.Shared.Test.PreInstalledBaselineSelectionTest ‑ BoundFromConfiguration_AnExplicitSet_FollowsTheKeyOnly(key: "false", expectedBaseline: False)
Memex.Portal.Shared.Test.PreInstalledBaselineSelectionTest ‑ BoundFromConfiguration_AnExplicitSet_FollowsTheKeyOnly(key: "true", expectedBaseline: True)
Memex.Portal.Shared.Test.PreInstalledBaselineSelectionTest ‑ BoundFromConfiguration_AnExplicitSet_FollowsTheKeyOnly(key: null, expectedBaseline: False)
Memex.Portal.Shared.Test.PreInstalledBaselineSelectionTest ‑ BoundFromConfiguration_NoPatterns_KeepsTheBaseline
Memex.Portal.Shared.Test.PublishStorageFaultIsNotAConflictTest ‑ AStorageFault_IsA500_NamingTheError(fault: System.AggregateException: One or more errors occurred. (Access to the path '/data/modules/activation.d/MeshWeaver.Markdown.Export.json' is denied.)
 ---> System.UnauthorizedAccessException: Access to the path '/data/modules/activation.d/MeshWeaver.Markdown.Export.json' is denied.
   --- End of inner exception stack trace ---)
Memex.Portal.Shared.Test.PublishStorageFaultIsNotAConflictTest ‑ AStorageFault_IsA500_NamingTheError(fault: System.InvalidOperationException: landing faulted
 ---> System.UnauthorizedAccessException: Access to the path '/data/modules/activation.d/MeshWeaver.Markdown.Export.json' is denied.
   --- End of inner exception stack trace ---)
Memex.Portal.Shared.Test.SessionDenialIsAnAnswerTest ‑ OnlyAVerdictReadsAsADenial(shape: "the same verdict nested, as a late denial dispatch"···, failure: System.InvalidOperationException: write failed
 ---> System.UnauthorizedAccessException: Access denied
   --- End of inner exception stack trace ---, isDenial: True)
MeshWeaver.Compiler.Pipeline.Test.SourcesWatcherStallBackoffTest ‑ The_production_classifier_recognises_a_stall_through_every_wrapping(because: "a stall as an aggregate's FIRST member", fault: System.AggregateException: One or more errors occurred. (Query provider(s) [pg] did not emit an Initial within the query fan-in's 16s bound for query 'nodeType:NodeType' (user 'system'). The merged Initial gates on EVERY provider, so this query has NO snapshot to answer with — it is reported as unavailable (retryable) rather than left hanging with no error. This is an availability failure, never a permission verdict: a consumer deciding access must fail CLOSED and say it could not establish the answer. Fix the stalled provider; never bump the consumer's timeout.)
 ---> MeshWeaver.Mesh.QueryProviderStalledException: Query provider(s) [pg] did not emit an Initial within the query fan-in's 16s bound for query 'nodeType:NodeType' (user 'system'). The merged Initial gates on EVERY provider, so this query has NO snapshot to answer with — it is reported as unavailable (retryable) rather than left hanging with no error. This is an availability failure, never a permission verdict: a consumer deciding access must fail CLOSED and say it could not establish the answer. Fix the stalled provider; never bump the consumer's timeout.
   --- End of inner exception stack trace ---, expected: True)
MeshWeaver.Compiler.Pipeline.Test.SourcesWatcherStallBackoffTest ‑ The_production_classifier_recognises_a_stall_through_every_wrapping(because: "a stall as an aggregate's SECOND member", fault: System.AggregateException: One or more errors occurred. (The operation has timed out.) (Query provider(s) [pg] did not emit an Initial within the query fan-in's 16s bound for query 'nodeType:NodeType' (user 'system'). The merged Initial gates on EVERY provider, so this query has NO snapshot to answer with — it is reported as unavailable (retryable) rather than left hanging with no error. This is an availability failure, never a permission verdict: a consumer deciding access must fail CLOSED and say it could not establish the answer. Fix the stalled provider; never bump the consumer's timeout.)
 ---> System.TimeoutException: The operation has timed out.
   --- End of inner exception stack trace ---
 ---> (Inner Exception #1) MeshWeaver.Mesh.QueryProviderStalledException: Query provider(s) [pg] did not emit an Initial within the query fan-in's 16s bound for query 'nodeType:NodeType' (user 'system'). The merged Initial gates on EVERY provider, so this query has NO snapshot to answer with — it is reported as unavailable (retryable) rather than left hanging with no error. This is an availability failure, never a permission verdict: a consumer deciding access must fail CLOSED and say it could not establish the answer. Fix the stalled provider; never bump the consumer's timeout.<---
, expected: True)
…

♻️ This comment has been updated with latest results.

@rbuergi
rbuergi merged commit c457670 into main Oct 14, 2026
78 of 80 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants