Repository navigation
fix(adoption): the stale-prebuilt refusal was armed and INERT — make it reachable - #2947
Conversation
…it reachable (#2813) #2821 built the three-way comparison that refuses a prebuilt assembly whose recorded source disagrees with the source the mesh holds. It has never fired. Four links were missing between a bundle and that comparison, and every one of them failed silently: 1. `PrebuiltAssemblySeeder.Seed` had a seven-parameter convenience overload that hard-coded `sourceFingerprint: null`, and BOTH production callers bound to it (`PluginBundleClient`, `ShippedPrebuiltBundles`). `AdoptedSourceFingerprint` was therefore never written and the first guard in `ApplyAdoptedSourceStamp` short-circuited: every adoption on every mesh returned `AdoptedUnverified`. 2. No producer wrote a fingerprint into a bundle at all. 3. `BundleReader` had nowhere to carry one. 4. The owner computed `CurrentSourceFingerprint` only "when there is something to compare it against" — a condition the incident's own ordering never satisfies. The owner publishes its snapshot first; the adoption's patch then lands carrying the fingerprint and the stamp request, and the sources watcher does not re-run because its `DistinctUntilChanged` keys on the source QUERIES, which did not change. The judgement read an absent live value and took the inconclusive branch. All four are closed. **The fingerprint shape is the load-bearing decision.** Producer and consumer must hash identically or every good bundle is refused — an outage strictly worse than the bug. The new `NodeTypeSourceFingerprint` (in `MeshWeaver.Compiler`, inside the framework-identity boundary, so a producer and consumer that can adopt across each other necessarily run the same implementation) hashes the COMPILE INPUT: the `NodeCompileShaping.CollectCompileSources` fold — the same call both the runtime and the bake already make — reduced to `(node path, SHA-256 of the code text)`, folded with `PartitionSourceFingerprint`. Hashing the source nodes' serialised content, which is what the live half did, cannot work across processes and each failure mode is a FALSE REFUSAL: `CodeConfiguration` carries run bookkeeping (`LastExecutedAt`, …) that moves with a Run button and no producer can know; the consumer has a TypeRegistry and the compiler-driven bake deliberately has none, so the two serialise the same node differently by design; and node metadata is not compile input. Also fixed: `ApplyCompileSuccess` never reset `BuildProvenance`, so a type refused as stale and then successfully recompiled kept reading `AdoptionRefused` forever. The seven-parameter overload is `[Obsolete(error: true)]` rather than deleted: AGENTS.md requires sweeping the live mesh before removing public framework surface, and that sweep cannot be completed — `search_chunks` answers `"searched": false` on both reachable deployments, which is a FAILED sweep. Both repos' node trees were swept by hand and hold no caller, so the symbol stays and nothing already compiled can break, while every source call site now fails loudly with the reason. Verified — each new test was proven able to fail by inverting one condition and restoring: - restoring the defect (consumer passes null): refusal test times out red, verified test reports `AdoptedUnverified`, legacy test still green; - salting one side's hash: verified test red, `BakeEquivalenceTest` red ("Strings differ"); - legacy branch claiming verified: legacy test red, alone. Closes #2813 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Kwar1ZGt5FMEpHiXmkzKCm
The worst outcome of the fingerprint comparison is not a missed staleness — it is refusing bundles that are fine, which would make every portal recompile everything. That failure was invisible to CI by construction: `Seed` returns true, the adopted/declared counts still balance (the owner refuses afterwards, when it stamps), the type flips to Pending, recompiles locally, and every per-type verdict is green. bake-then-gate.sh stages the bake and the gate from the SAME tree, so an "ADOPTION REFUSED" line in that run can only mean the compiler-driven producer and the runtime hash the same content differently. assert-bake-consumption.sh now fails on the phrase, with its own message rather than the fallback one (a reader sent to look for a declined bundle would find a perfectly adopted one). Proven able to fail: clean logs pass, an injected refusal line exits 1 naming #2813. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Kwar1ZGt5FMEpHiXmkzKCm
|
Added a second, stronger pin for the equality claim — and it is the one that runs on the real doc and samples trees rather than a fixture.
That failure was invisible to CI by construction, in exactly the way the original defect was:
Proven able to fail before shipping: clean logs exit 0; an injected Also filed #2948 for the stated residue ( |
There was a problem hiding this comment.
Pull request overview
This PR makes the “stale prebuilt adoption refusal” path (#2813 / #2821) actually reachable end-to-end by ensuring a producer records a source-content fingerprint into bundles, the reader preserves it, the consumer passes it into adoption, and the owner computes its live fingerprint unconditionally so the three-way comparison can fire in the real event ordering.
Changes:
- Add
NodeTypeSourceFingerprintand record per-assemblySourceFingerprintin bundle manifests, carrying it throughBundleWriter → BundleReader → PrebuiltAssemblySeeder. - Update adoption/seeding call sites to pass the fingerprint explicitly (with
nullonly when intentionally claiming a legacy bundle), and make the 7-argSeedoverload[Obsolete(error: true)]. - Adjust owner-side compilation/source watchers to compute and persist
CurrentSourceFingerprintreliably and resetBuildProvenancetoCompiledon successful local compilation; add end-to-end tests and documentation/What’s New entry.
Reviewed changes
Copilot reviewed 18 out of 18 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| tools/MeshWeaver.PluginTester/TreeBake.cs | Record SourceFingerprint when emitting baked assembly entries. |
| tools/MeshWeaver.PluginTester/CascadeBuild.cs | Record SourceFingerprint for cascaded package builds. |
| tools/MeshWeaver.PluginTester/BakeOutput.cs | Compute and attach live source fingerprints for mesh-driven bake outputs. |
| test/MeshWeaver.PluginTester.Test/BakeEquivalenceTest.cs | Assert mesh-driven vs compiler-driven bakes produce equal, non-empty, discriminating fingerprints. |
| test/MeshWeaver.PluginCatalog.Test/ShippedPrebuiltBundlesTest.cs | Update cref to new Seed signature. |
| test/MeshWeaver.PluginCatalog.Test/AdoptedBuildSourceStampTest.cs | Add end-to-end tests covering verified/unverified/refused adoption paths via real bundles. |
| src/MeshWeaver.PluginCatalog/PluginBundleClient.cs | Pass bundle SourceFingerprint into PrebuiltAssemblySeeder.Seed. |
| src/MeshWeaver.Plugin.Packaging/BundleWriter.cs | Add AssemblyEntry.SourceFingerprint and write it into the manifest. |
| src/MeshWeaver.Plugin.Packaging/BundleReader.cs | Carry manifest fingerprint into payloads for consumers. |
| src/MeshWeaver.Hosting/ShippedPrebuiltBundles.cs | Thread SourceFingerprint through shipped bundle seeding. |
| src/MeshWeaver.Graph/Configuration/PrebuiltAssemblySeeder.cs | Make the old overload obsolete(error) and add required fingerprint parameter to the new overload. |
| src/MeshWeaver.Graph/Configuration/NodeTypeDefinition.cs | Update docs/crefs describing adopted source fingerprint behavior. |
| src/MeshWeaver.Graph/Configuration/NodeTypeCompilationHelpers.cs | Compute/persist live fingerprint unconditionally; wait for live fingerprint before consuming one-shot stamp requests; reset BuildProvenance on compile success. |
| src/MeshWeaver.Graph/Configuration/NodeTypeAdoptionRegistry.cs | Update cref to new Seed signature. |
| src/MeshWeaver.Compiler/NodeTypeSourceFingerprint.cs | New shared fingerprint implementation over compile inputs (paths + code text SHA). |
| src/MeshWeaver.Documentation/Data/WhatsNew/2026-09-01-a-prebuilt-assembly-is-now-checked-against-the-source-you-actually-have.md | New user-facing “Fix” entry describing verified/refused/unverified adoption. |
| src/MeshWeaver.Documentation/Data/Architecture/NodeTypeCompilation.md | Document what is hashed, how the fingerprint travels, and why the earlier implementation was inert. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| var access = workspace.Hub.ServiceProvider.GetService<AccessService>(); | ||
| return Observable | ||
| .Using( | ||
| () => access?.ImpersonateAsSystem() | ||
| ?? System.Reactive.Disposables.Disposable.Empty, | ||
| _ => NodeSources.GetSources(workspace, def, typePath)) | ||
| .Take(1) | ||
| .Timeout(ReadBudget) | ||
| .Select(sources => NodeTypeSourceFingerprint.Compute(sources, typePath)); |
There was a problem hiding this comment.
Correct, and it would have gone red in CI — thank you. ImpersonationScopeSiteRatchetGuard scans tools/, and BakeOutput.cs is not on its allow list, so this was a NEW SITE.
Fixed in b4ee197 with access.RunAsSystem(...). Verified both directions rather than assumed: the guard passes with RunAsSystem, and putting the old shape back reproduces NEW SITE tools/MeshWeaver.PluginTester/BakeOutput.cs (1) exactly as you predicted. RunAsSystem also subsumes the null-access fallback, so the Disposable.Empty branch is gone too.
BakeEquivalenceTest is still green, so the producer/consumer fingerprint equality is unaffected by the reshaped read.
…ble.Using Copilot review, both findings real: 1. The new source-fingerprint read used `Observable.Using(() => access.ImpersonateAsSystem(), …)` — the #1790 identity-latch shape. Rx runs Using's resource factory on the SUBSCRIBING thread and disposes it when the inner observable TERMINATES, which for this cross-hub read is a different thread, so the subscriber is left running as System. `ImpersonationScopeSiteRatchetGuard` scans `tools/` and BakeOutput.cs is not on its allow list, so this would have gone RED in CI. Verified both ways: the guard passes with RunAsSystem and reports `NEW SITE tools/MeshWeaver.PluginTester/BakeOutput.cs (1)` when the old shape is put back. RunAsSystem also subsumes the null-access fallback, so the Disposable.Empty branch goes. 2. `using MeshWeaver.Mesh.Security;` was dead — AccessService and RunAsSystem both live in `MeshWeaver.Messaging`, already imported. (CS8019 is not on in this build, so the -warnaserror build was green either way; removed as genuine dead code, not as a fix.) BakeEquivalenceTest still green, so the producer/consumer fingerprint equality is unchanged by the reshaped read. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Kwar1ZGt5FMEpHiXmkzKCm
|
Both Copilot findings were real and are fixed in b4ee197. 1. Verified in both directions rather than assumed:
2.
|
Test Results (shard 0)793 tests 789 ✅ 3m 17s ⏱️ Results for commit b4ee197. |
Test Results (shard 1)1 138 tests 1 138 ✅ 3m 56s ⏱️ Results for commit b4ee197. |
Test Results (shard 5) 8 files 8 suites 4m 24s ⏱️ Results for commit b4ee197. |
Test Results (shard 4)1 168 tests 1 168 ✅ 3m 59s ⏱️ Results for commit b4ee197. |
Test Results (shard 3)1 153 tests 1 153 ✅ 7m 38s ⏱️ Results for commit b4ee197. |
Test Results (shard 2)3 030 tests 2 838 ✅ 6m 41s ⏱️ Results for commit b4ee197. |
Test Results 44 files 44 suites 29m 58s ⏱️ Results for commit b4ee197. |
The refusal built by #2821 has never fired
#2821 shipped the three-way comparison that refuses a prebuilt assembly whose recorded source
disagrees with the source the mesh holds, and #2820 records the consequence in one line: "Today
every adoption reads
AdoptedUnverified, because nothing writes a source fingerprint into a bundleyet."
It was worse than that. Four links were missing between a bundle and that comparison, and every
one of them fails silently — a guard whose input never arrives is indistinguishable from a guard
that passes:
PrebuiltAssemblySeeder.Seedhad a seven-parameter convenience overload that hard-codedsourceFingerprint: null, and both production callers bound to it (PluginBundleClient,ShippedPrebuiltBundles).AdoptedSourceFingerprintwas therefore never written and the firstguard in
ApplyAdoptedSourceStampshort-circuited.BundleReaderhad nowhere to carry one.satisfies. The owner computed
CurrentSourceFingerprintonly "when there is something tocompare it against". But the owner publishes its snapshot first; the adoption's patch then
lands carrying both the fingerprint and the stamp request, and the sources watcher does not
re-run — its
DistinctUntilChangedkeys on the source queries, which did not change. Thejudgement then read an absent live value and took the inconclusive branch. So even with links
1–3 closed, the refusal would still not have fired in the ordering that destroyed the documents.
All four are closed, and the refusal now fires end to end.
The load-bearing decision: what is hashed
Producer and consumer must hash identically, or every good bundle is refused — a self-inflicted
outage strictly worse than the staleness bug. The new
NodeTypeSourceFingerprinthashes thecompile input: the
NodeCompileShaping.CollectCompileSourcesfold — the same call both theruntime compile path and the tree bake already make, so they cannot fork on which files count —
reduced to
(node path, SHA-256 of the code text)and folded withPartitionSourceFingerprint(which sorts by path, so enumeration order cannot reach the result).
It lives in
MeshWeaver.CompilerbecauseFrameworkBuildIdentity.FullMvidAssembliesis thatassembly's transitive closure, and adoption is already gated on framework identity — so a producer
and a consumer that can adopt across each other necessarily run the same implementation of the hash.
Hashing the source nodes' serialised content — what the live half did — cannot work across
processes, and each failure mode is a FALSE REFUSAL:
CodeConfigurationcarries run bookkeeping (LastExecutedAt,LastExecutedBy,LastExecutedCodeHash,LastActivityPath) written when a reader presses Run on a code cell.The live hash would move with no source change, and no producer can know those values.
$type); the compiler-drivenbake deliberately has neither —
TreeNodeLoadermaterialises exactly the two content types acompile reads and leaves the rest null, precisely so a half-populated registry cannot degrade
content to
JsonElement. Two honest readers, two different JSON strings.Also fixed
ApplyCompileSuccessnever resetBuildProvenance, so a type refused as stale and thensuccessfully recompiled kept reading
AdoptionRefusedforever. One refusal in a node's historywould have marked it permanently — turning the operator signal the incident asked for into noise,
and making #2820's execute-time interlock refuse a type whose live source it had just compiled
itself.
ApplyCompileFailuredeliberately still does not reset it.The obsolete overload, and why it is not deleted
AGENTS.md requires sweeping the live mesh before removing public framework surface. That sweep
could not be completed:
search_chunksanswers"searched": falseon both reachable deployments(content indexing inactive) — a FAILED sweep, not a clean one. Both repos' node trees were swept by
hand and hold no caller.
So the seven-parameter overload is
[Obsolete(error: true)]rather than deleted: the symbol stays,nothing already compiled can break, and every source call site now fails loudly with the reason.
Passing
nullremains legitimate for a genuinely legacy bundle — but it must be written at the callsite, where a reviewer can see the claim being waived.
Verification — every new test was proven able to fail
Each inversion was applied, built, run, and restored:
null(i.e. restore the exact defect)AdoptedUnverified; legacy greenCompiled(refused → recompiled);BakeEquivalenceTestred, "Strings differ"AdoptedVerifiedGreen after restore, and:
MeshWeaver.Graph.Test— 1607/1607AdoptedBuildSourceStampTest— 5/5, three of them new, each driving a real bundle throughBundleWriter → BundleReader → ShippedPrebuiltBundles → Seed → the owner's stampon a real meshBakeEquivalenceTest— bakes one content set both ways (through a real mesh, whose value isthe consumer-side value, and through the mesh-free tree bake) and asserts the fingerprints are
equal, non-empty, and different for different types. This is the equality proof; without it the
fix would be an outage.
scripts/check-record-signatures.pyandscripts/check-type-forwards.pyboth clean (the newfields are init properties, never primary-constructor parameters — adding one is a binary
break across a mixed fleet).
-c Release -warnaserror, one per invocation,0 Error(s).Stated residue
An
@@include pulls a Code node no source query matches, so it is outside the fold and a change toan included-only snippet does not move the fingerprint. This is not new — it is the same set
CompiledSourcesrecords, soIsDirtyhas always missed it too. Closing it means resolvingincludes on the live side (a mesh read per include). It is named in the doc and in the code rather
than left implied, because a guard whose coverage is assumed is exactly how this mechanism went
inert the first time.
Docs
Doc/Architecture/NodeTypeCompilationdocumented the three-way as if it were live. It now stateswhat actually reaches the comparison, why the fingerprint shape is what it is, and the general
lesson: "the fix merged" is not "the fix runs." Plus a What's New entry (
Category: Fix).Closes #2813
🤖 Generated with Claude Code
https://claude.ai/code/session_01Kwar1ZGt5FMEpHiXmkzKCm