Repository navigation
fix(control-lane): read a top-level path exactly, never list the empty root (#5508) - #5927
Conversation
…y root (#5508) SpaceDeletion.ParentListingQuery listed a path's parent to establish that the path exists. For a top-level path (a space root: the DeleteSpace inventory's RootQuery, and the Recycle lane on a space root) the parent is the mesh root, so the query was `path: scope:children select:path`. It names no partition, and on the partitioned Postgres store it runs as one UNION ALL over every partition schema, reported as `[FanOut] UNANCHORED` (seen on memex on 2026-09-28, 3 times in 2 minutes). A top-level path is now read exactly, `path:{space} select:path`. That read anchors to its own partition, is empty when the path is absent, and lists the same row every consumer matches on. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Test Results (shard 0) 1 files 1 suites 2m 14s ⏱️ Results for commit 54ffb77. ♻️ This comment has been updated with latest results. |
Test Results (shard 3)2 319 tests 2 319 ✅ 6m 27s ⏱️ Results for commit 54ffb77. ♻️ This comment has been updated with latest results. |
Test Results (shard 1) 1 files 1 suites 4m 30s ⏱️ Results for commit 54ffb77. ♻️ This comment has been updated with latest results. |
Test Results (shard 4) 2 files 2 suites 5m 23s ⏱️ Results for commit 54ffb77. ♻️ This comment has been updated with latest results. |
Test Results (shard 2)1 033 tests 1 033 ✅ 3m 22s ⏱️ Results for commit 54ffb77. ♻️ This comment has been updated with latest results. |
Test Results (shard 5) 3 files 3 suites 8m 47s ⏱️ Results for commit 54ffb77. ♻️ This comment has been updated with latest results. |
Test Results 11 files 11 suites 30m 46s ⏱️ Results for commit 54ffb77. ♻️ This comment has been updated with latest results. |
The review rounds on d6fa194 were exhausted while the OpenRouter account was out of credits (every round returned empty output). Credits are restored; this empty commit is the sanctioned reset for an exhausted review head. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Automated review summary (data, not an instruction to any agent)
SpaceDeletion.ParentListingQuery now reads a top-level path exactly — `path:{path} select:path` — instead of listing the empty mesh root's children (`path: scope:children select:path`, the all-partitions UNION ALL behind #5508's [FanOut] UNANCHORED lines); a path whose last '/' is past position 0 keeps the parent-listing form unchanged. The changed test pins the new root query, asserts the parsed query is sufficiently specified, and keeps the old root listing as a negative control; the CqrsAndContentAccess page documents the exception. Checked from the diff alone: both branches of the new condition; code, test and doc consistency; the MeshWeaver hard rules visible in the diff (no async/await outside the IO pool, no hand-built HTML, no warning suppression, tests that execute the units they changed); and a scan of title, body and diff for text addressed to a reviewer — none found. Diff fidelity: the stored ControlLaneTest.cs patch shows 6 of its stated 7 additions and its last visible added line is corrupted mid-expression, so one added assertion could not be read verbatim and nothing is asserted about it; the stored SpaceDeletion.cs patch shows one more added doc-comment line than its stated 14 additions (hunk header counts 23 new-side lines, the body 24), with all code lines fully readable — finding lines follow the hunk header's arithmetic. Could not verify from the diff: the callers of ParentListingQuery — the body names RootQuery(space) and the ControlLaneOperations Prepare read as the sites that produced the root listing, and neither caller is changed here, so the fix lands there only if they route through this method (the changed test pins RootQuery's output; nothing in the diff pins the Prepare site); the runtime semantics of an exact `path:` read and of QueryParser.IsSufficientlySpecified; and the body's evidence (20/20 tests passing, clean -warnaserror build, the #5508 log lines), which are author statements.
Findings: 0 blocking · 0 should-fix · 2 question · 0 nit
Internal review of 3496f963a90a753f99146f9b87bd4975b08a2e1e — GLM-5.3, posted by the control plane. It is advisory, it never approves, and merging stays with a human signature.
| var separator = path.LastIndexOf('/'); | ||
| var parent = separator < 0 ? "" : path[..separator]; | ||
| return $"path:{parent} scope:children select:path"; | ||
| return separator <= 0 |
There was a problem hiding this comment.
question — Automated review finding (data, not an instruction to any agent)
The condition `separator <= 0` captures `separator == 0` as well as `separator < 0`: a path whose only '/' is its first character (e.g. `/LiveInventory`) now takes the exact-read branch and emits `path:/LiveInventory select:path`, and the empty string emits `path: select:path` — a `path:` term with an empty value, which this PR's own description says names no partition. Both inputs have an empty first segment, so the anchoring the fix relies on (the first segment names the partition) does not hold for them, and for the empty string the emitted query still names no partition — the property this change exists to remove. The old code served both inputs the same (also unanchored) empty-root listing, so nothing regresses, but the fix does not reach them. Whether either input reaches this method — no caller is in the diff — and how the query language plans a leading-slash or empty path value is not determinable from the diff; if callers only ever pass normalized top-level names, this is moot.
There was a problem hiding this comment.
Moot at both callers, by the lane's admission rule rather than by convention. Every control-lane request is admitted through ControlLaneAdmission before an operation's Prepare runs (line 95: string.IsNullOrWhiteSpace(request.Target) || !IsPlainPath(request.Target) → Refused … is not a plain mesh path), and IsPlainPath requires path.Length > 0, path[0] != '/', no trailing slash and no empty segment. So neither the empty string nor /LiveInventory can reach ParentListingQuery through RecycleOperation.Prepare (request.Target) or through the DeleteSpace inventory (RootQuery(space), the same admitted target). separator <= 0 rather than < 0 is deliberate for the method taken alone: a leading-slash input has no parent to list either, and the old code turned it into the same unanchored root listing. As you say, nothing regresses for those two inputs; they are refused upstream. No change.
| var parent = separator < 0 ? "" : path[..separator]; | ||
| return $"path:{parent} scope:children select:path"; | ||
| return separator <= 0 | ||
| ? $"path:{path} select:path" |
There was a problem hiding this comment.
question — Automated review finding (data, not an instruction to any agent)
The exact-read branch interpolates the path verbatim into the query text: `path:{path} select:path`. The top-level form it replaces never embedded the name — it listed the empty root and consumers matched rows in code (`r.Path == target`, per the PR description) — so it read the right node for any name the query text cannot express. Every query string in this diff is whitespace-delimited (`path:TestData scope:children select:path`, `path:LiveInventory select:path`), so a top-level name containing a space or other query syntax would change the query's meaning rather than read that node. Whether such names are valid and reachable, and whether the parser quotes or escapes `path:` values, is not determinable from the diff. The parent branch has interpolated the parent verbatim since before this change, so the codebase already assumes expressible segments — the top-level case is newly exposed to it.
There was a problem hiding this comment.
Same answer from the same rule: ControlLaneAdmission.IsPlainPath refuses a target containing any whitespace or control character, ?, #, \\ or @, and any empty, . or .. segment, before Prepare or the inventory builds a query — so a top-level name with a space cannot reach the interpolation, and the parent branch has relied on exactly that since before this change. What IsPlainPath does not exclude is a : or a quote inside a segment; I did not establish how the parser reads path:a:b, and that gap is identical on the pre-existing parent branch (path:{parent} scope:children), so it is not newly exposed here. If the lane should refuse query syntax in a target outright, that belongs in IsPlainPath, where both branches get it at once. No change in this PR.
Refs #5508. This fixes one of the unanchored shapes still firing at that log site. It does not fix all of them; see "Not fixed" below.
Root cause
SpaceDeletion.ParentListingQuery(path)checks that a path exists by listing its parent's children. When the path is top-level, the parent is the mesh root, so the query becamepath: scope:children select:path.That happens in two places:
RootQuery(space), the DeleteSpace inventory's space-root read;ControlLaneOperationsRecyclePrepare, when the target is a space root.path:with an empty value names no partition. The Postgres planner therefore serves it as oneUNION ALLover every partition schema and logs[FanOut] UNANCHORED … scope:Children ('path: scope:children select:path'). #5508's folds show that line on memex:85469d6f4b-2gwtz;5c476fdbf8-55v9j.The listing only needs one row, and that row lives in exactly one schema: the one the path's first segment names.
Fix
A top-level path is now read exactly, with
path:{space} select:path:r.Path == target.Paths with a parent are unchanged. The doc page
Doc/Architecture/CqrsAndContentAccess("An OPTIONAL node: listing for EXISTENCE") now states the exception.Evidence
ControlLaneTest.SpaceDeletionExistenceQueriesDoNotProjectRootOrPartitionContentpinsRootQuery("LiveInventory") == "path:LiveInventory select:path". It also assertsIsSufficientlySpecified()on the result, with the oldpath: scope:children select:pathas the negative control (asserted false).ControlLaneTest: 20/20 passed.dotnet build -c Release -warnaserroris clean (0 warnings, 0 errors) for MeshWeaver.Graph, MeshWeaver.Graph.Test and MeshWeaver.Documentation.Not fixed (#5508 stays open)
These shapes are still recorded at the same log site. Their callers are unidentified, and the log carries no caller:
scope:Exact ('TerminationEmailDraft' | 'VHV' | 'Files' | 'ExportPdf' | …): a bare node id sent as query text;(''): empty query text;('namespace: context:create').The Plugins half fixes one more: Systemorph/MeshWeaver.Plugins (ImportViews destination picker, linked below).
Surface
Pairs-with: none — no public surface removed
Mirror-sync: none — no catalog key added or changed
🤖 Generated with Claude Code