Repository navigation
fix(ci): compile platform plugin modules once - #3933
Merged
Merged
Conversation
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
The container workflow lacks required image inputs, and the validation guard is overly restrictive.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
Updates core CD to compile four composed Plugins modules in one shared container workspace, with regression coverage and documentation.
Changes:
- Switches four module entries to container builds.
- Adds mutation-tested CI validation.
- Updates architecture and release documentation.
File summaries
| File | Summary |
|---|---|
src/MeshWeaver.Documentation/Data/WhatsNew/2026-09-10-platform-releases-pack-plugin-modules-once.md |
Adds the release note; no findings. |
src/MeshWeaver.Documentation/Data/Architecture/ModuleOwnedSiblingsRide.md |
Documents the topology; two nit findings (2 and 1 votes) incorrectly attribute separate Plugins changes to this PR. |
.github/workflows/main-cd.yml |
Enables container builds, but lacks required image and digest inputs, blocking artifact compilation (critical, 2 votes). |
.github/scripts/test-cd-steps.py |
Adds the CI guard; its hard-coded targets requirement rejects valid uniform accept sets (moderate, 1 vote). |
Review details
Suppressed comments (2)
.github/scripts/test-cd-steps.py:627
- The reusable packer permits any uniform
acceptset for a global container build; it rejects divergent sets atnode-repo-module-pack.yml:1194-1200, and its input contract explicitly allows values such astargets embedded-resource. Requiring the literaltargetshere makes the guard reject a valid shared-workspace catalog if all entries later need another acknowledged construct. Validatebuild: containerand, if needed, that container entries share one accept set instead of hard-coding this particular set.
return [
f"{entry.get('module') or '<unnamed>'} does not use the shared container workspace"
for entry in entries
if entry.get("build") != "container" or entry.get("accept") != "targets"
]
src/MeshWeaver.Documentation/Data/Architecture/ModuleOwnedSiblingsRide.md:242
- This paragraph attributes the Plugins-side
Blazor.ChatandNorthwindlane changes toMeshWeaver#3732, but neither change is present in this core-only PR and the description says that consolidation will land separately. Please attribute these statements to the actual Plugins change (or remove them until it lands) so the architecture history does not claim this PR changed another repository.
(MeshWeaver#3732). Merging the former `floor` and `rest` calls then took this assembly from TWO
builds to **ONE**. `Northwind`, the only remaining SDK entry that rode an in-repo module sibling
(`MeshWeaver.Maps`), moved into the same workspace in that change.
- Files reviewed: 4/4 changed files
- Comments generated: 2
- Review effort level: Lite
💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Contributor
Test Results (shard 1)496 tests 496 ✅ 37s ⏱️ Results for commit 846bfbc. ♻️ This comment has been updated with latest results. |
Contributor
Test Results (shard 0)254 tests 254 ✅ 2m 28s ⏱️ Results for commit 846bfbc. ♻️ This comment has been updated with latest results. |
Contributor
Test Results (shard 3)390 tests 390 ✅ 52s ⏱️ Results for commit 846bfbc. ♻️ This comment has been updated with latest results. |
Contributor
Test Results (shard 4)1 893 tests 1 893 ✅ 3m 24s ⏱️ Results for commit 846bfbc. ♻️ This comment has been updated with latest results. |
Contributor
Test Results (shard 2)1 250 tests 1 248 ✅ 3m 47s ⏱️ Results for commit 846bfbc. ♻️ This comment has been updated with latest results. |
Contributor
Test Results (shard 5) 6 files 6 suites 4m 51s ⏱️ Results for commit 846bfbc. ♻️ This comment has been updated with latest results. |
Contributor
Test Results 17 files 17 suites 16m 1s ⏱️ Results for commit 846bfbc. ♻️ This comment has been updated with latest results. |
rbuergi
force-pushed
the
codex/fix-release-one-workspace
branch
3 times, most recently
from
September 10, 2026 16:16
6fb4df5 to
b7908d2
Compare
rbuergi
force-pushed
the
codex/fix-release-one-workspace
branch
from
September 10, 2026 16:48
b7908d2 to
846bfbc
Compare
This was referenced Sep 10, 2026
rbuergi
marked this pull request as ready for review
September 10, 2026 18:43
This was referenced Sep 10, 2026
Merged
Merged
rbuergi
added a commit
that referenced
this pull request
Sep 11, 2026
…line that names how it was answered #3221 asks whether a sibling that is ITSELF another package's declared module should ride into a second bundle. It is a design question, and everything that made it hard has since moved: #3732/#3933 gave a publication ONE compilation, #3934/#3946 made a record's module entry a FLOOR rather than an MVID pin, and #3962/#3973 corrected the guard's stated reason. This settles it, measured. THE ANSWER — it rides, IFF the platform host does not already ship it. Being another package's declared module is not a reason to exclude it: exclusivity was never available (AI `requires [Store]` while Essentials, which DECLARES the assembly, `requires [AI, …]` — a package cycle) and would break a supported install. What makes the ride safe is not exclusivity but ONE BUILD per name per publication, asserted by the producer at compose time and again by the consumer over the whole sealed set. MEASURED 2026-09-11, with denominators: * Reference graph, MeshWeaver.Plugins main @ d6fa0338 — 79 index.json scanned, 37 declare a content.module, 148 csproj under src/. 18 of 37 declared modules transitively reference another package's declared module; 4 of 37 are RIDDEN (Markdown.Collaboration ×14, AI ×13, Maps ×4, Import ×1). That is the upper bound, not the answer. * What is actually PACKED — run 34563887604, success: job 103155487545 (MeshWeaver.AI): "closure: MeshWeaver.Markdown.Collaboration.dll does NOT ride — the platform ships it (measured off …/platform-refs)" => the exact pair this issue names no longer rides at all. job 103155487593 (MeshWeaver.Northwind.Application): "closure: MeshWeaver.Maps.dll RIDES — module-owned" "closure: MeshWeaver.Import.dll RIDES — module-owned" => the shape IS live today, on two names, in one bundle of 37. * The verdict on the composed sets — Plugins publish-bake 103160836446 and gate shard 103159059393; MeshWeaver.Crm publish-bake 103166073109, which composes ai.module.nupkg and essentials.module.nupkg side by side (the pair the 2026-09-08 hold named): "module set: 5 … 0 carried by more than one bundle, 0 carried at more than one BUILD" / "module set: 4 … 0 … 0" CORRECTION — the page said the refusal was INERT until each satellite moves its pin. Measured over every `uses: Systemorph/MeshWeaver/.github/workflows/…` line in all six node repos: FIVE consume node-repo-gate.yml and node-repo-publish-bake.yml at @main (Plugins, Reinsurance, SocialMedia, Manufacturing, Crm), so they took the refusal the hour it merged; only MeshWeaver.Education pins a sha (67cbbe0…) and the verdict is absent from that file at that sha. Both directions off the LOGS, not inferred from the ref: Plugins run 34563887604 and Crm run 34567369849 print the verdict line; Education's publish-bake for run 34487980275 composes the same four bundles and prints none. THE ONE CODE CHANGE — node-repo-module-pack.yml's witness line printed its path TWICE ("measured off /tmp/app/tmp/app"), because `${PLATFORM_APP:+measured off $PLATFORM_APP}${PLATFORM_APP:-declared by …}` expands `:-` to the VALUE when the variable is set. That is the branch that matters — it is the one saying the answer came from the image rather than from a hand-maintained list — and it is quoted verbatim in the page above. Stated once in a `witness` variable instead. CONTROL, both directions, executed against the REAL workflow file (the two assignment lines and the echo extracted by content, never copied): before: MEASURED … the platform ships it (measured off /tmp/app/tmp/app); … ← defect DECLARED … the platform ships it (declared by src/platform-shipped.txt); … after: PASS MEASURED … (measured off /tmp/app) [asserts "/tmp/app/tmp/app" ABSENT] PASS DECLARED … (declared by src/platform-shipped.txt) [asserts "measured off" ABSENT] VERIFIED YAML parses — node-repo-module-pack 6 jobs check-workflow-timeouts.py 84 jobs, 2 exempt, 0 violations check-workflow-shell.py 0 live findings check-workflow-yaml-keys.py 34 files, 0 violations check-abbreviated-shas.py 34 files, 0 abbreviations check-workflow-permission-pairing.py 2 pairs, 0 violations MeshWeaver.Documentation -c Release -warnaserror 0 Error(s), 0 Warning(s) MeshWeaver.Documentation.Test -c Release -warnaserror 0 Error(s), 0 Warning(s) DocumentationLinkIntegrityTest + ArchitectureTopicMapTest 5/5 passed (doclinks.trx) Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
rbuergi
added a commit
that referenced
this pull request
Sep 11, 2026
…line that names how it was answered #3221 asks whether a sibling that is ITSELF another package's declared module should ride into a second bundle. It is a design question, and everything that made it hard has since moved: #3732/#3933 gave a publication ONE compilation, #3934/#3946 made a record's module entry a FLOOR rather than an MVID pin, and #3962/#3973 corrected the guard's stated reason. This settles it, measured. THE ANSWER — it rides, IFF the platform host does not already ship it. Being another package's declared module is not a reason to exclude it: exclusivity was never available (AI `requires [Store]` while Essentials, which DECLARES the assembly, `requires [AI, …]` — a package cycle) and would break a supported install. What makes the ride safe is not exclusivity but ONE BUILD per name per publication, asserted by the producer at compose time and again by the consumer over the whole sealed set. MEASURED 2026-09-11, with denominators: * Reference graph, MeshWeaver.Plugins main @ d6fa0338 — 79 index.json scanned, 37 declare a content.module, 148 csproj under src/. 18 of 37 declared modules transitively reference another package's declared module; 4 of 37 are RIDDEN (Markdown.Collaboration ×14, AI ×13, Maps ×4, Import ×1). That is the upper bound, not the answer. * What is actually PACKED — run 34563887604, success: job 103155487545 (MeshWeaver.AI): "closure: MeshWeaver.Markdown.Collaboration.dll does NOT ride — the platform ships it (measured off …/platform-refs)" => the exact pair this issue names no longer rides at all. job 103155487593 (MeshWeaver.Northwind.Application): "closure: MeshWeaver.Maps.dll RIDES — module-owned" "closure: MeshWeaver.Import.dll RIDES — module-owned" => the shape IS live today, on two names, in one bundle of 37. * The verdict on the composed sets — Plugins publish-bake 103160836446 and gate shard 103159059393; MeshWeaver.Crm publish-bake 103166073109, which composes ai.module.nupkg and essentials.module.nupkg side by side (the pair the 2026-09-08 hold named): "module set: 5 … 0 carried by more than one bundle, 0 carried at more than one BUILD" / "module set: 4 … 0 … 0" CORRECTION — the page said the refusal was INERT until each satellite moves its pin. Measured over every `uses: Systemorph/MeshWeaver/.github/workflows/…` line in all six node repos: FIVE consume node-repo-gate.yml and node-repo-publish-bake.yml at @main (Plugins, Reinsurance, SocialMedia, Manufacturing, Crm), so they took the refusal the hour it merged; only MeshWeaver.Education pins a sha (67cbbe0…) and the verdict is absent from that file at that sha. Both directions off the LOGS, not inferred from the ref: Plugins run 34563887604 and Crm run 34567369849 print the verdict line; Education's publish-bake for run 34487980275 composes the same four bundles and prints none. THE ONE CODE CHANGE — node-repo-module-pack.yml's witness line printed its path TWICE ("measured off /tmp/app/tmp/app"), because `${PLATFORM_APP:+measured off $PLATFORM_APP}${PLATFORM_APP:-declared by …}` expands `:-` to the VALUE when the variable is set. That is the branch that matters — it is the one saying the answer came from the image rather than from a hand-maintained list — and it is quoted verbatim in the page above. Stated once in a `witness` variable instead. CONTROL, both directions, executed against the REAL workflow file (the two assignment lines and the echo extracted by content, never copied): before: MEASURED … the platform ships it (measured off /tmp/app/tmp/app); … ← defect DECLARED … the platform ships it (declared by src/platform-shipped.txt); … after: PASS MEASURED … (measured off /tmp/app) [asserts "/tmp/app/tmp/app" ABSENT] PASS DECLARED … (declared by src/platform-shipped.txt) [asserts "measured off" ABSENT] VERIFIED YAML parses — node-repo-module-pack 6 jobs check-workflow-timeouts.py 84 jobs, 2 exempt, 0 violations check-workflow-shell.py 0 live findings check-workflow-yaml-keys.py 34 files, 0 violations check-abbreviated-shas.py 34 files, 0 abbreviations check-workflow-permission-pairing.py 2 pairs, 0 violations MeshWeaver.Documentation -c Release -warnaserror 0 Error(s), 0 Warning(s) MeshWeaver.Documentation.Test -c Release -warnaserror 0 Error(s), 0 Warning(s) DocumentationLinkIntegrityTest + ArchitectureTopicMapTest 5/5 passed (doclinks.trx) Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This was referenced Sep 11, 2026
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
Paired change: Systemorph/MeshWeaver.Plugins#1614 consolidates the Plugins repository's own two module-pack calls.
Why
The platform release for 80b3b63 failed closed because AI and Essentials carried byte-distinct builds of MeshWeaver.Markdown.Collaboration. Both were compiled from the same Plugins commit, but each legacy SDK entry rebuilt its transitive siblings in a different checkout. Pinning version attributes cannot converge deterministic MVIDs across those source paths.
The first combined Plugins workspace then exposed a separate compiler defect: it scheduled Northwind Application → Model → Domain correctly but supplied Roslyn only the direct Model output, so valid source naming a Domain type failed with CS0234/CS0246. The SDK supplies that transitive reference set.
Verification
Refs #3732.