From 589eea8784c7b1f0429fb3106a46ce22399fefc6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Roland=20B=C3=BCrgi?= <6334612+rbuergi@users.noreply.github.com> Date: Sat, 5 Sep 2026 09:56:08 +0200 Subject: [PATCH 1/2] State the framework identity as the platform commit instead of deriving it per job (#3308) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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") and FrameworkIdentity.ReadIdentity PREFERS that stamp over the MVID. A consumer reads `g`; 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` 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` — 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) --- .github/scripts/module-build-key.py | 6 +- .github/workflows/node-repo-module-pack.yml | 97 ++++++++++--------- .../ModuleIdentityPublishGuard.cs | 78 ++++++++------- 3 files changed, 102 insertions(+), 79 deletions(-) diff --git a/.github/scripts/module-build-key.py b/.github/scripts/module-build-key.py index c546c3cbab..21ea6e493a 100644 --- a/.github/scripts/module-build-key.py +++ b/.github/scripts/module-build-key.py @@ -73,7 +73,11 @@ # genuinely differ — and a REUSED pre-#3211 bundle would be refused at the publish step, which is the # right verdict for those bytes and the wrong thing to spend a run discovering. One rebuild wave, and # every recorded key afterwards attests bytes that state what they were built against. -RECIPE_VERSION = "2" +# 3: the from-source arm STATES the framework identity as the platform commit (`g`) +# instead of deriving an MVID from a compiler DLL built per matrix job (#3308). Bundles packed +# under recipe 2 state an identity no consumer can match, so they must be REPACKED rather than +# reused — which is exactly what changing this constant forces. +RECIPE_VERSION = "3" # Repo-level files that change what EVERY project compiles or tests. Presence and content both # count; a file the repo does not have hashes as null so adding it later changes the key. diff --git a/.github/workflows/node-repo-module-pack.yml b/.github/workflows/node-repo-module-pack.yml index 7acb214aa3..dc5931d7ee 100644 --- a/.github/workflows/node-repo-module-pack.yml +++ b/.github/workflows/node-repo-module-pack.yml @@ -1753,51 +1753,60 @@ jobs: if [ -n "$REFS" ]; then anchor="$REFS/MeshWeaver.Compiler.dll" where="the pinned platform image's extracted /app" + if [ ! -f "$anchor" ]; then + echo "::error::no identity anchor for $MODULE — expected MeshWeaver.Compiler.dll at '$anchor', i.e. in $where. Every bundle must state the framework build its bytes were compiled against (#3211): a consumer compares it against what it has landed, and an unstated one makes every reconcile answer 'up to date, identity could not be checked' forever. Nothing here may guess it, so the pack stops." + exit 1 + fi + # The image's own compiler, stamped by the build that produced the image, so + # FrameworkIdentity.ReadIdentity returns its `g` — the same token a portal image + # reports. Reading it off the DLL is right HERE because the DLL is the platform. + identity=(--graph-dll "$anchor") else - # 🚨 BUILT from the platform source, never scavenged out of $PACKDIR. Publish drops - # MeshWeaver.Compiler.dll beside the module ONLY when the module transitively - # ProjectReferences it — MeshWeaver.AI and MeshWeaver.Markdown.Collaboration both do, - # through MeshWeaver.Graph, and they were the ONLY two entries this lane built when - # the anchor landed, so "the platform ProjectReferences are real, so it IS beside the - # module" was measured on a population of two and read as a property of the arm. - # It is a property of the REFERENCE GRAPH: core's compose set grew to four on - # 2026-09-04 (#3290) and both new entries reach no compiler — MeshWeaver.Maps stops at - # MeshWeaver.Layout, MeshWeaver.Payments.Stripe is a leaf — so both were RED on their - # first run and took `CD delivered, or had a good reason not to` down with them for - # two hours (#3293). - anchordir="$GITHUB_WORKSPACE/meshweaver/src/MeshWeaver.Compiler" - # -warnaserror to match the module build above: when a module DOES reach the - # compiler transitively, that build is what compiles it, so omitting the flag here - # would let MeshWeaver.Maps accept a compiler build MeshWeaver.AI rejects — the same - # per-module asymmetry this change exists to remove. + # 🚨 STATE the identity; never DERIVE it from a DLL this job built (#3308). # - # 🚨 AND DELIBERATELY **NO** `-p:Version="$VERSION"` (#3176). #3293 passed it, to make - # this "the assembly publish would have copied" — but that is the one global property - # that must NOT reach here, because $VERSION is the MODULE's package version and the - # identity must be the PLATFORM's. MSBuild writes Version into the assembly's version - # attributes, so it lands in the metadata the MVID is computed over. Measured on THIS - # command, with the two package versions core CD packs, plus a control: - # -p:Version=1.3.18 (AI) -> 34337c31960d47f5b5251d32ef923fc6 - # -p:Version=1.0.24 (Essentials) -> 7c1c4f70de084da78659e6bda495e1c5 - # no override, run A -> 71cc81badb364c5d8558ac5e7db6a44e - # no override, run B -> 71cc81badb364c5d8558ac5e7db6a44e (identical) - # With it, each matrix job builds its OWN compiler and the run states one identity PER - # MODULE — already seen in the field on run 33874892203, by the earlier route to the - # same pollution: AI stated be27d0fb9ad54ae6a862bfa7aeb97c9b while - # Markdown.Collaboration stated d756b82e09804a11b0ea44d26233af6c for the same platform - # and the same commit. A - # per-module identity names no platform build a consumer can have landed, so #3154's - # (version, identity) comparison gets a value that means nothing — GREEN, and useless. - # Without it every job builds byte-identical bytes from one commit, so the whole run - # states ONE identity, which is what the field is for. - dotnet build "$anchordir/MeshWeaver.Compiler.csproj" -c Release -warnaserror \ - -p:MeshWeaverRoot="$GITHUB_WORKSPACE/meshweaver" - anchor="$anchordir/bin/Release/net10.0/MeshWeaver.Compiler.dll" - where="the platform's own build output (no platform image is pinned, so the platform was built from source)" - fi - if [ ! -f "$anchor" ]; then - echo "::error::no identity anchor for $MODULE — expected MeshWeaver.Compiler.dll at '$anchor', i.e. in $where. Every bundle must state the framework build its bytes were compiled against (#3211): a consumer compares it against what it has landed, and an unstated one makes every reconcile answer 'up to date, identity could not be checked' forever. Nothing here may guess it, so the pack stops." - exit 1 + # The arm used to `dotnet build` MeshWeaver.Compiler per matrix job and read the MVID + # off the result. #3293 built it from the platform tree instead of scavenging it, and + # #3306 removed the `-p:Version` that polluted it — and the run STILL stated one + # identity per module. Measured on run 33950389008 (2026-09-05, four entries, one + # commit, every leg `plan: BUILD`): + # + # 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 this same output path, carrying the module's + # own global properties — and the anchor build that follows finds it up to date and + # keeps those bytes. Maps and Payments.Stripe never build it, so their anchor build + # compiles it clean and they agree. Removing `-p:Version` from the ANCHOR build could + # never fix that: the pollution comes 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") and + # FrameworkIdentity.ReadIdentity PREFERS that stamp over the MVID. A consumer + # therefore reads `g` while a source-built bundle stated a 32-hex MVID: never + # equal, by construction, so #3154's (version, identity) comparison could never answer + # anything but "identity could not be checked". + # + # So: the platform IS this checkout, and a commit is exactly what the consumer reads. + # `--framework-mvid` is the packer's own flag for "the platform's own reading", and + # `g` is one of the three tokens it documents. One identity per platform commit, + # identical for every entry in the run, and no compiler build per matrix job at all. + 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:-}'). 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") + where="the platform checkout's own commit (no platform image is pinned, so the platform was built from source)" + echo "framework identity for this run: g$sha (stated from the platform checkout, not derived per job)" fi echo "packing $MODULE from $PACKDIR (compiler: $COMPILER); closure flags: ${closure[*]:-}" dotnet "$RUNNER_TEMP/pack-tool/MeshWeaver.Plugin.Build.dll" \ @@ -1807,7 +1816,7 @@ jobs: --plugin "$PACKAGE" \ --package-version "$VERSION" \ --min-mesh-version "$FLOOR" \ - --graph-dll "$anchor" \ + "${identity[@]}" \ --own-platform "$(bash "$GITHUB_WORKSPACE/meshweaver/.github/scripts/module-owned-platform.sh" "$GITHUB_WORKSPACE/src")" \ --out "$RUNNER_TEMP/bundles" diff --git a/test/MeshWeaver.Documentation.Test/ModuleIdentityPublishGuard.cs b/test/MeshWeaver.Documentation.Test/ModuleIdentityPublishGuard.cs index 9912ecf4be..49116e4b14 100644 --- a/test/MeshWeaver.Documentation.Test/ModuleIdentityPublishGuard.cs +++ b/test/MeshWeaver.Documentation.Test/ModuleIdentityPublishGuard.cs @@ -69,44 +69,54 @@ public void ThePackStep_NamesTheIdentityAnchor_AndFailsRedWhenThereIsNone() Assert.Contains("anchor=\"$REFS/MeshWeaver.Compiler.dll\"", pack, StringComparison.Ordinal); Assert.Contains("--graph-dll \"$anchor\"", pack, StringComparison.Ordinal); - // 🚨 THE FROM-SOURCE ARM BUILDS ITS ANCHOR — it does not read one out of $PACKDIR. - // `dotnet publish` drops MeshWeaver.Compiler.dll beside a module only when that module - // transitively ProjectReferences it. MeshWeaver.AI and MeshWeaver.Markdown.Collaboration - // both do, through MeshWeaver.Graph, and they were the ONLY two entries this lane built - // when the anchor landed — so a property of two reference graphs was written down as a - // property of the arm, and 100% of the population agreed. Core's compose set grew to four - // on 2026-09-04 (#3290): MeshWeaver.Maps stops at MeshWeaver.Layout and - // MeshWeaver.Payments.Stripe is a leaf, so both were RED on their first run and took - // `CD delivered, or had a good reason not to` with them. - Assert.Contains("anchordir=\"$GITHUB_WORKSPACE/meshweaver/src/MeshWeaver.Compiler\"", pack, StringComparison.Ordinal); - Assert.Contains("dotnet build \"$anchordir/MeshWeaver.Compiler.csproj\" -c Release -warnaserror", pack, StringComparison.Ordinal); - Assert.Contains("anchor=\"$anchordir/bin/Release/net10.0/MeshWeaver.Compiler.dll\"", pack, StringComparison.Ordinal); - - // The ratchet: scavenging the anchor out of the module's own publish output is the exact - // defect above, and it reads as reasonable every time. It must not come back. + // 🚨 THE FROM-SOURCE ARM STATES THE IDENTITY; IT DOES NOT BUILD ONE. + // + // Three cures were tried on this arm and all three were the same mistake — deriving the + // platform's identity from a DLL sitting in a per-MODULE job: + // #3211 read it out of $PACKDIR (present only when the module transitively + // ProjectReferences the compiler — a property of two reference graphs, written + // down as a property of the arm, and 100% of a population of two agreed); + // #3293 built it from the platform tree (correct location, still per job); + // #3306 removed the `-p:Version` that polluted that build (correct property, still per job). + // Run 33950389008 (2026-09-05, four entries, ONE commit, every leg `plan: BUILD`) still + // stated three identities: Maps and Payments.Stripe agreed on 0051e721… having compiled the + // compiler ONCE, while Markdown.Collaboration (6553ea37…) and AI (395909c2…) each compiled + // it TWICE and each got its own. The split is exactly the reference graph: those two reach + // the 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. No property removed from the ANCHOR build can fix pollution that arrives + // from the MODULE build upstream. + Assert.Contains("identity=(--framework-mvid \"g$sha\")", pack, StringComparison.Ordinal); + Assert.Contains("rev-parse HEAD", pack, StringComparison.Ordinal); + + // 🚨 THE RATCHET THAT REPLACES ALL THREE. The arm must not compile the platform's compiler + // at all: a per-job build is the mechanism every previous cure left in place, and it packs + // GREEN while handing #3154's comparison a value no consumer can match. If a fourth cure + // ever reaches for `dotnet build …MeshWeaver.Compiler.csproj` here, this fails. + Assert.DoesNotContain("MeshWeaver.Compiler.csproj", pack, StringComparison.Ordinal); + Assert.DoesNotContain("anchordir=", pack, StringComparison.Ordinal); + + // The older ratchet stands: scavenging the anchor out of the module's own publish output + // reads as reasonable every time. It must not come back either. Assert.DoesNotContain("anchor=\"$PACKDIR/MeshWeaver.Compiler.dll\"", pack, StringComparison.Ordinal); - // 🚨 #3176 — AND THE ANCHOR BUILD PASSES NO MODULE VERSION. `$VERSION` is the MODULE's - // package version; MSBuild writes it into the assembly's version attributes, which are part - // of the metadata the MVID is computed over. Passing it makes every matrix job build its - // OWN compiler, so a run states one identity PER MODULE — measured on THIS command, the two - // package versions core CD packs: -p:Version=1.3.18 → 34337c31960d47f5b5251d32ef923fc6, - // -p:Version=1.0.24 → 7c1c4f70de084da78659e6bda495e1c5, while two runs with NO override are - // byte-identical (71cc81badb364c5d8558ac5e7db6a44e twice). Observed in the field on run 33874892203 - // (MeshWeaver.AI be27d0fb…, MeshWeaver.Markdown.Collaboration d756b82e…, same platform, - // same commit). That is the SILENT half of this defect: absent the anchor the pack goes - // red, but a per-module anchor packs GREEN and hands #3154's comparison a value no - // consumer can match. The identity is the PLATFORM's, so the platform's own properties - // build it and nothing per-module may reach that command line. - var anchorBuild = pack[pack.IndexOf( - "dotnet build \"$anchordir/MeshWeaver.Compiler.csproj\"", StringComparison.Ordinal)..]; - anchorBuild = anchorBuild[..anchorBuild.IndexOf("anchor=\"$anchordir", StringComparison.Ordinal)]; - Assert.DoesNotContain("-p:Version", anchorBuild, StringComparison.Ordinal); - - // 🚨 BOTH arms end RED when there is no anchor — the branch picks WHERE to look, never - // whether to check. A bundle whose identity is a guess is worse than a pack that stops. + // 🚨 AND `g` IS THE TOKEN A CONSUMER ACTUALLY READS. A portal image is built with + // -p:CIRun=true, so AddCommitHashMetadata stamps + // AssemblyMetadata("MeshWeaverFrameworkIdentity", "g") and + // FrameworkIdentity.ReadIdentity PREFERS that stamp over the MVID. A bundle stating a + // 32-hex MVID could therefore never equal what a consumer reports — not "usually differs", + // never equal, by construction — so #3154's (version, identity) comparison could only ever + // answer "identity could not be checked". Determinism alone would not have fixed that; + // stating the commit does. + Assert.Contains("MeshWeaverFrameworkIdentity", File.ReadAllText(Path.Combine( + SourceScan.FindRepoRoot(), "src", "MeshWeaver.Plugin.Build", "FrameworkIdentity.cs")), + StringComparison.Ordinal); + + // 🚨 An unreadable platform commit ends the arm RED — it never falls back to a derived or + // empty identity. The branch picks WHERE the identity comes from, never whether to have one. Assert.Contains("if [ ! -f \"$anchor\" ]; then", pack, StringComparison.Ordinal); Assert.Contains("::error::no identity anchor for $MODULE", pack, StringComparison.Ordinal); + Assert.Contains("cannot read the platform commit", pack, StringComparison.Ordinal); Assert.DoesNotContain("--graph-dll \"${anchor:-", pack, StringComparison.Ordinal); } From 82861125b51eeaef333d735a0ac88b310280afc2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Roland=20B=C3=BCrgi?= <6334612+rbuergi@users.noreply.github.com> Date: Sat, 5 Sep 2026 10:07:17 +0200 Subject: [PATCH 2/2] Let git's failure speak: the platform-commit read no longer swallows stderr or the exit status MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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) --- .github/workflows/node-repo-module-pack.yml | 20 ++++++++++++-------- 1 file changed, 12 insertions(+), 8 deletions(-) diff --git a/.github/workflows/node-repo-module-pack.yml b/.github/workflows/node-repo-module-pack.yml index dc5931d7ee..1ba4fb7f13 100644 --- a/.github/workflows/node-repo-module-pack.yml +++ b/.github/workflows/node-repo-module-pack.yml @@ -1796,14 +1796,18 @@ jobs: # `--framework-mvid` is the packer's own flag for "the platform's own reading", and # `g` is one of the three tokens it documents. One identity per platform commit, # identical for every entry in the run, and no compiler build per matrix job at all. - 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:-}'). 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 + # NOT `$(… 2>/dev/null || true)`: that swallows git's message AND its exit status, so a + # checkout without .git would reach the check below as a blank string with no trace of + # why. The `if !` keeps `set -e` from aborting before the explanation, while git's own + # stderr still lands in the log. + if ! sha="$(git -C "$GITHUB_WORKSPACE/meshweaver" rev-parse HEAD)"; then + echo "::error::cannot read the platform commit at \$GITHUB_WORKSPACE/meshweaver — git rev-parse HEAD failed and its message is above. 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 + fi + if [ ${#sha} -ne 40 ] || [ -n "${sha//[0-9a-f]/}" ]; then + echo "::error::the platform commit at \$GITHUB_WORKSPACE/meshweaver read as '${sha}', which is not a 40-character hex sha. The bundle identity must name a commit a consumer can compare against (#3308), so the pack stops rather than stating something unmatchable." + exit 1 + fi identity=(--framework-mvid "g$sha") where="the platform checkout's own commit (no platform image is pinned, so the platform was built from source)" echo "framework identity for this run: g$sha (stated from the platform checkout, not derived per job)"