Repository navigation
fix(compile): the adopted source-stamp watcher counts a request committed only when a confirmed write consumed it - #5460
Conversation
…tted only when a confirmed write consumed it (#1105) The watcher advanced its commit mark inside the write's lambda, before the stamp was applied, so "committed" meant "a pass ran". A write that faulted after its lambda, or a judgement ApplyAdoptedSourceStamp deferred (request left standing while the live set is short of the bundle's paths, #4280), then made the next emission of the same request log "the write did not converge" at Error while the request was still open and converged moments later (Store/Order on memex-cloud: Error 22:54:24Z, converged and AdoptedVerified 74 s later). The mark now lives in AdoptedSourceStampLedger and moves in the write's onNext, only when the committed definition no longer carries the request; the write is the pure StampOnce. Real non-convergence (a confirmed consuming write, then the same request still dirty) stays an Error. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
Critical watcher wiring coverage and a moderate content-boundary test issue remain unresolved.
Get a fresh assessment by requesting another Copilot review.
Review effort: Lite
Findings: 1
Open (1)
What changed in this PR
Fixes adopted-source convergence tracking so only confirmed, request-consuming writes advance the commit ledger.
Changes:
- Adds per-watcher commit tracking and pure stamping.
- Adds regression tests for confirmed commits.
- Documents corrected convergence semantics.
| File | Summary | Review status |
|---|---|---|
test/MeshWeaver.Compiler.Pipeline.Test/AdoptedSourceStampCommitIsConfirmedTest.cs |
Tests deferred and consuming writes. | Moderate issue: use ContentAs<NodeTypeDefinition>(...) for resilient payload handling. 1 vote. |
src/MeshWeaver.Documentation/Data/Architecture/NodeTypeCompilation.md |
Documents confirmed convergence semantics. | No issues reported. |
src/MeshWeaver.Compiler.Pipeline/NodeTypeCompilationHelpers.cs |
Updates watcher confirmation and ledger handling. | Critical issue: add watcher-level coverage for deferred, faulted, and consuming update paths. 2 votes. |
💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| .Subscribe( | ||
| committed => | ||
| { | ||
| // 🚨 #1105 — the mark moves HERE, on the write's own confirmation, and only | ||
| // when the committed node no longer carries this request. It used to move | ||
| // inside the lambda, BEFORE the stamp was applied: a write that then | ||
| // faulted, or a judgement the stamp deferred (ApplyAdoptedSourceStamp |
There was a problem hiding this comment.
You're right, and I'm stating it as a known gap rather than claiming coverage I don't have.
What the tests pin. Both halves run with production code:
StampOnceruns the realApplyAdoptedSourceStampand defers on the A StaleAdopted live-source compile is dispatched DURING its package's install: source discovery under-matches, CS0246 on committed siblings, and CompileWatcher PARKS the transient failure #4280 state.AdoptedSourceStampLedgerholds the invariant that only a committed definition that no longer carries the request counts as a commit.
The negative control makes RecordCommit unconditional and reds the deferred case: 4 run, 1 failed.
What they do not pin. The wiring in InstallAdoptedSourceStampWatcher: that the mark is recorded only in onNext, from the committed node, and that the error arm does not record it.
A regression that moves the mark back into the lambda would have to re-thread the ledger into StampOnce. That function now takes no ledger, so the regression is harder to write by accident. It would still pass these tests, though.
A watcher-level test needs a real NodeType hub whose own-stream Update can be made to fault, or to confirm a deferred node, on demand. There is no such harness in MeshWeaver.Compiler.Pipeline.Test today, and I have not built one in this PR. Leaving this thread open as the record of that gap.
Test Results (shard 0) 1 files 1 suites 2m 41s ⏱️ Results for commit d11a9ad. |
Test Results (shard 1)396 tests 396 ✅ 56s ⏱️ Results for commit d11a9ad. |
Test Results (shard 4)1 606 tests 1 606 ✅ 2m 46s ⏱️ Results for commit d11a9ad. |
Test Results (shard 2) 1 files 1 suites 5m 54s ⏱️ Results for commit d11a9ad. |
Test Results (shard 3)943 tests 943 ✅ 7m 49s ⏱️ Results for commit d11a9ad. |
Test Results (shard 5) 2 files 2 suites 7m 22s ⏱️ Results for commit d11a9ad. |
Test Results 8 files 8 suites 27m 32s ⏱️ Results for commit d11a9ad. |
Refs #1105
What was wrong
InstallAdoptedSourceStampWatcher(src/MeshWeaver.Compiler.Pipeline/NodeTypeCompilationHelpers.cs) logs one Error: "the adopted build's source stamp was committed for request {RequestedAt} and the request is still standing with IsDirty=true — the write did not converge". That line is only true if "committed" means a write landed and consumed the request.The watcher advanced its commit mark inside the write's lambda, before the stamp was applied, so "committed" really meant "a pass ran". Two ordinary outcomes then produced the Error with nothing wrong:
ApplyAdoptedSourceStampreturns the definition untouched, with the request standing, while the live source set is still short of the paths the bundle was built from (A StaleAdopted live-source compile is dispatched DURING its package's install: source discovery under-matches, CS0246 on committed siblings, and CompileWatcher PARKS the transient failure #4280). TheWhereasked the same question on the emission. The lambda runs on the owner's current node, which can be earlier in an install than the emission was.In both cases the next emission is the one that should fulfil the request, and the mark reported it as a failure.
Evidence. This is the shape #1105's fingerprint now carries on both portals:
Hosting/Backup,Hosting/InstanceAction,Hosting/InstanceRequest,Hosting/PlatformBuildInbox,Store/Catalog,Store/Orderand others. Measured read-only on memex.meshweaver.cloud (39f49494eb):Store/Orderlogged the Error at 2026-09-22 22:54:24Z. At 22:55:38Z it readrequestedSourceStampAtabsent,currentSourceFingerprint == adoptedSourceFingerprint = b06c6eeee4daab3dandbuildProvenance: AdoptedVerified. The request had converged; only the log said otherwise.The fix
AdoptedSourceStampLedgerholds the mark, one instance per watcher install and never static. It advances in the write'sonNext, and only when the committed definition no longer carries the request (RecordCommit).StampOnce, which touches no mark.No bound, retry or level was changed on the non-convergence line itself.
Controls
AdoptedSourceStampCommitIsConfirmedTest(MeshWeaver.Compiler.Pipeline.Test) drives the productionStampOncewith the productionApplyAdoptedSourceStampover the #4280 "sources still landing" state:Total: 4, Failed: 0. The whole project:Total: 1085, Failed: 0.RecordCommitcounting any readable write as a commit (the old "a pass ran" meaning):Total: 4, Failed: 1. The failing case isADeferredJudgement_IsNotACommit_SoTheNextEmissionStampsInsteadOfReportingNonConvergence.Build
dotnet build -c Release -warnaserror, one project per invocation, each0 Error(s)/0 Warning(s):MeshWeaver.Compiler.PipelineMeshWeaver.Compiler.Pipeline.TestMeshWeaver.Hosting(dependent)MeshWeaver.DocumentationOnly
internalmembers were added, so there is no public surface change.Doc
Doc/Architecture/NodeTypeCompilationgains a new section: "The write did not converge" means a CONFIRMED stamp is still standing, and nothing less.What this does NOT establish
Crm/ClientPARKED, is a content leftover tracked on Orphaned Crm/Client NodeType node survives Phase C and no longer compiles against the deleted ClientContent/Counterparties types #5219. It is untouched here.Deploy
Nothing to recycle: the watcher is installed on each NodeType hub at activation, and a roll replaces them.
🤖 Generated with Claude Code