Repository navigation
Built-in _Policy nodes carry their MainNode, so they stop listing as content (#2383) - #2388
Merged
Merged
Conversation
…content (#2383) Six built-in providers mint a partition's _Policy as `new MeshNode("_Policy", ns) { ... }` and never set MainNode. The plain constructor defaults MainNode to the node's OWN path (MeshNode.cs:96), while line 329 of the same type states the contract every satellite is supposed to meet: "Satellite nodes' MainNode is set to their primary node path at creation time." The catalog's `is:main` predicate is literally `MainNode != Path`, so a _Policy minted this way is a main node by definition -- and "Access Policy" shows up under Contents on the Agent, Skill, LanguageModel, Harness and Doc covers. The listing was never wrong; the mint was. Each of the six initializers now sets `MainNode = <partition root>`. These are static seeds re-registered on every boot, so the corrected field takes effect on the next deploy with no repair step. (The Store-package half -- the gating reconcile in MeshWeaver.Plugins, which sets MainNode = itself EXPLICITLY and never re-checks it -- is a separate converging-predicate change in that repo; see #2383.) MeshNode.Satellite(id, mainNode) is added for new satellites, with a test pinning both that it points at its main node and that the plain constructor does NOT -- the trap this closes. Not an access defect: MeshWeaver.Security never reads MainNode; PartitionAccessPolicy resolves by path. Cosmetic, on the pages whose job is to sell. Verified: Mesh.Contract, AI and Documentation build 0 errors under -warnaserror; 2/2 tests.
Contributor
There was a problem hiding this comment.
Pull request overview
This PR fixes a metadata contract violation where built-in partition _Policy nodes were minted with MainNode defaulting to their own path, causing them to be treated as “main” nodes and show up under “Contents” on package covers. It introduces a small factory to mint sibling satellites correctly and updates the built-in _Policy seeds accordingly, with a regression test and a What’s New entry.
Changes:
- Add
MeshNode.Satellite(id, mainNode)to create sibling satellite nodes withMainNodepointing at the primary node. - Set
MainNodeto the partition root for built-in_Policynodes (AI providers + Documentation seed) so the catalog’sis:mainfilter excludes them. - Add a unit test that pins both the correct satellite behavior and the “plain constructor trap”, plus a What’s New entry.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| test/MeshWeaver.Data.Test/MeshNodeSatelliteTest.cs | Adds regression coverage for MeshNode.Satellite and the default MainNode behavior. |
| src/MeshWeaver.Mesh.Contract/MeshNode.cs | Introduces the MeshNode.Satellite factory and documents the satellite contract. |
| src/MeshWeaver.Documentation/DocumentationExtensions.cs | Fixes the Doc partition _Policy seed to point MainNode at the partition root. |
| src/MeshWeaver.Documentation/Data/WhatsNew/2026-08-26-access-policy-no-longer-listed-as-content.md | Documents the user-visible behavior change in What’s New format. |
| src/MeshWeaver.AI/BuiltInSkillProvider.cs | Fixes built-in Skill partition _Policy seed MainNode. |
| src/MeshWeaver.AI/BuiltInLanguageModelProvider.cs | Fixes built-in Model Provider + Language Model partition _Policy seed MainNode. |
| src/MeshWeaver.AI/BuiltInHarnessProvider.cs | Fixes built-in Harness partition _Policy seed MainNode. |
| src/MeshWeaver.AI/BuiltInAgentProvider.cs | Fixes built-in Agent partition _Policy seed MainNode. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Contributor
Contributor
Contributor
Contributor
Contributor
Contributor
Contributor
…ites _Access Two findings on #2388, both correct: - MeshNodeSatelliteTest declared namespace MeshWeaver.Mesh.Contract.Test inside MeshWeaver.Data.Test, breaking the project convention and namespace-based filters. - The MeshNode.Satellite doc listed _Access as an example. Access assignments are not sibling nodes with id _Access; they live under a _Access/ NAMESPACE (Doc/_Access/{id}), so the example would have prompted exactly the wrong usage. Replaced with the distinction. Rebuilt with -warnaserror; 2/2 tests pass under the corrected namespace.
Contributor
Author
|
Both correct, both fixed in the follow-up commit: the test now sits in |
rbuergi
added a commit
that referenced
this pull request
Aug 26, 2026
MeshNode.MainNode self-defaults to the node's own path, and the framework's compensating normalizer (MeshExtensions.NormalizeSatelliteMainNode, applied on both the CreateNode and UpsertNode paths) only fired for node types flagged IsSatelliteType -- the ones that also live in their own satellite TABLE. The `_`-prefixed SIBLINGS that share mesh_nodes -- `_Policy`, `_Provider`, `_GitSync`, `_DefaultInstallLedger` -- were normalised by NOTHING, so every writer had to remember the field by hand. Fifteen of them exist in core; none did. `is:main` KEEPS exactly the rows where MainNode == Path (PostgreSqlSqlGenerator emits `n.main_node = n.path`; StaticNodeQueryProvider skips a node when MainNode != Path), so a satellite is normally excluded PRECISELY because its MainNode differs from its path -- and one pointing at itself passes instead, as a main node. search_across_schemas hard-filters every union branch on the same predicate UNCONDITIONALLY, not only when the caller asks for is:main. That is why "Access Policy" listed under Contents on the package covers, and why the row also reached mesh-wide search. #2388 corrected six of the fifteen by hand. The grep behind it -- `new MeshNode("_` -- cannot match the target-typed spelling `new("_Policy", ns)`, and two of the writers are raw SQL, so nine were missed. This change fixes the root rather than repeating that sweep: * NormalizeSatelliteMainNode now also fires on a satellite-shaped ID (SatelliteTableMapping.IsSiblingSatellite), so CreateNode/UpsertNode derive the field for `_Policy` and every future sibling. Flagging those types instead was not an option: the flag also RELOCATES the node (CreateLayoutArea files a flagged type under `{ns}/_{Type}/{id}`) and routes it to a satellite table, which would hide `{ns}/_Policy` from the permission evaluator's `id = '_Policy'` lookup. * The nine missed writers are corrected: DocumentationNodeProvider (the SECOND Doc policy mint), RoleNodeType, LicenseNodeType, GlobalSettingsNodeType, AccessControlLayoutArea, InstanceAutoRegistrationService, AssignmentNodeFactory -- plus PostgreSqlAccessControl and SnowflakeAccessControl, which write SQL directly and bypass the handler entirely. Static seeds bypass the handler too, which is why the mint fixes are still needed alongside it. * A guard sweeps the static seeds the mesh actually serves, so the next writer that forgets is caught by a control rather than by a grep. * #2388's remark stated the is:main predicate BACKWARDS ("is:main == MainNode != Path") in seven places; that is the EXCLUSION test, not the selection test, and it now reads the way the code behaves. Caught in review -- the phrasing was inherited from the issue rather than checked against StaticNodeQueryProvider. Two traps the classifier has to avoid, both real nodes in this repo and both pinned by tests: a PARTITION may legitimately be named with a leading underscore (GlobalSettingsNodeType's SettingsPath is literally `_Setting`), so the owner is still derived from OwnerOfSatellitePath over the NAMESPACE rather than by scanning the path -- a path scan resolves `_Setting/_Policy` to "", the empty prefix that COALESCE(main_node, namespace) projects as a ROOT-scope grant. And `Admin/Partition/_Access` DECLARES the global root-scope access partition; its id is that partition's NAME, so it is a main node of the catalog that lists it. The guard found that second one on its first run. Snowflake note: `main_node` and `path` shared one variable there, so the INSERT wrote the policy's own path into BOTH columns. They are now separate -- `path` is unchanged. Not an access-control bypass. Both policy readers resolve by `id = '_Policy'` keyed on namespace -- rebuild_user_effective_permissions (PostgreSqlSchemaInitializer) and the in-memory PermissionEvaluator -- never by main_node, so what a policy grants or denies is unaffected. MainNode IS security-relevant in general (SatelliteAccessRule delegates permissions to it and degrades when it self-references; grants project at COALESCE(main_node, namespace); the SQL read clause matches `n.main_node LIKE prefix || '%'`), but PartitionAccessPolicy registers no SatelliteAccessRule and is not an AccessAssignment. The residual harm is disclosure-shaped: an internal governance node listed as content and returned by mesh-wide search. No StackOverflow risk: MainNode is a string path, there is no object-valued back-reference on MeshNode, GetHashCode excludes Content, and nothing recurses by following MainNode. Already-minted rows: the static seeds are framed into PartitionSourceFingerprint, so changing them changes each partition's fingerprint and the static-repo importer takes the full re-import path instead of the content-skip -- the durable rows in the DB-synced partitions repair themselves on the next deploy, with no migration. Rows written by AccessControlLayoutArea or the raw-SQL writers before this change keep their old value; they are cosmetic per the paragraph above, and the Store-package population is owned in MeshWeaver.Plugins (PluginGate.cs, which assigns the self-reference explicitly). Verified: Mesh.Contract, Graph, Documentation, AI, PluginCatalog, Hosting.PostgreSql, Hosting.Snowflake, Hosting.Monolith.TestBase all 0 errors under -c Release -warnaserror. 2722 tests green across Data (390), Security (376), Graph (1626), PathResolution (155), NodeOperations (76), Documentation (69), Hosting.PostgreSql (26, the container-backed access-control and permission-projection suites that exercise the changed SQL writer) and Hosting.Monolith (4). Both directions proven by reverting the fix: without the mint fixes the sweep fails naming Doc/_Policy, License/_Policy, Role/_Policy and _Setting/_Policy; without the normalization CreatingASiblingSatellite_DerivesItsMainNode fails with Actual: "TestData/_Policy". The two negative controls -- ordinary content stays a main node, an explicit MainNode is preserved -- pass in every configuration. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Core half of #2383 (the Store-package half is a converging-predicate change in MeshWeaver.Plugins'
PluginGate.cs, owned there).Six built-in providers mint a partition's
_Policyasnew MeshNode("_Policy", ns) { … }without settingMainNode. The plain constructor defaults it to the node's own path (MeshNode.cs:96), while line 329 of the same type states the satellite contract. The catalog'sis:mainis literallyMainNode != Path, so a_Policyminted this way is a main node by definition — and "Access Policy" appears under Contents on the Agent, Skill, LanguageModel, Harness and Doc covers. The listing was never wrong; the mint was.Change
MainNode = <partition root>— static seeds, re-registered on boot, so it takes effect on the next deploy with no repair stepMeshNode.Satellite(id, mainNode)for new satellites, with a test pinning both that it points at its main node and that the plain constructor does not (the trap this closes)Not an access defect
MeshWeaver.Securitynever readsMainNode;PartitionAccessPolicyresolves by path.Verified
Mesh.Contract, AI, Documentation: 0 errors under
-warnaserror; 2/2 tests. What's New:Fix.