Repository navigation
fix(graph): make "compiled Ok" checkable against the bytes actually served (#2471) - #2655
Merged
Merged
Conversation
…erved (#2471) A portal can serve STALE compiled code while reporting Ok/compiled, and survive every remedy the product offers. Measured on memex 2026-08-26: the source node carried the change, get_diagnostics said Ok, the rendered Tests area produced the OLD suite — through two NodeType recycles, four instance recycles and a forced compile over 30+ minutes, with the $Banner stale-build adornment empty throughout. WHY NOTHING COULD SEE IT. Every staleness check compared LatestAssemblyPath against the path the instance bound. A path is a STORE KEY — (nodeTypePath, LastCompiledVersion) — that each pod resolves through its own local cache, and a recompile of an already-Ok type does not advance the node's version. So the key is identical on both sides while the bytes behind it differ, and a recycle re-resolves the same key from the same local copy. A path comparison is structurally incapable of observing this state, and every remedy built on one is inert by construction. This lands the DETECTION: an assembly's MVID is minted per emission, so it answers the question a path cannot — are these the bytes this node is talking about? Same postcondition-not-hope move the platform already makes with framework-mvid.txt beside a bake and the _complete sentinel beside a publication. * ServedBuildIdentity — MVID from a file or from bytes, metadata only, nothing loaded; Mismatch() is null on ANY unknown, so a legacy node never fires. * NodeTypeDefinition.LatestAssemblyMvid, stamped by all three success writers (ApplyCompileSuccess, the adoption path in PrebuiltAssemblySeeder, and NodeTypeContractHandler's racing write-back). * EnrichWithNodeType reads the bound file's MVID once per activation and hands both identities to the watcher. A mismatch is true AT BIND TIME — there is no later publication to wait for, which is exactly why a publication-driven watcher stayed silent — so the offer is SEEDED with the verdict and logged at Error, not Warning: a portal serving a build nobody published is an integrity failure and nothing said so. * The watcher fires on EITHER identity, so a republication at the same key with different bytes is no longer invisible. Added, never substituted: a legacy node keeps healing on the path. * StaleBuildKind separates the two states, because they need opposite advice. A same-key mismatch gets NO recycle link and is never auto-recycled even under convergence: recycling re-binds the same local copy, and spending the watcher's Take(1) on it would leave the instance unchanged with its one shot gone and no banner shown. * get_diagnostics' Ok reply stops claiming "…and is loaded" — a claim about a hub's runtime state that the formatter, which reads the node's record, has no evidence for and which was FALSE for 30 minutes. It now names the mvid as a field and points at the check that IS about the served bytes. Added as an OVERLOAD, never an optional parameter. 11 new tests; the 6 with a before/after all fail against the previous behaviour. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
rbuergi
force-pushed
the
fix/served-bytes-are-verifiable
branch
from
August 29, 2026 07:06
429d6cc to
eda0af5
Compare
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
rbuergi
enabled auto-merge
August 29, 2026 07:09
Contributor
There was a problem hiding this comment.
Pull request overview
Warning
Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.
This PR adds build identity (assembly MVID) tracking so the platform can detect when a portal serves different compiled bytes than the NodeType reports as successfully built, and updates diagnostics/banner messaging to avoid overclaiming what is being executed.
Changes:
- Introduces
ServedBuildIdentity(MVID readers) and stampsNodeTypeDefinition.LatestAssemblyMvidon all successful build/adoption/write-back paths. - Extends stale-build detection/UI to distinguish “newer build available” vs “served build is not published” and avoids auto-recycle for same-key mismatches.
- Updates
get_diagnosticsformatting to name the build (MVID) and removes the “is loaded” overclaim; adds tests and documentation.
Reviewed changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| test/MeshWeaver.Hosting.Monolith.Test/DiagnosticsDescribesTheBuildNotTheServeTest.cs | Pins updated get_diagnostics Ok wording + new MVID field behavior. |
| test/MeshWeaver.Graph.Test/ServedBuildIsVerifiableTest.cs | Verifies MVID readers, mismatch semantics, and watcher behavior for same-key mismatches. |
| src/MeshWeaver.Messaging.Hub/Localization/strings.en.json | Adds banner text for served-build mismatch state. |
| src/MeshWeaver.Messaging.Hub/Localization/strings.de.json | Adds German banner text for served-build mismatch state. |
| src/MeshWeaver.Mesh.Operations/MeshOperations.cs | Adds FormatDiagnostics overload and includes MVID in Ok responses. |
| src/MeshWeaver.Graph/Configuration/StaleBuildBanner.cs | Adds StaleBuildKind + renders mismatch banner without recycle link. |
| src/MeshWeaver.Graph/Configuration/ServedBuildIdentity.cs | New MVID reader utility (file/bytes) and mismatch/unverifiable helpers. |
| src/MeshWeaver.Graph/Configuration/PrebuiltAssemblySeeder.cs | Stamps LatestAssemblyMvid when adopting prebuilt assemblies. |
| src/MeshWeaver.Graph/Configuration/NodeTypeEnrichmentHelpers.cs | Reads bound MVID on activation; watcher detects mismatches by path or MVID and avoids auto-recycle for same-path mismatch. |
| src/MeshWeaver.Graph/Configuration/NodeTypeDefinition.cs | Adds LatestAssemblyMvid to NodeTypeDefinition contract. |
| src/MeshWeaver.Graph/Configuration/NodeTypeContractHandler.cs | Stamps LatestAssemblyMvid during write-back race with compile success. |
| src/MeshWeaver.Graph/Configuration/NodeTypeCompilationHelpers.cs | Stamps LatestAssemblyMvid on compile success. |
| src/MeshWeaver.Documentation/Data/WhatsNew/2026-08-29-a-page-now-tells-you-when-it-is-running-old-code.md | Documents the new detection and updated guidance. |
Suppressed comments (1)
src/MeshWeaver.Messaging.Hub/Localization/strings.en.json:1
- This text says the page is 'executing' specific bytes, but the detection is based on the bound file’s MVID (a metadata read) rather than a verified runtime-loaded module. To keep the message strictly accurate (and aligned with the PR’s goal of avoiding overclaims), consider wording like 'the bytes this page bound/loaded from disk' instead of 'executing'.
{
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+12
to
+32
| /// <summary>What the banner is ABOUT — two states that look alike and need opposite advice.</summary> | ||
| public enum StaleBuildKind | ||
| { | ||
| /// <summary> | ||
| /// The type has PUBLISHED a newer build and this instance is still on the previous one. Fully | ||
| /// functional, and a recycle picks the new one up — so the banner is an offer. | ||
| /// </summary> | ||
| NewerBuildAvailable, | ||
|
|
||
| /// <summary> | ||
| /// 🚨 The bytes this instance is EXECUTING are not the bytes the type says it published — the | ||
| /// served assembly's MVID differs from <see cref="NodeTypeDefinition.LatestAssemblyMvid"/>. | ||
| /// | ||
| /// <para>This is NOT an offer, and the difference matters to the reader: the node's | ||
| /// <c>Ok</c> is not evidence about what is being served, and a recycle re-binds the same local | ||
| /// copy, so pressing the button changes nothing. Systemorph/MeshWeaver#2471 — measured on memex | ||
| /// 2026-08-26 across two NodeType recycles, four instance recycles and a forced compile, with | ||
| /// this very adornment empty throughout because a PATH comparison cannot see it.</para> | ||
| /// </summary> | ||
| ServedBuildIsNotPublished, | ||
| } |
Comment on lines
+1882
to
+1884
| && (!string.Equals(d.LatestAssemblyPath, boundAssemblyPath, StringComparison.Ordinal) | ||
| || ServedBuildIdentity.Mismatch( | ||
| d.LatestAssemblyMvid, boundAssemblyMvid, nodeType) is not null)) |
Comment on lines
+1902
to
+1909
| var samePath = string.Equals( | ||
| published, boundAssemblyPath, StringComparison.Ordinal); | ||
| if (samePath) | ||
| logger?.LogError( | ||
| "Served build is NOT the published build for instance '{InstancePath}': " | ||
| + "{Detail}", instanceHub.Address, | ||
| ServedBuildIdentity.Mismatch( | ||
| x.Def?.LatestAssemblyMvid, boundAssemblyMvid, nodeType)); |
| // and change nothing — six times over, on memex. The banner's job in that state is to say | ||
| // the status is not evidence, not to suggest a cure it does not have. | ||
| if (offer.Kind == StaleBuildKind.ServedBuildIsNotPublished) | ||
| return Controls.Markdown(host.Localize("ui.mdServedBuildIsNotPublished")); |
Contributor
Contributor
Contributor
Contributor
Test Results (shard 2)814 tests - 143 814 ✅ - 143 5m 1s ⏱️ -8s Results for commit 1693a7a. ± Comparison against base commit ecef6db. This pull request removes 376 and adds 233 tests. Note that renamed tests count towards both. |
Contributor
Contributor
Test Results (shard 1)1 168 tests +148 1 168 ✅ +148 6m 27s ⏱️ +2s Results for commit 1693a7a. ± Comparison against base commit ecef6db. This pull request removes 228 and adds 376 tests. Note that renamed tests count towards both. |
Contributor
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.
Lands the detection half of #2471 — a portal serving stale compiled code while reporting
Ok.Why nothing could see it, and why no remedy worked
Measured on
memex, 2026-08-26: the source node carried the change,get_diagnosticssaidOk/compiled, andReinsuranceDemo/Walkthrough's in-nodeTestsarea still rendered the old suite — through two NodeType recycles, four instance recycles and a forced compile, over 30+ minutes, with the$Bannerstale-build adornment empty throughout.That is not a coincidence of timing. Every staleness check in the platform compared
NodeTypeDefinition.LatestAssemblyPathagainst the path the instance bound. A path is a store key,(nodeTypePath, LastCompiledVersion), and:Oktype does not advance the node's version.So the key can be identical on both sides while the bytes behind it differ per replica — and a recycle simply re-resolves the same key from the same local copy. A path comparison is structurally incapable of observing this state, and every remedy built on one is inert by construction. The banner was not broken; it was blind.
This is the worst class of defect in the codebase: a claim of
Okthat is not backed by the bytes being served is a lie with a green tick, strictly worse than an error. A merged, published, correct fix becomes indistinguishable from one that is live.What makes the claim checkable
An assembly's MVID is minted per emission, so it answers the question the path cannot: are these the bytes this node is talking about? Recording it on the node and reading it off the bound file turns "compiled Ok" from something believed into something compared — the same postcondition-not-hope move the platform already makes with
framework-mvid.txtbeside a bake and the_completesentinel beside a publication.ServedBuildIdentity(new) — MVID from a file or from bytes. Metadata only, nothing loaded: loading to ask forManifestModule.ModuleVersionIdwould pin bytes this process may be about to replace. Every reader degrades tonullrather than throwing — it runs on the hot path of every per-instance hub binding, and a detector that faults an activation is a worse defect than the one it detects.NodeTypeDefinition.LatestAssemblyMvid— stamped by all three success writers:ApplyCompileSuccess(the one stamp shape), the adoption path inPrebuiltAssemblySeeder(from the image in hand), andNodeTypeContractHandler's write-back, which races the first and therefore has to stamp the same shape.EnrichWithNodeTypereads the bound file's MVID once per activation and hands both identities down. 🚨 The mismatch is true at bind time — there is no later publication to wait for, which is exactly why a publication-driven watcher stayed silent — so the offer is seeded with the verdict rather than withnull, and logged atLogError, not Warning: a portal serving a build nobody published is an integrity failure, and the entire cost of A portal can serve STALE compiled code while reporting Ok/compiled — recycles, forced compile and the stale-build banner are all inert #2471 was that nothing said so.The two states need opposite advice
StaleBuildKindseparates them, and the difference is load-bearing:NewerBuildAvailable(path advanced)AutoRecycle, unchangedServedBuildIsNotPublished(same key, different bytes)Offering the recycle button in the second state would hand the viewer the exact remedy that was measured to report success and change nothing, six times over. And the watcher is
Take(1): spending that one shot on a recycle that converges on nothing leaves the instance in the same state with no banner and nothing left to fire.get_diagnosticsstops overclaimingThe
Okreply ended "The NodeType assembly was built without errors and is loaded." The first half is what the formatter knows — it reads the node's record. The second is a claim about a hub's runtime state that nothing in the read establishes, and it was false for 30 minutes while the portal served the previous build. It now states only what it knows, names the build (mvidas a field, so a caller can compare rather than read), and points at the check that is about the served bytes.Added as an overload, never an optional parameter — a defaulted parameter replaces the signature, which is the binary break
scripts/check-record-signatures.pyexists to refuse, one shape over. Same reasonStaleBuildOffer's new fields are init-only properties.Tests
11 new (
ServedBuildIsVerifiableTest,DiagnosticsDescribesTheBuildNotTheServeTest). Six have a before/after and all six fail against the previous behaviour (the fix reverted in the watcher'sWhere, the offer'sKind, the auto-recycle guard and theOkmessage):The other five pin the new primitive itself and have no unfixed counterpart — the type does not exist on
main. They are not decoration:OfFile_ReadsTheAssemblysRealMvid_WithoutLoadingItchecks the reader againstModule.ModuleVersionId(a reader returning a constant would pass every "it reads something" assertion and detect nothing), andMismatch_IsNullWheneverThereIsNoEvidenceOfOne+ALegacyNodeWithNoRecordedMvid_BehavesExactlyAsBeforepin the property that keeps this from bannering the whole mesh on its first boot.Green:
MeshWeaver.Graph.Test1727/1727;MeshWeaver.Hosting.Monolith.Test(Diagnostics/StaleBuild/SelfHeal/Prebuilt/BatchBake/CompilePark/CodeEditRecompile) 47/47;MeshWeaver.Messaging.Hub.TestLocalization 56/56; full solution builds clean;check-record-signatures.pyandcheck-type-forwards.pyclean againstorigin/main.🚨 What this does NOT do — the rest of #2471
This is detection, not convergence. After it, the stale state is loud — an
Errorlog naming both builds, a banner on the page, and anmvidinget_diagnosticsa caller can compare — instead of silent under a green tick. What remains:(nodeTypePath, LastCompiledVersion)is not unique per build, which is the actual root cause and a change to the resolution path, not the reporting path.ServedBuildIdentity.Unverifiableexists so such a sweep can report what it could not check — an all-green count over an all-unknown fleet must not read as proof — but nothing calls it yet.Leaving #2471 open for 1–3.
🤖 Generated with Claude Code