Move DELETES every satellite under the moved node — comments, threads and access grants are destroyed and the move reports success #3272

Activity

1. rbuergi

   added 2 commits that reference this issue on Sep 4, 2026

   docs: mark the AsynchronousCalls move sketch as a shape, not the cont…

   ...

   3071e3e

   Merge origin/main (#3273's authorship fix) into the satellite fix

   ...

   76f4941
2. rbuergi commented on Sep 4, 2026

   

   rbuergi

   on Sep 4, 2026

   ContributorAuthor

   More actions

   Fixed by #3277, merged into main at 2026-09-04T07:35:59Z as 192a60d84a36cd554eeeb633ea8d9f4f2087caac.

   Closing by hand: core's merge queue does not fire Closes links, so the Closes #3272 in the PR body left this open even though the fix is on main. Verified on origin/main rather than on the PR:

   $ git log --oneline -1 origin/main
   192a60d84 Merge pull request #3277 from Systemorph/fix/3272-move-must-not-orphan-satellites
   $ git show origin/main:src/MeshWeaver.Mesh.Contract/SatelliteTableMapping.cs | grep -c SatelliteContainerOf   → 1
   $ git show origin/main:src/MeshWeaver.Mesh.Contract/CreateNodeRequest.cs     | grep -c RequireComplete        → 4
   $ git show origin/main:test/MeshWeaver.Hosting.Test/MoveCarriesSatellitesTest.cs | grep -c Move_CarriesThe…   → 1
   $ git show origin/main:src/MeshWeaver.Documentation/Data/Architecture/MovingNodes.md | head -3                → the page
   

   Mechanism, as filed and as confirmed

   The issue's diagnosis was right and I confirmed it on both backends rather than assuming it. The copy leg's path:{source} scope:subtree is a primary-table read everywhere:

   • in-memory — StorageAdapterMeshQueryProvider.IsExcludedFromResults strips satellite-path rows from every non-satellite-targeted query, deliberately, to mirror PG's table separation;
   • Postgres — PostgreSqlPartitionedMeshQuery.NeedsFanOut → ResolveTable picks ONE table from the query path's satellite segment or a satellite nodeType filter; path:{main} scope:subtree names neither, so it reads mesh_nodes. That file says it outright: "ListChildPaths only sees mesh_nodes rows, never satellite-table rows."

   So IncludeSatellites = true filtered a set that was already empty, while the delete leg's ListDescendantPaths removed every satellite. Measured before the fix, with the copy leg's own query printed:

   BEFORE storage descendants of TestData/sat-…: [.../_Comment/c1]
   COPY-LEG QUERY 'path:TestData/sat-… scope:subtree' returns: [TestData/sat-…]
   MOVE success=True error=
   AFTER  storage descendants of TestData/satmoved-…: []
   AFTER  storage descendants of TestData/sat-…: []
   

   One correction to the issue's blast radius: Source/Test (→ the code table) were never affected. PostgreSqlStorageAdapter.ResolveQueryTables already unions the CONTENT satellite tables into a primary-table query — added after a Space was GitSync-exported without any of its C# sources. Only the metadata satellites (_Comment, _Thread, _Approval, _Tracking, _Activity, _Access, _Notification, …) were being lost.

   What landed

   (a) The satellites move. The issue's note that the copy leg cannot switch to ListDescendantPaths (RLS) is exactly right, so the two responsibilities are split: storage names which satellite CONTAINERS exist — a path set, never content — and the RLS-filtered query says what may be carried out of them. New SatelliteTableMapping.SatelliteContainerOf(path) cuts a path at the same seam its existing sibling OwnerOfSatellitePath does, and each container is read back as path:{owner}/_Segment scope:subtree limit:all — the one shape every backend resolves to the satellite's own table. One query per container that actually exists (usually none); scope:subtree carries nesting with it; only containers whose owner is a main node the copy is actually carrying are swept.

   (b) …and a move that still cannot carry everything REFUSES. New CopyNodeRequest.RequireComplete, set only by the move, compares the set about to be carried against the same ListDescendantPaths inventory the delete leg is planned from — before anything is created. Nothing written at the target, nothing removed at the source:

   Refused: the copy cannot carry 1 node(s) stored under {source}: {source}/_Comment/c1
   

   That closes the class rather than this instance: an unreadable node, a satellite kind added later, a backend whose query surface diverges again — each becomes a loud refusal instead of a silent deletion. It cannot pass vacuously either: RequireComplete with no IStorageAdapter behind it refuses rather than reporting a completeness it never established.

   🚨 Worth knowing operationally: a move that used to "succeed" destructively can now fail — e.g. a subtree holding something the mover cannot read. That is the intended trade.

   Copy and Delete

   • CopyNodeRequest shared the defect (IncludeSatellites = true on a plain copy carried nothing) and is fixed by the same sweep.
   • DeleteNodeRequest does not — recursive delete already enumerates storage.ListDescendantPaths, so it removes satellites, which is correct for a delete.

   Evidence

   test/MeshWeaver.Hosting.Test/MoveCarriesSatellitesTest.cs — satellites on the moved node, on a descendant, and a nested one; plus the refusal case. Both tests fail with the handler change reverted ("Expected collection to contain an item matching the predicate because the satellite at _Comment/c1 must ride along…", "Expected value to be false because a copy that cannot carry the whole stored subtree must refuse, not report success, but found True") and pass with it. The test prints the copy leg's own query result, so a green run still shows the satellite is absent from it — otherwise a pass could mean "the query started returning satellites" rather than "the sweep works".

   Documented at Doc/Architecture/MovingNodes.

   Interaction with #3263 (which merged first, into the same handler): the two are orthogonal and both are kept — #3263 decides what a re-created node is (every node read from IStorageAdapter, authorship stamps preserved for a move), #3272 decides which nodes there are and whether the set is complete. The satellites this sweep finds go through #3263's Authoritative(...) read too.

   Not fixed here

   A node written between the copy's enumeration and the delete leg's re-enumeration is still deleted uncopied — a pre-existing TOCTOU that the completeness check cannot see, since the delete leg deliberately re-reads storage after the copy. Worth its own issue if it is ever observed.
3. rbuergi

   closed this as completedon Sep 4, 2026