Repository navigation
Modules__Required: the ceiling written down and guarded, and the OVERWRITE nobody could see (Memex#131) - #2540
Merged
Merged
Conversation
Memex#131: memex-cloud declared Modules__Required__5 (MeshWeaver.Mcp.dll) against a block that rendered 0..4, so the key reached no container and /mcp went on answering 404 with Mcp__BaseUrl configured a few lines below it. #2512 widened the block to 0..7 for the same class of failure on Memex#129; this raises the ceiling to 9, writes down what the ceiling IS and why one exists, and puts the invariant under test. Why 9: the image's baseline Modules:Required is seven entries (0..6 — Radzen, Analysis, EntityViews, GoogleMaps, Speech, Social, AI), so an overlay that restates the baseline consumes 0..6 and anything it ADDS starts at 7. A ceiling of 7 leaves exactly one appendable slot; ten leaves three. Why a ceiling exists at all, now stated in the block: the ConfigMap's contract is that it names every key literally, because that is what EveryConfigKeySetInAValuesFile_IsBoundInTheConfigMap can check. A Helm `range` would render correctly and be invisible to the guard. A bounded list is the price of a checkable template — so the bound has to be written where the person who hits it reads it, in a repository that is not the one holding the overlay. Two guards (test/MeshWeaver.Documentation.Test/PlatformBakeLaneGuard.cs): * ModulesRequired_IsRenderedAsAContiguousRangeFromZero — the block must render 0..N with no gap, and a comment must still say `renders 0..N`. A gap is the silent variant of the same defect: the overlay's index reaches no container while its neighbours do, so the deployment requires a different set than the one it declares and helm reports success. * NoValuesFile_DeclaresAModulesRequiredIndexAboveTheCeiling — an over-ceiling index fails with the ceiling NAMED and the instruction to raise it. The general guard already refuses such a key, but tells the author to "template each one", which is the wrong instruction for an ordered list whose bound they cannot see. Proven red before being trusted, three ways, each restored after: A. delete the index-3 block -> "renders 0, 1, 2, 4, 5, 6, 7, 8, 9 — index 3 missing" B. values file declares __10 -> "renders Modules__Required__0..9 — RAISE THE CEILING" C. the literal Memex#131 shape (block back to 0..4, overlay declares 5) -> both fire, naming ceiling 0..4 and the stale comment. Also documents, in the block, that an overlay entry overrides the baseline BY INDEX and never appends — Memex#128's index 5 would have replaced MeshWeaver.Social.dll's guard the moment it started rendering. That is fixed on the Memex side (Mcp moves to index 7). Refs Memex#131. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
This PR fixes a configuration/Helm templating ceiling where Modules__Required__N overrides beyond the chart’s rendered range were silently dropped (leading to features like /mcp staying effectively disabled despite values being set). It extends the rendered index range to 0..9 (10 slots) and adds explicit test guards to prevent both gaps and “values exceed ceiling” regressions, with the ceiling rationale documented where future maintainers will see it.
Changes:
- Raise the rendered
Modules__Required__Nkeys in the portal ConfigMap template to include indices8and9, and document the intentional ceiling and its rationale. - Add two new guards in
PlatformBakeLaneGuardto enforce (a) contiguous0..Nrendering and (b) no values file declaring an index above the rendered ceiling, with actionable failure messages. - Add a “What’s New” entry documenting the fix and the new effective limit (up to 10 required modules).
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| test/MeshWeaver.Documentation.Test/PlatformBakeLaneGuard.cs | Adds guards that enforce contiguous Modules__Required__0..N rendering and prevent values files from declaring indices above the chart ceiling. |
| src/MeshWeaver.Documentation/Data/WhatsNew/2026-08-27-required-modules-ceiling.md | Documents the behavior change (up to 10 required modules) and the new guard behavior. |
| deploy/helm/templates/memex-portal/config.yaml | Extends Modules__Required__N templating to indices 8 and 9 and records the ceiling rationale inline. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
…only the one it loses
The overflow this issue started from (an index past the chart's ceiling reaching
no container) turned out to be the visible half. The other half is worse and is
live: `Modules:Required` is an ARRAY, so an overlay entry binds BY INDEX and
never appends. memex-cloud's `Modules__Required__5: MeshWeaver.Mcp.dll` does not
add MCP to the list — it REPLACES index 5 of the image's own list, which is
`MeshWeaver.Social.dll`. Rendered through the chart:
Modules__Required__4: "MeshWeaver.Speech.dll"
Modules__Required__5: "MeshWeaver.Mcp.dll" <- was MeshWeaver.Social.dll
Nothing in the system can see it. The replaced module is not MISSING — nobody
asks for it any more — so MissingRequired and RequiredModuleStatus have nothing
to say; the override IS rendered, so the chart's coverage gate passes; the
deploy succeeds and /health stays green having silently stopped guarding a
module. It is only visible where the image's baseline and the deployment's
overlay finally meet, which is the running process: the two halves ship from
different repositories.
So the check is at boot, beside MissingRequired, reading the same key the same
way: MeshBuilderModuleActivation.ShadowedRequired reports every entry a LATER
configuration provider replaced with a different module that appears nowhere
else in the effective list. Provider ORDER is the whole mechanism — a value a
provider supplies that is not the effective one has been shadowed — so there is
no provider-type sniffing and any layering (files, env, command line) is
covered.
Two shapes are deliberately NOT reported, because both are how the key is meant
to be used, and both are pinned:
* BLANKING an entry the deployment cannot satisfy (empty winner) — the
sanctioned opt-out, and the only remedy the 2026-08-23 rollouts had.
* REORDERING, where the replaced module is still required at another index.
Proven red, both directions, restored after each:
* Detector removed (return [] — the tree as it stands on main): the memex-cloud
shape reports NOTHING. Expected ["MeshWeaver.Social.dll"], actual [].
* Detector made naive (drop the blank + reorder exclusions): BlankingAnEntry and
ReorderingTheList both fail, each reporting two false positives — so neither
exclusion is decoration.
Refs Memex#131.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
rbuergi
changed the title
Contributor
Contributor
Test Results (shard 4) 8 files 8 suites 11m 50s ⏱️ Results for commit 6094544. |
Contributor
Contributor
Contributor
Contributor
Contributor
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.
What Memex#131 reported, and what it turned out to be
memex-cloud declares
Modules__Required__5: "MeshWeaver.Mcp.dll". The chart's block rendered0..4, so the key reached no container,/mcpwent on answering 404 withMcp__BaseUrlconfigured a few lines below it, and Memex'sconfig-key-coveragegate went red. #2512 already widened the block to0..7for the same failure on Memex#129 — so that red is gone.Fixing it exposed a worse, live defect.
Modules:Requiredis an array, so an overlay entry binds by index and never appends. Index 5 of the image's own list isMeshWeaver.Social.dll. So the moment index 5 started rendering,MeshWeaver.Mcp.dlldid not add MCP — it replaced Social's requirement. Rendered through this chart with Memexmain's overlay:That is invisible from every direction: the replaced module is not missing (nobody asks for it any more), so
MissingRequiredandRequiredModuleStatushave nothing to say; the override is rendered, so the coverage gate passes; the deploy succeeds and/healthstays green having quietly stopped guarding a module.Three changes
1. Ceiling 7 → 9, and the reason is now in the block.
Why a ceiling exists at all: this ConfigMap's contract is that it names every key literally, because that is what
EveryConfigKeySetInAValuesFile_IsBoundInTheConfigMapcan check — a Helmrangerenders correctly and is invisible to the guard. A bounded list is the price of a checkable template, and the person who hits the bound sits in a different repository from the chart, so it has to be written down rather than counted off the template.Why 9: the image's baseline
Modules:Requiredis seven entries (0..6— Radzen, Analysis, EntityViews, GoogleMaps, Speech, Social, AI). An overlay restating the baseline consumes0..6, so anything it adds starts at 7. A ceiling of 7 leaves exactly one appendable slot. Ten leaves three.2. Two static guards in
test/MeshWeaver.Documentation.Test/PlatformBakeLaneGuard.cs, beside the existing coverage guard — for overflow:ModulesRequired_IsRenderedAsAContiguousRangeFromZero— renders0..Nwith no gap, and a comment still saysrenders 0..N. A gap is the silent variant: the overlay's index reaches no container while its neighbours do, so the deployment requires a different set than it declares.NoValuesFile_DeclaresAModulesRequiredIndexAboveTheCeiling— an over-ceiling index fails with the ceiling named. The general guard already refuses such a key, but its instruction is "template each one", which is the wrong instruction for an ordered list whose bound the author cannot see.3. A boot-time check in
MeshBuilderModuleActivation.ShadowedRequired— for collision:Reports every entry a later configuration provider replaced with a different module that appears nowhere else in the effective list. Provider order is the whole mechanism — a value a provider supplies that is not the effective one has been shadowed — so there is no provider-type sniffing and any layering (files, env, command line) is covered. It sits beside
MissingRequired, reads the same key the same way, and reports on the same channel.It has to be at boot rather than in a chart guard: the image's baseline ships from one repository and the overlay from another, and the two are only ever visible together in the running process.
Two shapes are deliberately not reported, because both are how the key is meant to be used, and both are pinned by tests: blanking an entry a deployment cannot satisfy (the sanctioned opt-out, and the only remedy the 2026-08-23 rollouts had), and reordering, where the module is still required at another index.
Proven RED — five seeds, each restored
A guard nobody watched fail is not a guard.
hasKeyblockModules__Required__100..4, overlay declares 5)0..4, and the comment-drift assertion catches the stale sentenceShadowedRequiredreturns[](i.e. main today)Expected: ["MeshWeaver.Social.dll"] / Actual: []ShadowedRequiredmade naive (drop the blank + reorder exclusions)BlankingAnEntry_IsNotAShadowandReorderingTheList_IsNotAShadowboth fail with two false positives each — neither exclusion is decorationVerification
dotnet build test/MeshWeaver.Documentation.Test/… -c Release -warnaserror→0 Warning(s) 0 Error(s); 94/94 passeddotnet build test/MeshWeaver.Graph.Test/… -c Release -warnaserror --no-incremental→0 Warning(s) 0 Error(s);ConfiguredModuleActivationTest13/13 passeddotnet build src/MeshWeaver.Mesh.Contract -c Release -warnaserror→ clean. The change is purely additive (one new public static + one extra report loop), so no binary break.helm templateof Memexmain's memex-cloud overlay renders as expectedscripts/check-config-key-coverage.pyagainst Memexmain's overlays with this chart → PASSCompanion
The Memex-side PR moves MCP to index 7 — the first index past the baseline — which is what actually makes
/mcprequired without dropping Social, and which needs this ceiling to exist.Refs Memex#131.
🤖 Generated with Claude Code