Repository navigation
State the framework identity as the platform commit instead of deriving it per job (#3308) - #3339
Conversation
…ng it per job (#3308) Core CD is RED on this today: run 33950389008 (2026-09-05 07:05Z, four entries, ONE platform commit, every leg `plan: BUILD`) stated THREE identities, and #3310's agreement gate refused the lane — MeshWeaver.Maps 0051e7210e4144a6a18620a36e629d84 (1 compiler build) MeshWeaver.Payments.Stripe 0051e7210e4144a6a18620a36e629d84 (1 compiler build) MeshWeaver.Markdown.Collaboration 6553ea371eab49e4afdd0af20ea7ac59 (2 compiler builds) MeshWeaver.AI 395909c22d3042d4b23fb055337db63b (2 compiler builds) The split is exactly the reference graph. MeshWeaver.AI and MeshWeaver.Markdown.Collaboration reach MeshWeaver.Compiler transitively, so the MODULE build compiles it FIRST — into the same output path, under the module's own global properties — and the anchor build that follows keeps those bytes. Maps and Payments.Stripe never build it, so their anchor build compiles it clean and they agree with each other. That is why the two previous cures could not work. #3293 moved the anchor build into the platform tree; #3306 removed the `-p:Version` polluting it. Both fixed the ANCHOR build, and the pollution arrives from the MODULE build upstream, which is entitled to its own version. And a derived MVID could not be matched even if it were stable. A portal image is built with -p:CIRun=true, so AddCommitHashMetadata stamps AssemblyMetadata("MeshWeaverFrameworkIdentity", "g<sha>") and FrameworkIdentity.ReadIdentity PREFERS that stamp over the MVID. A consumer reads `g<sha>`; a source-built bundle stated 32-hex. Never equal, by construction — so #3154's (version, identity) comparison could only ever answer "identity could not be checked", which is the exact state #3211 exists to prevent. So the arm stops deriving and STATES it: the platform IS this checkout, its commit IS what a consumer reads, and `--framework-mvid` is the packer's own flag for that (its own error text names `g<sha>` as a legal token). One identity per platform commit, identical for every entry, and no compiler build per matrix job at all. The refs arm is unchanged: there the DLL genuinely IS the platform, already stamped, and already yields `g<sha>` — so this makes the two arms agree rather than inventing a shape. An unreadable platform commit is RED, never a fallback. RECIPE_VERSION 2 → 3: bundles packed under recipe 2 state an identity no consumer can match, so they must be repacked rather than reused. The guard moves with its subject and gets STRONGER: instead of pinning the location of an anchor build, it now forbids `MeshWeaver.Compiler.csproj` in the pack step at all — the mechanism all three previous cures left in place. Mutation- proved: reintroducing that build turns the guard red. Verified: Documentation.Test 0 Error(s) Release -warnaserror; the identity guards 5/5; node-repo-pack-verify.py --self-test passes. Closes #3308 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
🟡 Changes recommended
The workflow’s SHA validation currently accepts short hex prefixes (8+ chars) even though the identity format is documented as g<full-commit-sha> (40 hex), which can reintroduce mismatched identities.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
This PR fixes the reusable module-pack workflow’s “from-source” arm so it states the framework identity as the platform commit (g<sha>) instead of deriving it from a per-matrix-job build artifact, eliminating identity skew across matrix entries and avoiding an extra compiler build per job.
Changes:
- Update
.github/workflows/node-repo-module-pack.ymlto computeidentity=(--framework-mvid "g$sha")from the checkout commit when no platform image (REFS) is pinned, and pass that identity to the pack tool. - Strengthen the documentation test guard to ratchet against reintroducing per-job
MeshWeaver.Compiler.csprojbuilds in the pack step. - Bump the module build key
RECIPE_VERSIONto force repacking rather than reusing bundles produced under the previous (unmatchable) identity recipe.
File summaries
| File | Description |
|---|---|
test/MeshWeaver.Documentation.Test/ModuleIdentityPublishGuard.cs |
Updates the guard expectations to require “state identity” behavior and forbid compiler builds in the pack step. |
.github/workflows/node-repo-module-pack.yml |
Changes the from-source arm to state --framework-mvid g<sha> and removes --graph-dll derived identity in that arm. |
.github/scripts/module-build-key.py |
Bumps RECIPE_VERSION to invalidate/repack older bundles under the new identity recipe. |
Review details
- Files reviewed: 3/3 changed files
- Comments generated: 1
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| sha="$(git -C "$GITHUB_WORKSPACE/meshweaver" rev-parse HEAD 2>/dev/null || true)" | ||
| case "$sha" in | ||
| [0-9a-f][0-9a-f][0-9a-f][0-9a-f][0-9a-f][0-9a-f][0-9a-f][0-9a-f]*) : ;; | ||
| *) | ||
| echo "::error::cannot read the platform commit at \$GITHUB_WORKSPACE/meshweaver (git rev-parse HEAD gave '${sha:-<nothing>}'). No platform image is pinned, so the checkout IS the platform and its commit IS the identity every bundle of this run must state (#3308). Nothing here may guess it, so the pack stops." | ||
| exit 1 | ||
| ;; | ||
| esac | ||
| identity=(--framework-mvid "g$sha") |
Test Results (shard 1)475 tests 475 ✅ 39s ⏱️ Results for commit 8286112. ♻️ This comment has been updated with latest results. |
Test Results (shard 3)369 tests 367 ✅ 42s ⏱️ Results for commit 8286112. ♻️ This comment has been updated with latest results. |
Test Results (shard 0)251 tests 251 ✅ 2m 0s ⏱️ Results for commit 8286112. ♻️ This comment has been updated with latest results. |
Test Results (shard 5) 4 files 4 suites 1m 42s ⏱️ Results for commit 8286112. ♻️ This comment has been updated with latest results. |
Test Results (shard 4)2 315 tests 2 315 ✅ 2m 36s ⏱️ Results for commit 8286112. ♻️ This comment has been updated with latest results. |
Test Results (shard 2)588 tests 396 ✅ 2m 56s ⏱️ Results for commit 8286112. ♻️ This comment has been updated with latest results. |
Test Results 15 files 15 suites 10m 38s ⏱️ Results for commit 8286112. ♻️ This comment has been updated with latest results. |
…stderr or the exit status check-workflow-shell.py caught it and is right: `sha="$(git … 2>/dev/null || true)"` swallows BOTH git's message and its exit status, so a checkout without .git would have reached the validation as a blank string with no trace of why. The refusal was still fail-closed, but the log could not have said what went wrong. Now `if ! sha="$(git … rev-parse HEAD)"` — `set -e` stays out of the way until the explanation is printed, and git's own stderr lands in the log. The shape check is also a real 40-hex assertion rather than an 8-character prefix glob. check-workflow-shell.py run locally: 0 live findings (the 1 remaining is the pre-existing allow-listed entry). Documentation.Test 0 Error(s) 0 Warning(s) Release -warnaserror; ModuleIdentityPublishGuard 5/5. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The two reds, attributed1.
|
This is red on core
mainright nowRun 33950389008 (2026-09-05 07:05Z) — four entries, one platform commit, every leg
plan: BUILD(no reuse) — stated three identities, and #3310's agreement gate refused the lane.Core CD has failed on it repeatedly today; the one green run in between skipped the module-pack
job entirely, so it attested nothing.
MeshWeaver.Maps0051e7210e4144a6a18620a36e629d84MeshWeaver.Payments.Stripe0051e7210e4144a6a18620a36e629d84MeshWeaver.Markdown.Collaboration6553ea371eab49e4afdd0af20ea7ac59MeshWeaver.AI395909c22d3042d4b23fb055337db63bWhy the two previous cures could not have worked
The split is exactly the reference graph, and the lane's own comment already says so:
MeshWeaver.AIandMeshWeaver.Markdown.CollaborationreachMeshWeaver.Compilertransitively.So the module build compiles it first — into the same output path, under the module's own
global properties — and the anchor build that follows keeps those bytes. Maps and Payments.Stripe
never build it, so their anchor build compiles it clean and they agree with each other.
-p:Versionpolluting that build — correct property, still per job.Both fixed the anchor build. The pollution arrives from the module build upstream, which is
entitled to its own version. No property removable from the anchor build can reach it.
And determinism alone would not have been enough
A portal image is built with
-p:CIRun=true, soAddCommitHashMetadatastampsAssemblyMetadata("MeshWeaverFrameworkIdentity", "g<sha>"), andFrameworkIdentity.ReadIdentityprefers that stamp over the MVID. A consumer reads
g<sha>; a source-built bundle stated32-hex. Never equal, by construction — so #3154's
(version, identity)comparison could onlyever answer "identity could not be checked", which is the exact state #3211 exists to prevent.
Even a perfectly stable MVID would have been unmatchable.
The change
The from-source arm stops deriving and states it. The platform IS this checkout, its commit IS
what a consumer reads, and
--framework-mvidis the packer's own flag for that — its own errortext names
g<sha>as a legal token, and the only validation is "no whitespace".One identity per platform commit, identical for every entry, and no compiler build per matrix
job at all — which also removes a full
dotnet buildfrom every leg.The refs arm is unchanged: there the DLL genuinely is the platform, is already stamped, and
already yields
g<sha>. This makes the two arms agree rather than inventing a third shape.RECIPE_VERSION2 → 3, because bundles packed under recipe 2 state an identity no consumer canmatch and must be repacked, not reused.
The guard moves with its subject, and gets stronger
It used to pin where the anchor build lived — a config-level assertion that was green through all
three incarnations of this bug. It now forbids
MeshWeaver.Compiler.csprojin the pack stepat all: the per-job build is the mechanism every previous cure left in place, and it packs
GREEN while handing #3154 a value no consumer can match.
Mutation-proved: reintroducing that build turns the guard red (
Assert.DoesNotContain() Failure: Sub-string found), and it passes with it gone.Verification
MeshWeaver.Documentation.Test—0 Error(s), Release,-warnaserror.node-repo-pack-verify.py --self-testpasses.a lane whose bundles disagree is refused, and after this they cannot disagree — every entry
states the same commit.
Closes #3308