Repository navigation
fix(lane): the global build asserts its OWN postcondition — one emitted assembly per selected module (Plugins#1051) - #2978
Conversation
…ed assembly per selected module (Plugins#1051) `build-workspace` reported success while emitting nothing for modules the pack matrix then demanded, so the disagreement surfaced N jobs downstream as seven red bundle jobs on a PR whose whole diff was one XML doc comment (Plugins#1051, run 33480755857). "The compiler did not throw" was being read as "the selected set was emitted"; they are different claims and nothing compared them. The producer now checks, before it uploads, exactly what every consumer demands — per SELECTED container entry: a non-empty `<Module>/<Module>.dll`, a `<Module>.closure.txt`, and a build log carrying the `platform AssemblyVersion` the binding-identity check reads. Fed `select.outputs.modules`, the same enumerator the matrix expands: a postcondition derived from a second enumerator is the defect one level up. * No `if:` on the step — the sdk-only case is handled inside the script (asserts nothing, says so) rather than by a condition that renders as a grey tick. * Its self-test runs FIRST and fails the job. The headline case is a NON-EMPTY workspace missing one selected module — the case a glob passes, and #1051 verbatim. * The platform checkout lands AFTER the container build so `/repo:ro` stays exactly the content repository. * The pack job's downstream message is corrected: with the postcondition in place, a missing DLL there means the WRONG workspace arrived (a sibling call's artifact, Plugins#1077), not a build that skipped the module. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
This PR strengthens the node-repo module-pack CI lane by making the global workspace build assert its own postcondition (that it emitted the artifacts the pack matrix will consume), and documents the principle in the build-architecture docs.
Changes:
- Add a
build-workspacepostcondition step innode-repo-module-pack.ymlthat validates, per selectedbuild: containerentry, the presence of the module DLL, closure manifest, and a readable platformAssemblyVersionin the build log. - Introduce
.github/scripts/workspace-build-verify.pyto perform the above verification (including a self-test). - Update
Doc/Architecture/ModuleBuildArchitecturewith guidance that each stage must assert its own postcondition.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
src/MeshWeaver.Documentation/Data/Architecture/ModuleBuildArchitecture.md |
Documents the “assert your own postcondition” principle and ties it to the lane’s build→pack contract. |
.github/workflows/node-repo-module-pack.yml |
Adds an unconditional postcondition check before uploading the workspace artifact; clarifies downstream error messaging. |
.github/scripts/workspace-build-verify.py |
New verification script used by the workflow to fail the build job if selected container modules were not emitted as required. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| entries = container_entries(modules) | ||
| code, errors, notes = verify(entries, Path(args.output)) |
Test Results (shard 0)216 tests 216 ✅ 2m 16s ⏱️ Results for commit a7b1368. |
Test Results (shard 3)578 tests 578 ✅ 51s ⏱️ Results for commit a7b1368. |
Test Results (shard 1)419 tests 419 ✅ 1m 10s ⏱️ Results for commit a7b1368. |
Test Results (shard 5)1 126 tests 934 ✅ 1m 7s ⏱️ Results for commit a7b1368. |
Test Results (shard 4)1 640 tests 1 640 ✅ 1m 31s ⏱️ Results for commit a7b1368. |
Test Results (shard 2)507 tests 507 ✅ 1m 32s ⏱️ Results for commit a7b1368. |
Test Results4 486 tests 4 294 ✅ 8m 28s ⏱️ Results for commit a7b1368. |
The defect
Module bundles / Build the workspace (one global build)reported success while emittingnothing for modules the pack matrix then demanded. The disagreement surfaced N jobs downstream:
Seven bundle jobs, on a PR whose entire diff was one XML doc comment (Plugins#1051, run
33480755857). The message was accurate and one job too late: the stage that knew the selection
was the green one, and each stage that discovered the gap knew only its own module.
Root cause of the reporting gap: a build's exit code answers "did the compiler throw?", and that
was being read as "the selected set was emitted". Two enumerators — the projects handed to the
build and the modules the matrix expands — were free to disagree, and nothing compared them.
(The particular disagreement in that run was the run-wide
workspace-buildartifact collisionfixed by #2958 / Plugins#1077. This change is the level above it: the producer can no longer be
green while short, whatever the cause.)
The change
build-workspacenow checks, before it uploads anything, exactly what its consumers demand,per SELECTED container entry:
<Module>/<Module>.dll<Module>.closure.txtplatform AssemblyVersionin the build logThree properties make it a postcondition rather than a second opinion:
needs.select.outputs.modules— the JSON thepack matrix expands — not a re-derivation and not a glob of the output. A postcondition derived
from a different enumerator is the original defect one level up.
assemblies side by side, so "there are DLLs in there" is true in exactly the failure being
diagnosed.
if:on the step, and its self-test runs first and fails the job. Theselection-with-no-container-entries case is handled inside the script (asserts nothing, says
so) rather than by a condition that renders as a grey tick indistinguishable from a pass.
It deliberately never asks the reverse question — the workspace carries in-tree dependencies of
selected modules beside them, so "emitted but not selected" is normal.
Also here:
-v $GITHUB_WORKSPACE:/repo:rostays exactly the content repository and nothing else. (The pack job checks core out up front
because it has no such mount.)
there means the wrong workspace arrived (a sibling call's artifact — Plugins#1077 — or a
torn download), not a build that skipped the module. It now says so and names the lane key.
Doc/Architecture/ModuleBuildArchitecturegains "Every stage asserts its OWN postcondition —never the next one's", with the generalisation for any stage added to this lane.
Proven able to fail
An assertion that cannot fail is the defect this PR is about, one level up. Both arms, same
command, on a fixture reproducing #1051 exactly — the FLOOR call's workspace (2 modules, non-empty,
2 DLLs, closure manifests, a good build log) handed to the REST call's selection:
RED arm
(the
sdkentry in the same selection,MeshWeaver.Testing, is correctly not blamed)GREEN control arm — same command, after the three assemblies are added:
Self-test — 2 green-run assertions, 8 mutations that must go red (headline: a NON-EMPTY
workspace missing one selected module), 4 cases that must NOT fire (sdk entries, a defaulted
buildkey, an unselected in-tree dependency, an empty selection):Also verified locally:
check-workflow-shell.py --self-testall cases pass and the tree scan is0 livefindings; the workflow parses as YAML;MeshWeaver.Documentation.Testbuilds-c Release -warnaserrorwith0 Error(s)andDocumentationLinkIntegrityTestpasses 1/1.Follow-through
MeshWeaver.Pluginspicks this up with a pin bump on its twonode-repo-module-pack.ymlcalls;that PR carries
Closes Systemorph/MeshWeaver.Plugins#1051.🤖 Generated with Claude Code