Declare an image assembly SUPERSEDED so a type can move out of it into a module in one wave - #3225
Conversation
#1268's blocker) The container build's reference set is the image's /app minus Graph.ShadowedAssemblyNames, and that set holds only what the run compiles FROM SOURCE — membership being reachability- driven through ProjectReference edges. Correct, until a type moves OUT of an image-shipped assembly into a module in the SAME repository: the image is one wave behind and still defines the type, the module defines it too, and the ProjectReference edge that would have shadowed the stale copy is precisely the edge the move deletes. CS0436, in the one case the shadow set structurally cannot see. `superseded-image-assemblies:` on node-repo-module-pack.yml → `--superseded-image-assembly` on build-project adds the name to the shadow set, dropping the image's stale copy for that run. Declared, never inferred: an automatic collision-resolver would mask a genuine two-producers-of-one-assembly defect, which BakeHost.ShippedByHostProblem (#3175) exists to make RED. Fails closed on an empty name, a name the image does not carry (wrong, or STALE), and a name the run already builds. No "is anything else needed from it" check — the compiler answers that with CS0246/CS0234 naming the missing type. Optional with an inert default: the required input set (modules, platform-ref) is unchanged, so the five calling repos are unaffected. Staleness of an entry is NOT detected and is filed as #3223. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01G4xPnGYEbdu8AVvR5jytyj
There was a problem hiding this comment.
🟡 Changes recommended
The updated reusable workflow contains a duplicate YAML key (default) and the input parsing currently allows malformed comma lists (empty entries) to become a silent no-op, contradicting the “fail closed” contract.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
Adds an explicit “superseded image assembly” mechanism to the module-pack build lane so a type can move out of an image-shipped assembly into a module in the same wave without CS0436 imported-type collisions, with corresponding CLI plumbing, tests, and documentation.
Changes:
- Extend
mw-plugin-test build-project(ProjectBuild) withSupersededImageAssemblies, validating inputs and adding them toGraph.ShadowedAssemblyNames. - Wire a new reusable-workflow input (
superseded-image-assemblies) throughnode-repo-module-pack.ymlto pass--superseded-image-assemblyflags into the container build. - Add a focused regression test suite plus a documentation section describing the shadow-set blind spot and the new escape hatch.
File summaries
| File | Description |
|---|---|
| tools/MeshWeaver.PluginTester/ProjectBuild.cs | Implements the “drop superseded image assemblies” behavior by extending the shadow set with fail-closed validation. |
| tools/MeshWeaver.PluginTester/Program.cs | Adds CLI parsing and usage text for --superseded-image-assembly. |
| test/MeshWeaver.PluginTester.Test/SupersededImageAssemblyTest.cs | Verifies both the control arm (CS0436 without the option) and the success/fail-closed behaviors with the option. |
| src/MeshWeaver.Documentation/Data/Architecture/ModuleBuildArchitecture.md | Documents the scenario, rationale, and known gap (#3223). |
| .github/workflows/node-repo-module-pack.yml | Adds the reusable-workflow input and forwards it into the global container build invocation. |
Review details
- Files reviewed: 5/5 changed files
- Comments generated: 2
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| default: '' | ||
| default: '' |
| # The image's stale copy of a moved type's old home — see superseded-image-assemblies. | ||
| # Empty ⇒ no flags ⇒ byte-identical invocation to before this input existed. | ||
| superseded=() | ||
| for asm in ${SUPERSEDED//,/ }; do superseded+=(--superseded-image-assembly "$asm"); done | ||
| [ "${#superseded[@]}" -eq 0 ] || echo "superseding ${#superseded[@]} image assembl(y|ies) from the reference set: ${SUPERSEDED}" |
Test Results (shard 1)459 tests 459 ✅ 53s ⏱️ Results for commit 71ae158. |
Test Results (shard 3)346 tests 344 ✅ 43s ⏱️ Results for commit 71ae158. |
Test Results (shard 0)243 tests 243 ✅ 2m 1s ⏱️ Results for commit 71ae158. |
Test Results (shard 5) 4 files 4 suites 1m 37s ⏱️ Results for commit 71ae158. |
Test Results (shard 2)578 tests 386 ✅ 2m 37s ⏱️ Results for commit 71ae158. |
Test Results (shard 4)2 201 tests 2 201 ✅ 2m 38s ⏱️ Results for commit 71ae158. |
Test Results 15 files 15 suites 10m 32s ⏱️ Results for commit 71ae158. |
The defect
MeshWeaver.Plugins#1268 moves three Razor views (
CollaborativeMarkdownView,CommentableView,MarkdownEditorView, namespaceMeshWeaver.Blazor.Components) out of the image-shipped assemblyMeshWeaver.Blazor.Viewsinto the registry-served moduleMeshWeaver.Markdown.Collaboration, so thatmodule has ONE producer (#3175 / plugins#1262). Its global build fails:
(same for the other two, from both the
.razor.csand the Razor generator's.g.cs)Why the existing shadow set cannot see it
ProjectBuild.Runreads the image withContainerReferenceSet, then builds the reference set asthe whole container MINUS
Graph.ShadowedAssemblyNames—ProjectBuild.cs, the loop atif (graph.ShadowedAssemblyNames.Contains(assemblyName)) continue;.shadowedismodels.Values.Select(m => m.AssemblyName).Concat(prebuiltReferences.Keys)— onlyassemblies this run compiles from source, and graph membership is reachability-driven
through
ProjectReferenceedges under the source root.Blazor.Viewsin the graph is exactly the edge ci: an ordinary failing test was reported as a dying host #1268 deletes. So it isnot shadowed, the image's previous copy is imported, and it still defines the three types.
This is the duplicate-producer problem one level down — two definitions of one TYPE rather than two
producers of one ASSEMBLY — and it lasts exactly one wave. It is also general: it will recur for every
type that leaves an image-shipped assembly for a module in the same repo.
The change
superseded-image-assemblies:(comma/whitespace-separated, optional) onnode-repo-module-pack.yml→--superseded-image-assembly <name>(repeatable) onbuild-project,which adds each name to the shadow set so the image's stale copy leaves the reference set.
An exclusion, not a preference. My first design brief for this said the named assembly must also
be built by the run's workspace, else fail. That is wrong and would have refused the only case the
feature exists for: the module's graph does not build
Blazor.Viewsand never will — that is thesituation. Recorded here because the mistake is easy to repeat.
Three load-bearing properties:
mask a genuine two-producers defect, which
BakeHost.ShippedByHostProblem(Release availability must confirm a CONSISTENT sealed set (dependency records match), not bundle presence — memex-cloud rolled to ci.7621 and then DECLINED SocialMedia at adoption #3175) exists to makeRED. The caller names the assembly.
STALE now the image dropped it), and a name the run already builds from source (redundant, and a
silent no-op would leave the misunderstanding in place). An input that quietly does nothing reads
exactly like an input that worked.
CS0246/CS0234naming the missing type, a positive and specific signal. Noted in the code sonobody adds a redundant one.
Five callers are unaffected
MeshWeaver.Plugins,.Education,.Reinsurance,.SocialMedia,.Manufacturingcall this lane. Theinput is
required: falsewithdefault: ''; parsed by YAML, the lane's required set is still exactly['modules', 'platform-ref'](verified, not assumed — a new REQUIRED input on a reusable workflow isa silent
startup_failure: zero jobs, zero contexts, which reads as GREEN). With it empty the shellbuilds no flags, so the
docker runline is byte-identical to before.Verification
tools/MeshWeaver.PluginTester-warnaserror→0 Warning(s) / 0 Error(s)src/MeshWeaver.Documentation-warnaserror→0 Warning(s) / 0 Error(s)test/MeshWeaver.PluginTester.Test(WHOLE project)Failed: 0, Passed: 324, Total: 324,.trxat 14:48Z vs 14:48:23Z starttest/MeshWeaver.Documentation.Test(WHOLE project, rebuilt first — docs areEmbeddedResource)Failed: 0, Passed: 246, Total: 246check-workflow-timeouts.py64 job(s) checked, 2 reusable-call job(s) exempt, 0 violation(s)SupersededImageAssemblyTesthas five cases: the effect, and one per fail-closed path — plus acontrol arm that builds the identical tree WITHOUT the option and requires
CS0436.🚨 The control arm earned its keep immediately. My first synthetic repro did not collide: a
declared type conflicts with nothing until something RESOLVES the name, so the control passed and the
positive test was proving nothing. Fixed by having the module's source actually bind
Moved.Thing(which is why the real case surfaced — the Razor generator's
.g.csbinds it). Then the control failedfor the wrong reason — the per-project
Failurecarries only"1 warning(s) under the no-warn policy"— so the diagnostic code is now asserted against the builder's captured narration.
What only Plugins#1268 can confirm: everything above runs against a synthetic container directory.
The real portal image, the Razor generator inside it, and the actual
MeshWeaver.Blazor.Viewsareexercised for the first time by #1268's next run.
Follow-up
#3223 — a superseded entry has no staleness check: once the pin moves onto an image built after the
move, that image's copy no longer defines the conflicting types but the assembly still exists, so the
"not carried" check does not fire. Durable form is a type-name overlap assertion; it needs the source's
declared type names and so belongs after the compilations are known, which is why it is not folded in
here. Linked from the code comment and the doc.
Doc:
Doc/Architecture/ModuleBuildArchitecturegains "Moving a type OUT of an image-shipped assemblyinto a module (the shadow set's blind spot)".
No What's New entry — internal build-process change, no user-visible effect.
Pairs-with: none — core lane change; MeshWeaver.Plugins#1268 is the first consumer. (Verified: this
diff removes no public type and no public member.)