Compile state is mesh-owned — no upsert writer gets to overwrite it (#748 stage 2) by rbuergi · Pull Request #950 · Systemorph/MeshWeaver

rbuergi · 2026-08-08T09:28:43Z

Fixes #748.

Partial: this is a stage, not the whole issue. It closes the harm #748 opens with ("imports
stamped it back") at its root and records the constraint the remaining phases have to design
around. The field move itself is not done — see What is left below, with the reason.

The defect

A NodeType node carries two things with different owners: the AUTHORED definition (a repo, a
package, a user owns it) and the compile bookkeeping (only the mesh that ran the compile can know
it). Every upsert writer holds a copy of the second one that is stale by construction:

a repo file embeds whatever verdict it carried when it was exported,
an installer ships the package author's,
StaticRepoImporter's existing snapshot comes from path:{p} scope:descendants — the
eventually-consistent query index, which lags the compile pipeline because the compile does not
run under the importer's lock.

Whichever copy the writer held, it won. That is the mechanism behind the symptom recorded on this
issue (memex, 2026-08-02): four healthy SocialMedia types reverted to their pre-framework-roll
Pending with a latestReleasePath naming a release that had already rotated away. The compile
had succeeded; the satellite recorded it; the next re-import stamped the index's older view back
over the node. The mirror image is the documented stale-green class — a weeks-old Ok claiming an
assembly nobody still has, which looks healthy right up to the cold cache that parks the type.

StaticRepoImporter tried to hold this line client-side (PreserveLiveOperational against
target). It could only ever be as fresh as the index it read, and reading a single node's content
from the query is precisely the shape CQRS exists to forbid.

The fix

Move the ownership rule to the owner, inside CreateOrUpdateNodeRequest's merge
(MeshExtensions.PreserveMeshOwnedOperational), and delete the importer's client-side copy.

Authoritative. The owner's merge is the one place the question has a real answer.
Universal. Every upsert writer is covered — GitSync import, plugin install, webhook, instance
sync, MCP — instead of the one that remembered to patch it up.
Stronger than "read something fresher". Cross-hub, the members come out equal to what this
handle already holds, so they drop out of the RFC 7396 patch entirely and the owner keeps its
own. A stale mirror cannot even express the regression.
Deliberate writes are untouched. Every compile-state writer — the watchers,
RequestNodeTypeRelease, the MCP compile tool — goes through GetMeshNodeStream(path).Update,
never an upsert. Verified against the full writer inventory.

IsNoOpUpsert mirrors the merge, which is the cheaper half of the same harm: a re-import differing
only in bookkeeping used to take the write path and mint a Version for nothing — and that churn is
exactly what marks a NodeType "server-modified", so the two-way sync starts preserving a local edit
nobody made.

Absent-in-live stays absent: a verdict baked into a repo file can never seed a node that has none.

The constraint the rest of #748 must design around (recorded in `NodeTypeCompileState`)

CompilationStatus is the compile lock. HandleDispatchCompile transitions Pending → Compiling
inside the per-NodeType hub's OWN Update and dispatches Roslyn only when that lambda made the
transition — atomic solely because the hub owns the node it swaps on, so its action block serialises
every contender.

The satellite is a different path, therefore a different owner: MeshNodeStreamHandle.IsOwn is
plain path equality with the hub address, so from the NodeType hub the satellite is a remote
handle. A remote Update evaluates its lambda against the local mirror and ships the diff, so the
guard would be read off a snapshot that is not the owner's state at apply time — two contenders can
both conclude "I transitioned" and both start Roslyn, and the merge refusing one write afterwards
does not un-dispatch the compile that already began. The trigger pair
(RequestedReleaseAt / LastReleaseRequestHandledAt) has the same shape.

So the control plane moves exactly one of two ways, both single-owner: (a) the compile watchers
move with it onto the satellite's own hub, which then owns the CAS; or (b) the control-plane
members stay on the node and only the RESULT members move. Splitting the CAS across two owners is
the one design that must not ship — the double-compile it admits is the "hub unresponsive after the
second compile" wedge class.

What is left

Phase 1 (satellite + mirror) landed in #752. This PR is stage 2a. Still open:

The field move itself (issue phases 2–3): flip ~50 reader subsystems to the satellite, stop
writing the node members, re-point the DataExtensions monotonic-trigger merge guard, coordinated
MeshWeaver.Plugins PR for NodeTypeCompileGate / Provisioning. A full inventory taken for this
PR puts it at ~500 sites in src/ + memex/ and ~500 in test/, across the most wedge-prone
subsystem in the repo — not one PR, and the lock question above has to be answered first.
ReleaseNotes is mis-classified. It sits in MemberNames (mesh-owned) yet is user-authored
through a JsonPointerReference binding in NodeTypeLayoutAreas/NodeTypeConfigForm. With this
change an upsert can no longer carry it, which is right for a sync and wrong for the form —
the form writes via stream.Update, so nothing regresses today, but the classification should be
settled before the field moves.
PackageInstaller.IsUnchanged still carries the narrow NodeType special-case the issue calls
removable. Replacing it with a strip-both-sides compare has to be sequenced after AlignedIncoming
(typed content materializes C# defaults) or it re-opens the root-churn flap — deliberately not
touched here.
41 samples/Graph/Data/**/*.json NodeType files still embed compile verdicts (compilationStatus,
latestAssemblyPath, compiledSources, …). Export strips them now, so committing the stripped
form is safe — but it shifts first-compile timing for ~20 dynamic-NodeType tests, so it wants its
own PR.
The create path is unchanged. A first import can still seed a fresh node from a file's baked
verdict. Stripping there would break established fixtures that seed compile state on create, so it
needs its own decision.

Legacy persisted data

No migration, and none needed — nothing is removed from the shape. NodeTypeDefinition keeps
all 19 members, so every stored node deserializes exactly as before. The change is only about who
gets to write them: an existing node's values are now what survives an upsert instead of what the
writer brought. A node that never had them still has none.

(When phase 3 does remove the properties, [JsonExtensionData] UnknownMembers on NodeTypeDefinition
makes that deserialization-safe for stored nodes — confirmed present. This PR does not rely on it.)

Verification

Full solution, CI flags: dotnet build MeshWeaver.slnx -c Release -p:CIRun=true -warnaserror — clean,
0 warnings.

Suite	Result
`MeshWeaver.GitSync.Test` (full)	157/157 — incl. `NodeTypeOwnershipSyncTest`, the end-to-end proof that the import still keeps live compile state with the importer's own preserve deleted, and that the exported file carries no verdict
`MeshWeaver.PluginCatalog.Test` (full)	168/168 — install idempotence over the changed `IsNoOpUpsert`
`MeshWeaver.Graph.Test` (operational / compile-state / importer / conflict-policy)	42/42
`CreateOrUpdateNodeRequestTest`, `ReleaseRequestWatcherHighWaterTest`, `NodeTypeCompileParkTest`, `CompileSourceSnapshotWedgeTest`	14/14
`CodeEditRecompileTest`	11/11
`MeshWeaver.Documentation.Test`	19/19

New test: CreateOrUpdateNodeRequestTest.Upsert_WithStaleCompileState_TakesAuthoredChange_AndKeepsLiveCompileState
— an upsert carrying an authored change plus an older verdict lands the change and keeps the live
version, assembly pointer and status; an operational member absent on the live node stays absent.

The "exported form carries no compile state" assertion already exists and still passes
(NodeTypeOwnershipSyncTest, NodeTypeOperationalContentTest pins the member set against the record
in both directions) — not duplicated.

🤖 Generated with Claude Code

A NodeType node carries two things with different owners: the AUTHORED definition, which a repo / package / user owns, and the compile bookkeeping, which only the mesh that ran the compile can know. Every upsert writer holds a copy of the second one that is stale by construction — a repo file embeds the verdict it was exported with, an installer ships the package author's, and a sync's `existing` snapshot comes from the eventually-consistent query index, which lags the compile pipeline because the compile does not run under the writer's lock. Whichever copy the writer held, it won. That is how four healthy SocialMedia types on memex reverted to their pre-framework-roll `Pending` with a `latestReleasePath` naming a release that had already rotated away (2026-08-02): the compile succeeded, the satellite recorded it, and the next re-import stamped the index's older view back over the node. The mirror image is worse — a weeks-old "Ok" claims an assembly nobody still has, and the type looks healthy right up to the cold cache that parks it. StaticRepoImporter tried to hold this line client-side, carrying the operational members over from that same lagged snapshot. It could only ever be as fresh as the index it read, and reading one node's content from the query is the shape CQRS exists to forbid. Move the rule to the owner, inside the upsert's own merge, where the answer is authoritative and every writer is covered instead of the one that remembered: GitSync import, plugin install, webhook, instance sync, MCP. Cross-hub the effect is stronger than "read something fresher" — the members come out equal to what the handle already holds, so they drop out of the RFC 7396 patch entirely and a stale mirror cannot even express the regression. IsNoOpUpsert mirrors it, which is the cheaper half of the same harm: a re-import whose only difference was bookkeeping used to take the write path and mint a Version for nothing, and that churn is exactly what marks a NodeType "server-modified" so the two-way sync starts preserving a local edit nobody made. Deliberate compile writes are untouched — the watchers, RequestNodeTypeRelease and the MCP compile tool all go through GetMeshNodeStream(path).Update, never an upsert. Also records, on NodeTypeCompileState, the constraint the remaining phases of #748 have to design around: CompilationStatus IS the compile lock, and its compare-and-swap is atomic only because the NodeType hub owns the node it swaps on. The satellite is a different path, so a different owner, so a remote handle — a CAS evaluated there reads a snapshot that is not the owner's state at apply time, and two contenders can both conclude they transitioned and both start Roslyn. The control plane moves with its watchers or it does not move; splitting it across two owners is the one design that must not ship. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

Copilot

Pull request overview

This PR implements stage 2a of #748 by enforcing “compile state is mesh-owned” at the owner-side upsert merge (CreateOrUpdateNodeRequest), so no upsert writer (GitSync import, plugin install, instance sync, MCP, etc.) can overwrite live compile bookkeeping with a stale copy.

Changes:

Apply NodeType operational/compile-state preservation inside the upsert merge (and mirror it in the no-op upsert detector) so stale operational fields cannot land and unchanged re-imports become true no-ops.
Remove the importer’s client-side preservation (which could only be as fresh as the query index snapshot) and rely on the authoritative owner merge instead.
Add an end-to-end regression test covering “authored change lands + stale compile verdict does not regress live state,” plus a What’s New entry describing the user-visible effect.

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
test/MeshWeaver.Hosting.Monolith.Test/CreateOrUpdateNodeRequestTest.cs	Adds regression test proving an upsert can carry authored edits without regressing live compile state.
src/MeshWeaver.Mesh.Contract/NodeTypeOperationalContent.cs	Updates documentation to reflect the new owner-side “every upsert preserves live operational state” rule.
src/MeshWeaver.Mesh.Contract/MeshExtensions.cs	Enforces operational-state preservation in the upsert merge and applies the same rule in `IsNoOpUpsert` comparisons.
src/MeshWeaver.Graph/StaticRepoImporter.cs	Removes importer-side preservation and documents why it must not rely on eventually-consistent query snapshots.
src/MeshWeaver.Graph/Configuration/NodeTypeCompileState.cs	Records the compile-lock ownership constraint for later phases of moving compile state to a satellite.
src/MeshWeaver.Documentation/Data/WhatsNew/2026-08-08-nodetype-compile-state-survives-sync.md	Adds a What’s New entry describing that sync/install/import no longer reverts NodeType compile state.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

github-actions · 2026-08-08T09:44:34Z