Repository navigation
CD builds the portal host from plugins, and asserts every delivery leg ran - #2364
Merged
Merged
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
This PR strengthens the main-cd workflow’s post-conditions by ensuring that when the gate decides to publish, every delivery “leg” job actually ran and concluded successfully, and by making the run outcome more visible in the UI.
Changes:
- Expands
delivery-verdictjob dependencies to include all delivery legs (not just[gate, promote]). - Adds a “positive check” that fails a publishing run if any required leg is not
success(includingskipped,cancelled, or empty). - Adds a step-summary line that labels runs as PUBLISHED, NO-OP, or NO PUBLISH for quick visual triage.
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
rbuergi
changed the title
Contributor
Author
|
A second, sharper instance — this one has a leg that FAILED, not merely skipped, while The example in the issue body is an all-skipped run. Run 32964902603 (main So on a run that decided to publish, tried, and failed, the job literally named "CD delivered, or had a good reason not to" concluded Two things this adds to the case for the fix as specified:
Live effect while this is open: no portal image has published since |
Contributor
Test Results (shard 3) 9 files 9 suites 5m 15s ⏱️ Results for commit be806e8. ♻️ This comment has been updated with latest results. |
Contributor
Test Results (shard 0)599 tests 595 ✅ 15m 55s ⏱️ Results for commit be806e8. ♻️ This comment has been updated with latest results. |
Contributor
Test Results (shard 5) 9 files 9 suites 5m 17s ⏱️ Results for commit be806e8. ♻️ This comment has been updated with latest results. |
Contributor
Test Results (shard 1)2 048 tests 1 855 ✅ 8m 26s ⏱️ Results for commit be806e8. ♻️ This comment has been updated with latest results. |
Contributor
Test Results (shard 2)2 832 tests 2 729 ✅ 9m 31s ⏱️ Results for commit be806e8. ♻️ This comment has been updated with latest results. |
Contributor
Test Results (shard 4)2 142 tests 2 140 ✅ 14m 32s ⏱️ Results for commit be806e8. ♻️ This comment has been updated with latest results. |
Contributor
Contributor
Author
Review point taken — and the PR is now narrowed to the assertion onlyThe NO-OP label: correct, and it would have done the opposite of what it was for. An empty Every branch verified under bash: PR-event → NO-OP · real delivery → PUBLISHED · publish-then-leg-failed → DELIVERY FAILED · declined → NO PUBLISH · gate errored/cancelled → INCONCLUSIVE. The repoint is reverted. What ships in this PR is the part that is unambiguous: a run that decides to publish must have run every leg, and the summary must say what happened. |
rbuergi
changed the title
rbuergi
changed the title
rbuergi
force-pushed
the
fix/cd-assert-every-leg-ran
branch
from
August 26, 2026 12:55
efcbe8b to
f505e2e
Compare
Raised by the maintainer watching main-cd runs complete in 0 minutes and report success: "no cd run should complete in 0 minutes" and "we need a check that everything was actually run". Both are the same observation — a green tick that is indistinguishable from work. What was already handled: delivery-verdict classifies the gate's VERDICT and fails on an empty one, on a stuck reconcile, and on a red-main push. Those guards are good and stay. What was NOT: nothing checked that a run which DECIDED to publish actually executed the nine jobs that carry out a delivery. delivery-verdict needed only [gate, promote], so it could not see the other seven — and a job that `skipped` renders the same grey/green tick as one that ran. A publish missing a leg reported success. So: every leg is now a NEED, and a publishing run asserts each concluded 'success' by name. `skipped`, `cancelled`, `failure` and EMPTY all fail, and the error names the offending jobs. There is deliberately no "empty means fine" branch — that assumption is exactly what let the verdict pass on an empty gate output (run 32914425222, documented in this file). Also a step-summary line saying plainly whether the run PUBLISHED, was a NO-OP, or declined — because a 3-second success and a 20-minute delivery are otherwise identical at a glance, which is how a merged fix sat unshipped while its instance waited for it. On the 0-minute runs themselves: those are CD firing on a PR's Build-and-Test (it triggers on every completed run and the gate is conditioned to push-on-main), so they are a legitimate no-op and are NOT made to fail — erroring on every PR would train everyone to ignore the alarm. They are now labelled ⚪ NO-OP in the summary instead, which is the part that was actually missing: not a failure, but a visible difference. Verified both directions under bash: all-success PASSES; one leg skipped, cancelled, or empty each FAIL naming the leg.
#2293 deleted memex/aspire/Memex.Portal.Distributed from this repo. Three workflows still named it, so main-cd failed at its very first step for every commit on main: VERSION=$(dotnet msbuild memex/aspire/Memex.Portal.Distributed/... -getProperty:Version) ##[error]Process completed with exit code 1 promote is all-or-nothing, so NOTHING published. Production stayed up on the images it already had — stable, but frozen, including on the deletion itself. Repointed in main-cd.yml, edge-images.yml and release-images.yml: 1. Each checks out MeshWeaver.Plugins (path plugins-repo) and publishes the host from there, resolving the platform through -p:MeshWeaverRoot=${{ github.workspace }}. 2. `ref: ${{ vars.MW_PLUGINS_REF || 'main' }}` — default main, overridable by a repo variable, mirroring the plugins lane's own MW_PLATFORM_REF. This does NOT decouple "which plugins commit shipped with which core commit"; it makes that coupling steerable, so pinning to a known-good plugins commit during an incident is a variable change rather than editing a workflow under pressure. 3. VERSION now reads src/MeshWeaver.Mesh.Contract — a project that STAYS here. It must be computed in THIS repo: the same property resolves to 1.0.0 in the plugins tree against 3.0.0-rc8.ci.<n> here, and an install's self-updater compares that monotonically, so a plugins-derived version would roll every install BACKWARDS past VersionSelect's monotonic-build-number assumption. Verified any core project yields the identical value. It stays `-getProperty:` rather than a grep of Directory.Build.props, deliberately: a regex keeps returning a plausible, well-formed, WRONG number after the definition moves, and that number gets published. And it now REFUSES on empty instead of continuing — an empty version becomes `:` or `latest` downstream, which is the failure where a tag exists and points at the wrong thing. Root cause worth stating: this was a known-open item ("CD repoint for memex-portal-ai's build source") that was not sequenced before the deletion merged. The question "what does the world look like BETWEEN the two merges?" was asked of the COMPILE pairing and verified three ways; it was never asked of the delivery pipeline. Right question, one system too narrow. Verified: all three files parse; every `dotnet publish` of the host resolves to plugins-repo/src/... and carries MeshWeaverRoot; no workflow still reads Version from a moved project; no reference to the deleted path remains anywhere under .github/workflows.
This reverts commit ea9b9c9.
Caught in review: the summary step treated an empty `needs.gate.outputs.reason` as a legitimate NO-OP. But an empty reason is ALSO what a gate that errored, was cancelled, or produced no outputs looks like -- and the step runs under `if: always()`, so a broken run could end up stamped with the one label that says nothing was due. The label meant to make the two cases distinguishable would have merged them. Now keyed on `needs.gate.result == 'skipped'`, which is the only state that genuinely means "no delivery was due". Every other empty-verdict shape is labelled INCONCLUSIVE and sits under a red run. PUBLISHED additionally requires promote=success: a run that decided to publish and then lost a leg is DELIVERY FAILED, which is exactly what this morning's run was and what the old label would have called a success. Verified every branch under bash (Actions' shell): PR-event → NO-OP; real delivery → PUBLISHED; publish-then-leg-failed → DELIVERY FAILED; declined → NO PUBLISH; gate errored or cancelled → INCONCLUSIVE. The repoint that shared this PR is reverted (previous commit): PlatformBakeLaneGuard is right that main-cd must check out no other repository, and the portal image's build home is an architectural cutover to be decided, not a hotfix.
This reverts commit 40d25f4.
…epo input Maintainer decision, 2026-08-26, verbatim: "we want to run full plugin build in main memex build" / "plugins should not have a lane in this sense" / "only single plugins on pr, and dependencies". That supersedes the ADOPT model this guard encoded -- plugins arriving pre-built from a bundle their own lane published -- with: the platform's main build builds everything (core, every plugin, the portal host, the bake) and publishes it atomically under one framework identity. It removes the #1814 bake-identity class by construction, and it is what the portal host's move to MeshWeaver.Plugins already forced: without that checkout, main-cd cannot build the deployment image at all (measured today: every push run failed at the version step and nothing published). The "no `repository:` input" assertion is therefore replaced, not deleted, and the replacement is POSITIVE: the set of checked-out repositories in main-cd is exactly ["Systemorph/MeshWeaver.Plugins"]. Present, and the only one. A third would be a new decision rather than an extension of this one. The other two assertions -- bake the Doc tree, never bake the samples trees -- stand unchanged. Verified both directions: passes with the plugins checkout present; with a second `repository:` planted it fails naming both ("Actual: [Plugins, Evil]"), so it refuses exactly what it is meant to refuse and cannot pass vacuously. The doc comment records the decision and date so the next reader does not "restore" the old rule as a fix.
rbuergi
force-pushed
the
fix/cd-assert-every-leg-ran
branch
from
August 26, 2026 13:17
f505e2e to
be806e8
Compare
rbuergi
added a commit
that referenced
this pull request
Aug 26, 2026
…form Maintainer decision 2026-08-26: "we want to run full plugin build in main memex build"; "plugins should not have a lane in this sense"; "only single plugins on pr, and dependencies". This is the main-cd half. (The plugins-PR half already exists: ci.yml narrows to the changed modules and their dependency closure via affected-modules.py.) publish-bake now, after baking the platform's own content against the mw-plugin-test image this run built: 1. checks out MeshWeaver.Plugins (ref: vars.MW_PLUGINS_REF || main -- the same steerable pin portal-image uses; PlatformBakeLaneGuard asserts it is the only foreign repository); 2. BAKES every plugin against the SAME image (compiler-driven, no mesh), with the same postconditions as the platform bake: an identity file and at least one bundle, or red; 3. GATES the bake -- renders and executes every Tests area on the baked bytes; 4. ASSERTS the plugin bake's framework identity equals the platform bake's. Same run, same image, so equal by construction -- asserted anyway, because two bakes that disagree would publish bundles no portal can adopt together (#1814's class); 5. PUBLISHES the bundles under source key `plugins` (what portals already adopt by), sealed, with RELEASE_VERSION REQUIRED and equal to the version THIS run computed. Item 5 is the one that matters beyond wiring. A plugin bundle's version used to come from manifest.lock while its bytes came from a bake against a pinned image dozens of publishes old -- Plugins#695, and the route by which an already-fixed defect kept shipping (#2356). Here the version is minted by the run that produced the bytes, and the publish refuses if it is absent. The property, verbatim from the review that named it: "the version this bundle is stamped with was computed by the run that produced its bytes." What this retires: plugins' own publish lane (portal-ai-image.yml, never run; the publish half of publish-bake), the repository_dispatch rebake (dead since 2026-08-22, #2235), and the MW_IMAGE_DIGEST pin's drift. What it does not change: plugins PRs still gate only what they touch. Stacked on #2364 (the portal-host repoint + every-leg assertion) because both edit main-cd.yml; rebases onto main once that lands. PlatformBakeLaneGuard passes 9/9 against this file; YAML parses; publish-bake is 15 steps. Not exercised end to end until it runs on main -- there is no local way to run publish-bake. The pieces it composes (the tester image, bake-then-gate's two calls, publish-bake-bundles.sh) are each already exercised in this same job or in dotnet-test.yml's plugin-gate.
rbuergi
added a commit
that referenced
this pull request
Aug 26, 2026
…form Maintainer decision 2026-08-26: "we want to run full plugin build in main memex build"; "plugins should not have a lane in this sense"; "only single plugins on pr, and dependencies". This is the main-cd half. (The plugins-PR half already exists: ci.yml narrows to the changed modules and their dependency closure via affected-modules.py.) publish-bake now, after baking the platform's own content against the mw-plugin-test image this run built: 1. checks out MeshWeaver.Plugins (ref: vars.MW_PLUGINS_REF || main -- the same steerable pin portal-image uses; PlatformBakeLaneGuard asserts it is the only foreign repository); 2. BAKES every plugin against the SAME image (compiler-driven, no mesh), with the same postconditions as the platform bake: an identity file and at least one bundle, or red; 3. GATES the bake -- renders and executes every Tests area on the baked bytes; 4. ASSERTS the plugin bake's framework identity equals the platform bake's. Same run, same image, so equal by construction -- asserted anyway, because two bakes that disagree would publish bundles no portal can adopt together (#1814's class); 5. PUBLISHES the bundles under source key `plugins` (what portals already adopt by), sealed, with RELEASE_VERSION REQUIRED and equal to the version THIS run computed. Item 5 is the one that matters beyond wiring. A plugin bundle's version used to come from manifest.lock while its bytes came from a bake against a pinned image dozens of publishes old -- Plugins#695, and the route by which an already-fixed defect kept shipping (#2356). Here the version is minted by the run that produced the bytes, and the publish refuses if it is absent. The property, verbatim from the review that named it: "the version this bundle is stamped with was computed by the run that produced its bytes." What this retires: plugins' own publish lane (portal-ai-image.yml, never run; the publish half of publish-bake), the repository_dispatch rebake (dead since 2026-08-22, #2235), and the MW_IMAGE_DIGEST pin's drift. What it does not change: plugins PRs still gate only what they touch. Stacked on #2364 (the portal-host repoint + every-leg assertion) because both edit main-cd.yml; rebases onto main once that lands. PlatformBakeLaneGuard passes 9/9 against this file; YAML parses; publish-bake is 15 steps. Not exercised end to end until it runs on main -- there is no local way to run publish-bake. The pieces it composes (the tester image, bake-then-gate's two calls, publish-bake-bundles.sh) are each already exercised in this same job or in dotnet-test.yml's plugin-gate.
This was referenced Aug 26, 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.
Raised while watching
main-cdruns complete in 0 minutes and report success: "no cd run should complete in 0 minutes" and "we need a check that everything was actually run".What was already covered
delivery-verdictclassifies the gate's verdict and fails on an empty one, a stuck reconcile, and a red-main push. Those guards are good and unchanged — they were added today after run32914425222passed on an empty gate output.What wasn't
Nothing checked that a run which decided to publish actually executed the nine jobs that carry out a delivery.
delivery-verdictneeded only[gate, promote], so seven legs were invisible to it — and askippedjob renders the same grey/green tick as one that ran.The change
Every leg becomes a
needs, and a publishing run asserts each concludedsuccessby name.skipped,cancelled,failureand empty all fail, with the offending jobs named. No "empty means fine" branch — that assumption is what let the verdict pass on an empty gate output in the first place.Plus a step-summary line saying plainly whether the run PUBLISHED, was a NO-OP, or declined. A 3-second success and a 20-minute delivery are otherwise identical at a glance, which is how a merged fix sat unshipped while its instance waited for it.
On the 0-minute runs themselves
Those are CD firing on a PR's Build-and-Test — it triggers on every completed run while the gate is conditioned to push-on-main. That is a legitimate no-op, so it is not made to fail: erroring on every PR would train everyone to ignore the alarm, which is worse than the ambiguity. They are now labelled
⚪ NO-OPin the summary. The missing thing was a visible difference, not a failure.Verified
Both directions under bash: all-success passes; one leg
skipped,cancelled, or empty each fails, naming the leg.No What's New entry: CI-internal.