Skip to content

A stale MainNode from a plugin's .md Skill survives install: the ?? in ParseCanonical is dead code, and the 1b' guard misses the shape #2939

Closed

Description

@rbuergi

Corrected 2026-09-01. The first version of this issue placed the origin in core's
MarkdownFileParser. That was wrong. The stale MainNode is minted in MeshWeaver.Plugins
(src/MeshWeaver.AI/Persistence/SkillFileParser.cs) — tracked as
Systemorph/MeshWeaver.Plugins#1053. This issue is now scoped to the three core behaviours that
let a stale MainNode land and stay: a dead null-coalesce that cannot heal it, a repair guard that
misses this shape, and the already-corrupted rows.

What happens

A Skill node authored as .md inside a plugin partition ends up with MainNode pointing at the
platform Skill/ partition instead of its own path. The node is written, state: Active,
current — and invisible to every search, because the catalog's is:main projection is SQL
n.main_node = n.path.

Measured on memex.meshweaver.cloud, 2026-09-01:

get    @Hosting/Skill/deployment      →  the full node:
         path:         Hosting/Skill/deployment
         mainNode:     Skill/deployment           ← should be Hosting/Skill/deployment
         namespace:    Hosting/Skill              ← correctly rebased
         version: 7   lastModified: 2026-08-30T20:22:35Z   lastModifiedBy: system-security
         state:   Active

search "path:Hosting/Skill/deployment"   →  count: 0

Namespace is rebased into the partition; MainNode is not.

Blast radius — measured

Corrupted (all authored as .md): Hosting/Skill/{deployment,deployment-activity,instance,platform-update},
Essentials/Skill/email, Store/Skill/ci-policy.

Correct: every .json-authored skill (Hosting/Skill/setup-environment, Observability/Skill/debug,
Store/Skill/plugin, Essentials/Skill/models) — those declare mainNode explicitly or never enter
the markdown path — plus every .md skill whose last write predates the trigger below
(Essentials/Skill/communicate, Store/Skill/coupon, RolePlay/Skill/scene, Store/Skill/application).

