A rebased node keeps its main-node pointer (#2939) by rbuergi · Pull Request #2949 · Systemorph/MeshWeaver

rbuergi · 2026-09-01T08:39:46Z

Closes #2939. One of two required halves — see "Not sufficient on its own" below. Pairs with Systemorph/MeshWeaver.Plugins#1053 (the origin), which merges after this.

The defect

MeshNode.MainNode is a stored, non-nullable init property whose default is evaluated once, at construction. Path is computed. So a record rebase moves one and not the other:

var minted  = new MeshNode("deployment", "Skill");          // MainNode = "Skill/deployment"
var rebased = minted with { Namespace = "Hosting/Skill" };  // Path     = "Hosting/Skill/deployment"
                                                            // MainNode = "Skill/deployment"  ← stale

Because the field can never be null, "the writer never touched it" and "the writer meant this" are the same bytes — HasExplicitMainNode reads true for the stale value, so every writer downstream persists it as deliberate. The node drops out of is:main (SQL n.main_node = n.path) while get returns it, Active and fully formed. Nothing errors, nothing logs, no status flips.

Seven live nodes on memex.meshweaver.cloud: Hosting/Skill/{deployment,deployment-activity,instance,platform-update}, Essentials/Skill/email, RemoteControl/Skill/remote, Store/Skill/ci-policy. A reachability sweep across all 53 packages — reading the mesh by two independent paths (get vs search) and reporting the disagreement — found the unreachable set to be exactly the set where main_node != path, with no false positives.

Reproduced live, not argued

Measured on the memex portal (full evidence is commented on #2939):

search nodeType:Skill limit:200        →  count: 71, truncated: false     (a COMPLETE listing)
get    @Hosting/Skill/deployment       →  Active, version: 7, full content — and NOT among the 71
                                          path:     Hosting/Skill/deployment
                                          mainNode: Skill/deployment
get    @Hosting/Skill/setup-environment→  mainNode == path — and it IS in the listing

Same partition, same node type, same import path; the only difference is the pointer. All seven are get-able and absent from the listing.

This also measures the 1b′ guard's gap rather than asserting it. The stale values are two-segment defaults (Skill/deployment), not the one-segment MainNode == Id the old guard triggered on — so the guard looked at these nodes and correctly found nothing matching its rule. That is precisely the shape the second arm added here catches.

🚨 Not sufficient on its own

Restoring MainNode == Path does not by itself make a decentral node searchable. A second, independent defect produces the identical symptom — #2942: MeshQueryRequest.FromQueries builds a union but fills the legacy single Query field with list[0]; StaticNodeQueryProvider reads Query while StorageAdapterMeshQueryProvider iterates EffectiveQueries. A static node matched only by query #2 is silently absent — and the skill query set leads with the platform catalog and follows with the package partition, i.e. exactly query #2.

Both halves are required; fixing either alone leaves the decentral skills unreachable. This PR is the stored-value half, and claims nothing about search on its own.

What changed

1. One rebase API — MeshNode.WithPath(id, ns) / WithNamespace(ns). Moves Path and MainNode together; preserves an AUTHORED MainNode (a package's _Access grant's mainNode IS its scope). with { Namespace = … } is now the shape to grep for.

PackageInstaller.ParseCanonical uses it. Its MainNode = parsed.MainNode ?? (…) was dead code — the field is non-nullable, so the right-hand side never ran, and the stated intent (preserve an authored mainNode) cannot be expressed with a null check. 🚨 The install bulk path writes through persistence.WriteManyAndPublishCreated, bypassing HandleCreateNodeRequest entirely (measured — see the negative control below), so this line is the only defence on an install.

2. One repair guard, now covering updates. The 1b′ repair was pasted into the singular and the batch create path. It is now one helper, RepairStaleSelfDefaultMainNode, used by both — plus CreateOrUpdateNodeRequest, on the merged node. That last one matters: a full-instance upsert can move a MainNode anywhere except back onto the node's own path (that intent is exactly what reads as untouched — MeshNode.MainNode's own remarks), so a re-import compared equal, skipped as a no-op, and left the pointer wrong forever. IsNoOpUpsert now knows the stored row may itself need repair.

The trigger stays deliberately narrow — both halves are load-bearing:

Shape	Repaired?	Why
`MainNode == Id` on a namespaced node	✅	the pre-existing rule (the 42P01 phantom-partition bug)
ends with `/{Id}` and a different first segment	✅	a self-default frozen in another partition
a parent inside its own partition	❌	`GitHubSyncConfig`'s `MainNode = spacePath`; a `~/Threads` tile targeting `{owner}/Threads`
another partition under a different id	❌	a tile targeting `Store/Foo` with id `Store-Foo`
any satellite node type	❌	step 1b points it at its owner

An over-reaching repair is the reverse defect (#2383): it promotes a satellite to a main node and re-scopes its grants.

3. The rest of the class. Sweep of with { Namespace = … } on a MeshNode across both repos: 4 sites carried the defect and now use WithPath — EmbeddedResourceStorageAdapter, DocumentationNodeProvider, MeshOperations.NormalizeNamespace (an agent's @/ACME/Projects namespace was normalised, its MainNode was not) and SanitizeNodeId. FileSystemStorageAdapter, CachingStorageAdapter, PathRemappingStorageAdapter and the move rebase already handled MainNode explicitly and are untouched. Plugins had exactly one, fixed in the paired PR.

Two sites deliberately not changed: PackageInstaller.ParseNode:3345 and GitHubSyncService.cs:707 assign MainNode = "{rebasedNs}/{id}" unconditionally. They heal a stale default but clobber an authored one — the opposite defect. Rebasing an authored pointer into a partition means prefixing it, which is a judgement call that does not belong in this change set.

4. The finding is conserved — Doc/Architecture/MainNodeRebasing, linked from the Architecture index, including the two-defects-one-symptom section.

Public surface

Added only (WithPath, WithNamespace) — nothing renamed or removed, so no in-mesh caller the compiler cannot see can break.

Verified

Every touched project + dependents built -c Release -warnaserror, one project per invocation: 0 Warning(s), 0 Error(s).

Tests, all green: MeshNodeRebaseTest 7, PluginCatalog install suites 16, PartitionRootBootstrapTest + CreateOrUpdateNodeRequestTest + NodeVersionCountsRealChangesTest 28, Hosting.Test 269, NodeOperations.Test 76, Documentation.Test 148.

Negative controls — every new test was proven able to fail. With the ?? restored and the guard's second arm disabled:

NodeRepoMainNodeRebaseTest → Expected "Gadget/Skill/deployment" … but found "Skill/deployment"
PartitionRootBootstrapTest.MainNode_StaleNamespacedSelfDefault… → same shape
CreateOrUpdateNodeRequestTest.Upsert_HealsAStoredStale… → same shape

Then, with the guard restored but ParseCanonical still inverted, the install test still failed — which is how we know the installer bypasses the create guard and that ParseCanonical is independently pinned. Two negative-control tests (a deliberate cross-partition pointer; a self-default-shaped pointer inside its own partition) passed throughout. The doc page also failed DocumentationLinkIntegrityTest once on bare sibling links before they were made absolute.

Re-verified after merging origin/main (six commits, one conflict in Architecture.md where both sides added a table link — resolved by keeping both; git diff origin/main confirms nothing of main's was dropped).

Not in this PR — the live data repair is OUTSTANDING

This change does not repair the seven existing rows. It stops new ones being created, and it lets an upsert heal a stale pointer if and when one touches that node — but nothing here reaches out and fixes stored data. The repair is a separate, explicit action on a live portal:

GetMeshNodeStream(path).Update(n => n with { MainNode = n.Path })

for Hosting/Skill/{deployment,deployment-activity,instance,platform-update}, Essentials/Skill/email, RemoteControl/Skill/remote, Store/Skill/ci-policy.

Acceptance for that repair is the same two-read check that found the defect: search nodeType:Skill goes from 71 → 78. Anyone can verify it; it is not a claim only the repairer can check.

Sequencing is deliberate — the repair runs after this merges. Repairing first would let the next install re-corrupt the rows, and the work would have to be done twice.

And a reminder from above: even a repaired pointer does not restore search visibility on its own. #2942 is the other half.

🤖 Generated with Claude Code

https://claude.ai/code/session_01Kwar1ZGt5FMEpHiXmkzKCm

…ndable `MeshNode.MainNode` is a STORED, non-nullable init property whose default is evaluated once, at construction. `Path` is computed, so a `with { Namespace = … }` copy moves the path and leaves `MainNode` naming the namespace the node was BORN in — and because the field can never be null, that leftover is indistinguishable on the wire from a deliberate satellite pointer, so every writer downstream persists it. The node then drops out of `is:main` (SQL `n.main_node = n.path`) while `get` still returns it, Active and fully formed, with nothing logged. Three changes: 1. **One rebase API.** `MeshNode.WithPath(id, ns)` / `WithNamespace(ns)` move the path and `MainNode` together and preserve an AUTHORED `MainNode` (`HasExplicitMainNode`). `PackageInstaller.ParseCanonical` now uses it — its `MainNode = parsed.MainNode ?? (…)` was DEAD CODE (the field is non-nullable, so the right-hand side never ran) and the intent it states cannot be expressed with a null check. The install path writes through `persistence.WriteManyAndPublishCreated`, bypassing the create-request guard entirely, so this line is the only defence there. 2. **One repair guard, and it covers updates.** The 1b′ repair was pasted into the singular and the batch create paths; it is now one helper (`RepairStaleSelfDefaultMainNode`) used by both plus `CreateOrUpdateNodeRequest`, on the MERGED node — a full-instance upsert provably cannot move a `MainNode` back onto the node's own path, so a re-import used to compare equal, skip as a no-op, and leave the row invisible forever. `IsNoOpUpsert` now knows that too. The trigger stays narrow: MainNode's LAST SEGMENT is this node's own id AND it names a different partition. Either half alone over-reaches (a `~/Threads` tile targeting `{owner}/Threads`; a tile targeting `Store/Foo` with id `Store-Foo`). 3. **The other rebase sites.** `EmbeddedResourceStorageAdapter`, `DocumentationNodeProvider`, `MeshOperations.NormalizeNamespace` and `SanitizeNodeId` all carried the same defect and now use `WithPath`. Doc: Doc/Architecture/MainNodeRebasing — the trap, the rule, the guard's trigger table, and the two incidents (#2383, #2939) it has caused. Pairs with Systemorph/MeshWeaver.Plugins#1053, which fixes the origin: the skill importer minted every `.md` skill in the platform `Skill` partition and rebased afterwards. Merge this first. Public surface is ADDED only (`WithPath`, `WithNamespace`) — nothing renamed or removed, so no in-mesh caller can break. Closes #2939 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Kwar1ZGt5FMEpHiXmkzKCm

Copilot

Pull request overview

This PR fixes a subtle but high-impact integrity bug where rebasing a MeshNode via a record copy updates the computed Path but can leave the stored MainNode pointing at the old location, making otherwise-healthy nodes vanish from is:main-based search/listing while still being readable via get.

Changes:

Added MeshNode.WithPath(id, ns) / WithNamespace(ns) to rebase Path and (when not explicitly authored) MainNode together.
Centralized and broadened the “stale self-default MainNode” repair so it applies consistently across create, batch create, and upsert/update flows (including no-op upsert detection).
Added targeted regression tests plus documentation/What’s New entries to capture the failure mode and the safe rebase rule.

Reviewed changes

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

Show a summary per file

File	Description
test/MeshWeaver.PluginCatalog.Test/NodeRepoMainNodeRebaseTest.cs	Regression test proving install rebases keep nodes as their own main node unless `MainNode` was explicitly authored.
test/MeshWeaver.Hosting.Monolith.Test/PartitionRootBootstrapTest.cs	Adds create-path tests for the deeper “namespaced stale self-default from another partition” shape plus negative controls.
test/MeshWeaver.Hosting.Monolith.Test/CreateOrUpdateNodeRequestTest.cs	Adds update/upsert regression test ensuring re-import heals already-corrupted rows that a source node can’t express as “self-main”.
test/MeshWeaver.Data.Test/MeshNodeRebaseTest.cs	Unit tests codifying the record-copy trap and verifying `WithPath`/`WithNamespace` behavior (including explicit `MainNode`).
src/MeshWeaver.PluginCatalog/PackageInstaller.cs	Switches canonical node rebasing to `WithPath` to avoid freezing stale `MainNode` during install.
src/MeshWeaver.Mesh.Operations/MeshOperations.cs	Replaces `with { Namespace/Id = … }` rebases with `WithNamespace`/`WithPath` to keep `MainNode` aligned when it wasn’t explicitly set.
src/MeshWeaver.Mesh.Contract/MeshNode.cs	Introduces the `WithPath`/`WithNamespace` APIs with detailed guidance on why plain record copies are unsafe here.
src/MeshWeaver.Mesh.Contract/MeshExtensions.cs	Consolidates the repair rule into `RepairStaleSelfDefaultMainNode`, applies it to create + batch create + upsert, and ensures no-op detection doesn’t skip required repairs.
src/MeshWeaver.Hosting/Persistence/EmbeddedResourceStorageAdapter.cs	Uses `WithPath` during served-path normalization to prevent stale `MainNode` when aligning Id/Namespace.
src/MeshWeaver.Documentation/DocumentationNodeProvider.cs	Uses `WithPath` when aligning doc-node Id/Namespace to the served path so `MainNode` stays consistent.
src/MeshWeaver.Documentation/Data/WhatsNew/2026-09-01-a-node-moved-into-a-partition-stays-findable.md	User-facing release note describing the bug and the fix.
src/MeshWeaver.Documentation/Data/Architecture/MainNodeRebasing.md	Architecture write-up documenting the trap, the rule, and the repair guard’s intentionally narrow trigger.
src/MeshWeaver.Documentation/Data/Architecture.md	Adds the new architecture page to the index.

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

…tMainNode, never Path Copilot review, #2949. The doc comment claimed allocation-free while the body called `node.Path`, a computed interpolation that allocates a string per call — on a predicate that runs on every create and every upsert in the mesh. Both uses are gone. "Is MainNode this node's own path" is exactly MeshNode.HasExplicitMainNode, which already compares segment-wise for this reason, and the partition comparison runs on Namespace directly — its first segment IS the node's partition, so Path never has to be built. An empty namespace now short-circuits arm (b) explicitly instead of relying on the caller's guard. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Kwar1ZGt5FMEpHiXmkzKCm

github-actions · 2026-09-01T08:54:20Z