The file format is the discriminator, not the date. The built-in Skill/* roster is unaffected
because Skill/{id} is its correct home.

Origin (in Plugins — see MeshWeaver.Plugins#1053)

MeshNode.MainNode (src/MeshWeaver.Mesh.Contract/MeshNode.cs:96) is a stored, non-nullable init
property whose default is evaluated once, at construction
:

public string MainNode { get; init; } = string.IsNullOrEmpty(Namespace) ? Id : $"{Namespace}/{Id}";

SkillMarkdown.TryParse mints every skill as new MeshNode(id, SkillNodeType.RootNamespace) —
freezing MainNode = "Skill/{id}" — and SkillFileParser then rebases with
node with { Namespace = ns }, which does not re-run the initializer. AgentFileParser does the
same job correctly by constructing new MeshNode(id, ns) up front, which is why Agent nodes in the
same partitions are fine.

Core's three contributions

1. PackageInstaller.ParseCanonical cannot heal it — the ?? is dead code

src/MeshWeaver.PluginCatalog/PackageInstaller.cs:3218:

// Preserve an AUTHORED mainNode: an _Access grant's mainNode IS its scope …
MainNode = parsed.MainNode ?? (string.IsNullOrEmpty(ns) ? id : $"{ns}/{id}"),

🚨 MainNode is non-nullable and always populated, so the right-hand side never runs. The
intent — preserve an authored mainNode — is correct and necessary, but a null check cannot
distinguish "authored" from "defaulted from a different namespace"
. MeshNode.HasExplicitMainNode
(MeshNode.cs:130) is the predicate built for exactly this, and PackageInstaller.cs:2216 already
uses it correctly a thousand lines away.

The two sibling rebase sites assign unconditionally and would have healed it:

  • PackageInstaller.cs:3342 — MainNode = $"{rebasedNs}/{id}",
  • GitSync/GitHubSyncService.cs:707 — parsed with { Id = id, Namespace = rebasedNs, MainNode = $"{rebasedNs}/{id}" }

Fix: MainNode = parsed.HasExplicitMainNode ? parsed.MainNode : (string.IsNullOrEmpty(ns) ? id : $"{ns}/{id}"),

2. The 1b′ repair guard misses this shape, and runs only on create

src/MeshWeaver.Mesh.Contract/MeshExtensions.cs:713-733 (duplicated at :1332-1341) repairs a
stale self-default MainNode — but its trigger is node.MainNode == node.Id, a one-segment
stale default. Ours is a two-segment stale default (Skill/deployment), which by shape alone is
indistinguishable from the legitimate parent-pointer case the comment calls out
(GitHubSyncConfig.MainNode = spacePath). It also lives only in HandleCreateNodeRequest, while the
importer writes via CreateOrUpdateNodeRequest → UpdateAccordingToSourceNode.

Worth considering: a non-satellite node whose MainNode names a path outside its own partition
is not a state any authoring path intends, and could be refused or warned rather than stored silently.

3. Why it started landing on 2026-08-29 — #2652 is the trigger, and is not the bug

This was latent since 2026-08-27 (when the .md skill parser shipped). Before #2652
("let an upsert move a node's MainNode", merged 2026-08-29T08:42:23Z, fixing #2631),
IsNoOpUpsert never compared MainNode and UpdateAccordingToSourceNode kept the stored one — so
the wrong value was silently discarded on every re-import and the correct stored value survived.

#2652 gated the move on HasExplicitMainNode (MeshExtensions.cs:4289, :4355). For these nodes
that returns true (Skill/deployment ≠ Hosting/Skill/deployment), so the parser's frozen
default is read as a deliberate satellite pointer and persisted. #2652 is correct; it unmasked
this.

This also explains a second measured oddity: files unchanged in the repo since 08-19/08-23/08-26
acquired lastModified of 2026-08-30. Before #2652 an unchanged skill re-import was a genuine no-op;
after it, the MainNode difference alone flips it to the write path.

Why this is worse than a drop

  • get returns the node, fully formed and Active — it looks perfectly healthy.
  • search cannot see it, so search nodeType:Skill scope:subtree — the documented way to find
    skills, and how SkillNodeType.SkillQueries targets them — returns a set with silent holes.
  • Nothing errors, nothing logs, no status flips.

It cost a full investigation on a downstream repo, from the confident and wrong premise that a
GitSync reporting success had silently failed to import four nodes.

Repair

Re-importing does not fix the existing rows — it writes through the same path. They need an
explicit re-stamp, which MeshNode.cs:122 already names as the only route that can restore a main
node (a full-instance upsert provably cannot):

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

for every non-satellite node where main_node <> path and main_node does not resolve within the
node's own partition. Affected on memex.meshweaver.cloud today: Hosting/Skill/{deployment, deployment-activity,instance,platform-update}, Essentials/Skill/email, Store/Skill/ci-policy.

What was verified by running vs read from source

Verified by running (MCP against memex.meshweaver.cloud): every mainNode/path/namespace/
lastModified value quoted; that get returns the nodes while search returns count: 0; that the
four Hosting skills were present in the synced commit's tree; that Hosting/Guide
(nodeType: Markdown) rewritten 2026-08-31T05:00:56Z is search-visible, so the generic markdown
importer is not implicated; that Observability/Skill/debug is debug.json. #2652's merge timestamp
and changed files read from the GitHub API.

Read from source, not executed: the call chain SkillMarkdown.TryParse → SkillFileParser →
ParseCanonical, and the HasExplicitMainNode evaluation. Every line is quoted with a file:line
above. No repro test was run.

Related

🤖 Generated with Claude Code

Activity

  1. changed the title [-]A package install writes a Skill node's MainNode from the package-relative path, so the node exists to get() and is invisible to search()[/-] [+]A stale MainNode from a plugin's .md Skill survives install: the ?? in ParseCanonical is dead code, and the 1b' guard misses the shape[/+] on Sep 1, 2026
  2. rbuergi commented on Sep 1, 2026

    @rbuergi
    ContributorAuthor

    Reproduced live on the memex portal today, which pins both the mechanism and the exact blast radius. Full evidence in MeshWeaver.Plugins#1053; the short version:

    Two reads of the same production node disagree —

    • search nodeType:Skill limit:200 → count: 71, truncated: false (a complete listing)
    • get @Hosting/Skill/deployment → returns the node, Active, version: 7, full content

    — and the node is not among the 71. The stored pointer is why:

    path:     Hosting/Skill/deployment
    mainNode: Skill/deployment          ← Namespace rebased, MainNode left behind
    

    A healthy sibling in the same partition, Hosting/Skill/setup-environment, carries mainNode: Hosting/Skill/setup-environment and does appear.

    Seven nodes affected, all get-able and all absent from the listing: Hosting/Skill/{deployment, deployment-activity, instance, platform-update}, Essentials/Skill/email, RemoteControl/Skill/remote, Store/Skill/ci-policy. That is the same set an independent reachability sweep across all 53 packages found, arrived at by a different method — and it is exactly the set where main_node != path, zero false positives.

    Two notes that bear on this issue's scope:

    1. The 1b′ guard's shape is confirmed as the gap. These are two-segment stale defaults (Skill/deployment), not the one-segment MainNode == Id shape the guard triggers on — so the guard sees nothing to repair, exactly as this issue states.
    2. The code fix is necessary but not sufficient. A second, independent defect produces the same symptom: fix(query): the static-node provider must evaluate EVERY query of a union, not only the first #2942 (FromQueries fills the legacy single Query with list[0], and StaticNodeQueryProvider reads Query while StorageAdapterMeshQueryProvider iterates EffectiveQueries). The skill query set puts the package partition second, so a node matched only by query Test #2 is dropped regardless of its MainNode.

    Live data repair is deliberately sequenced after this merges — with the defect still live, the next install can re-corrupt the rows and the repair would have to be redone. Acceptance test for the repair is the same two-read check: the search nodeType:Skill count should go 71 → 78.

  3. added a commit that references this issue on Sep 1, 2026
  4. rbuergi commented on Sep 1, 2026

    @rbuergi
    ContributorAuthor

    🚨 It is not a stale default — it is a MUTUAL MainNode CYCLE, and the corruption protects itself from repair

    Attempted the live repair on memex after this issue's fix merged. It cannot be done through the normal surface, and finding out why changed what this issue is about.

    Two nodes, each naming the other as its main node

    get @Hosting/Skill/deployment
      path:     Hosting/Skill/deployment
      mainNode: Skill/deployment              ← names the platform copy
      version:  7   created 2026-08-19   state: Active
    
    get @Skill/deployment
      path:     Skill/deployment
      mainNode: Hosting/Skill/deployment      ← names the package copy
      version:  3   created 2026-08-26   state: Active
    

    Both Active, both carrying the full SkillDefinition, with byte-identical instructions. Neither is self-referential; each points at the other. So the shape is not "the writer left MainNode at its construction default" — it is a two-node cycle created by two writers that each rebased the other half.

    That matters for the fix already merged: MainNode == Id (one segment) and "points outside its own partition" (two segments) both describe a node pointing at a default. Neither describes a pair pointing at each other, and a guard that repairs one end of this cycle by setting MainNode = Path fixes that node while leaving the other still pointing at it — which is fine — but a guard that instead follows MainNode to find "the real main node" will loop.

    The repair is blocked BY the defect

    patch @Hosting/Skill/deployment  {"mainNode": "Hosting/Skill/deployment"}
    → Access denied: user 'rbuergi' lacks Update permission on 'Hosting/Skill/deployment'
    

    rbuergi is a global admin and holds Provider Admin in the neighbouring partition. The denial happens because authorization resolves against MainNode, not Path — the same delegation CheckDeletePermissionForNode performs (hub.GetEffectivePermissions(node.MainNode ?? node.Path, userId)). So the permission check on Hosting/Skill/deployment is answered by Skill/deployment's scope, in the platform Skill partition.

    The consequence is the important part: a node with a wrong MainNode delegates its own authorization to a partition it does not belong to. That is not only why the repair is refused — it is a standing access-control fact. For these seven nodes, who may edit them is currently decided by the wrong partition's grants. It happens to fail closed here; there is no reason in the mechanism why it must.

    What this means for the repair

    The "re-stamp the rows with GetMeshNodeStream(path).Update(n => n with { MainNode = n.Path })" plan in this issue and in Plugins#1053 will not work as an ordinary authenticated write. The repair has to run somewhere authorization is not taken from the field being repaired:

    • as system (ImpersonateAsSystem) in a Repair migration — the sanctioned shape per AGENTS.md, which also forbids the raw psql UPDATE alternative; or
    • through a lifecycle op that routes rather than mutates content.

    I stopped rather than reaching for either: a system-identity write against production access-control metadata is a change of a different kind from re-stamping a pointer, and it should be a reviewed change with a test, not something I improvise from an MCP session.

    Also worth folding into the guard's acceptance

    The invisibility is stronger than measured earlier. It is not only nodeType:Skill that misses them — a path-scoped query does too:

    search "path:Hosting/Skill scope:descendants"  →  count: 1
      Hosting/Skill/setup-environment      (the one healthy node in that partition)
    

    Four of the five Hosting skills are absent from a query that names their own container. So any acceptance test phrased as "the node appears in a listing" needs to state which listing; the seven are missing from every query shape I tried, and reachable only by exact get.

    Status: the seven rows are still corrupt on memex. The code fix (merged) stops new ones being minted; it does not repair these, and the repair is a bigger piece of work than the issue currently describes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions