What happened?
apply_patch currently allows parentId updates that can create a cycle in the scene hierarchy.
A minimal case is two Site nodes:
Site_A.parentId = Site_B
Site_B.parentId = Site_A
The store also mirrors these relationships into the corresponding children arrays, producing:
Site_A
└── Site_B
└── Site_A
└── ...
This state remains locally schema-valid:
BaseNode.parentId accepts a string or null.
SiteNode.children accepts string[].
validate_scene validates nodes individually with their Zod schemas and does not appear to perform a global acyclicity check.
The generic reparent guard checks that the new parent exists and can hold children, but I could not find a check that prevents moving a node beneath itself or beneath one of its descendants.
This means a globally invalid cyclic scene graph can pass node-level validation.
On an active scene, apply_patch subsequently calls the live-sync persistence path, so the invalid graph can also proceed toward draft persistence before any global cycle check is performed.
I checked this against current main:
64fc7d8
Steps to reproduce
Starting from a scene containing a normal Site_A:
Create a second Site as a root node:
{
"op": "create",
"node": {
"id": "site_b",
"object": "node",
"type": "site",
"parentId": null,
"children": []
}
}
Reparent the original Site under the new Site:
{
"op": "update",
"id": "<site_a_id>",
"data": {
"parentId": "site_b"
}
}
Reparent site_b under the original Site:
{
"op": "update",
"id": "site_b",
"data": {
"parentId": "<site_a_id>"
}
}
Inspect the resulting graph.
The resulting relationships are conceptually:
Site_A.parentId = Site_B
Site_B.parentId = Site_A
Site_A.children includes Site_B
Site_B.children includes Site_A
Run validate_scene.
Both Site nodes are individually schema-valid, so the hierarchy cycle is not detected by node-local schema validation.
Expected behavior
Any hierarchy mutation should reject a reparent operation that would create a cycle.
For example, changing parentId should fail if the proposed parent is:
the node itself; or
any descendant of the node.
A cyclic graph should also be rejected by scene-level validation before it can be persisted.
Conceptually:
node
└── descendant
node.parentId = descendant
→ reject with invalid_parent / cyclic_hierarchy
Browser & OS
N/A
Screenshots or screen recordings
N/A
Additional context
Suggested fix
The primary fix should be at the hierarchy mutation boundary, rather than only adding guards to individual consumers.
Before accepting any parentId change, verify that the proposed parent is not the node itself and is not reachable from that node through the existing hierarchy.
A small helper could look conceptually like this:
function wouldCreateHierarchyCycle(
nodeId: AnyNodeId,
newParentId: AnyNodeId | null,
nodes: Record<AnyNodeId, AnyNode>,
): boolean {
if (!newParentId) return false
if (newParentId === nodeId) return true
const seen = new Set()
let currentId: AnyNodeId | null = newParentId
while (currentId) {
if (currentId === nodeId) return true
if (seen.has(currentId)) return true
seen.add(currentId)
const current = nodes[currentId]
if (!current) return false
currentId = (current.parentId as AnyNodeId | null) ?? null
}
return false
}
Then the generic reparent path can reject the mutation before updating either side of the relationship:
if (
'parentId' in data &&
data.parentId !== current.parentId
) {
const newParentId =
typeof data.parentId === 'string'
? (data.parentId as AnyNodeId)
: null
if (
newParentId &&
wouldCreateHierarchyCycle(
current.id as AnyNodeId,
newParentId,
scene.nodes,
)
) {
throw new PatchRefusedError(
'invalid_parent',
index,
current.id,
reparenting "${current.id}" under "${newParentId}" would create a hierarchy cycle,
)
}
// existing reparent logic...
}
Ideally the same invariant should live in a shared core hierarchy helper so it is reused by:
apply_patch
updateNode / updateNodes
createNode / createNodes
any future reparent API
rather than being enforced only in the MCP layer.
Scene-level validation
It would also be useful for scene validation to verify global hierarchy properties that per-node Zod validation cannot prove.
At minimum:
- every non-null parentId resolves to an existing node
- child.parentId and parent.children are mutually consistent
- hierarchy traversal is acyclic
- a root node does not also have a structural parent
A simple DFS/parent-chain pass with a visiting/visited set would be enough to detect cycles.
For example:
function validateHierarchyAcyclic(
nodes: Record<AnyNodeId, AnyNode>,
): string[] {
const visiting = new Set()
const visited = new Set()
const issues: string[] = []
const visit = (id: AnyNodeId) => {
if (visiting.has(id)) {
issues.push(hierarchy cycle detected at "${id}")
return
}
if (visited.has(id)) return
visiting.add(id)
const parentId =
(nodes[id]?.parentId as AnyNodeId | null | undefined) ?? null
if (parentId && nodes[parentId]) {
visit(parentId)
}
visiting.delete(id)
visited.add(id)
}
for (const id of Object.keys(nodes) as AnyNodeId[]) {
visit(id)
}
return issues
}
This would complement, rather than replace, mutation-time prevention.
Defensive traversal guards
Even after the mutation path is fixed, some consumers should probably remain defensive because scenes can also enter through:
persisted older files;
imports;
migrations;
plugins;
external tooling.
In particular, resolveLevelId() could use the same pattern already present in findLevelAncestorId():
const seen = new Set()
while (current) {
if (seen.has(current.id as AnyNodeId)) {
return 'default'
}
seen.add(current.id as AnyNodeId)
if (current.type === 'level') {
return current.id
}
current = current.parentId
? nodes[current.parentId]
: undefined
}
Likewise, recursive render/traversal utilities could optionally keep an ancestor set and stop on repeated node IDs rather than relying on the hierarchy always being well-formed.
These defensive checks should be secondary safeguards; the main fix should still prevent invalid hierarchy mutations from committing in the first place.
Regression tests
A regression test for the mutation boundary could use a minimal scene:
Site_A
Site_B
Then attempt:
Site_A.parentId = Site_B
Site_B.parentId = Site_A
Expected:
second mutation → invalid_parent
and assert that the failed operation does not partially modify:
Site_A.parentId
Site_B.parentId
Site_A.children
Site_B.children
rootNodeIds
It would also be useful to cover a deeper descendant case:
A
└── B
└── C
Attempt:
A.parentId = C
Expected:
reject
Finally, a scene-level validation test could construct a cyclic graph directly and verify that the validator reports it even though every individual node still passes its own schema.
Additional context
Relevant paths on current main:
packages/mcp/src/tools/patch-guards.ts
packages/mcp/src/bridge/scene-bridge.ts
packages/core/src/store/actions/node-actions.ts
packages/core/src/hooks/spatial-grid/spatial-grid-sync.ts
packages/core/src/utils/heal-scene-graph.ts
A few details make this potentially more than a malformed-state-only issue:
apply_patch permits parentId changes when the target exists and exposes children, but there is no apparent generic ancestor/descendant cycle check.
validate_scene is node-local and therefore does not establish that the scene hierarchy is globally acyclic.
publishLiveSceneSnapshot() runs after a successful apply_patch; when a scene is bound, the current graph is exported and saved as a draft.
The existing scene healer repairs stale child links and missing reverse parent links, but a symmetric cycle such as:
A.parentId = B
B.children contains A
B.parentId = A
A.children contains B
does not look locally inconsistent and is therefore not naturally removed by those repairs.
There are consumers that assume the parent chain terminates. In particular, the core resolveLevelId() walks parentId using an unbounded while (current) loop. The adjacent findLevelAncestorId() already has a 16-step loop guard explicitly intended to prevent a corrupt parent-chain cycle from hanging a frame loop.
The rendering hierarchy is also recursively driven through children using NodeRenderer, so a reachable child cycle is worth guarding even if mutation-time prevention is added.
I have not claimed an observed browser hang or stack overflow here; the concrete issue is that the mutation path can admit a schema-valid cyclic hierarchy and allow it to proceed into downstream scene consumers/persistence.
The safest repair seems to be:
mutation-time acyclicity enforcement
+
scene-level hierarchy validation
+
defensive traversal guards
with the first item treated as the primary correctness boundary.
What happened?
apply_patch currently allows parentId updates that can create a cycle in the scene hierarchy.
A minimal case is two Site nodes:
Site_A.parentId = Site_B
Site_B.parentId = Site_A
The store also mirrors these relationships into the corresponding children arrays, producing:
Site_A
└── Site_B
└── Site_A
└── ...
This state remains locally schema-valid:
BaseNode.parentId accepts a string or null.
SiteNode.children accepts string[].
validate_scene validates nodes individually with their Zod schemas and does not appear to perform a global acyclicity check.
The generic reparent guard checks that the new parent exists and can hold children, but I could not find a check that prevents moving a node beneath itself or beneath one of its descendants.
This means a globally invalid cyclic scene graph can pass node-level validation.
On an active scene, apply_patch subsequently calls the live-sync persistence path, so the invalid graph can also proceed toward draft persistence before any global cycle check is performed.
I checked this against current main:
64fc7d8
Steps to reproduce
Starting from a scene containing a normal Site_A:
Create a second Site as a root node:
{
"op": "create",
"node": {
"id": "site_b",
"object": "node",
"type": "site",
"parentId": null,
"children": []
}
}
Reparent the original Site under the new Site:
{
"op": "update",
"id": "<site_a_id>",
"data": {
"parentId": "site_b"
}
}
Reparent site_b under the original Site:
{
"op": "update",
"id": "site_b",
"data": {
"parentId": "<site_a_id>"
}
}
Inspect the resulting graph.
The resulting relationships are conceptually:
Site_A.parentId = Site_B
Site_B.parentId = Site_A
Site_A.children includes Site_B
Site_B.children includes Site_A
Run validate_scene.
Both Site nodes are individually schema-valid, so the hierarchy cycle is not detected by node-local schema validation.
Expected behavior
Any hierarchy mutation should reject a reparent operation that would create a cycle.
For example, changing parentId should fail if the proposed parent is:
the node itself; or
any descendant of the node.
A cyclic graph should also be rejected by scene-level validation before it can be persisted.
Conceptually:
node
└── descendant
node.parentId = descendant
→ reject with invalid_parent / cyclic_hierarchy
Browser & OS
N/A
Screenshots or screen recordings
N/A
Additional context
Suggested fix
The primary fix should be at the hierarchy mutation boundary, rather than only adding guards to individual consumers.
Before accepting any parentId change, verify that the proposed parent is not the node itself and is not reachable from that node through the existing hierarchy.
A small helper could look conceptually like this:
function wouldCreateHierarchyCycle(
nodeId: AnyNodeId,
newParentId: AnyNodeId | null,
nodes: Record<AnyNodeId, AnyNode>,
): boolean {
if (!newParentId) return false
if (newParentId === nodeId) return true
const seen = new Set()
let currentId: AnyNodeId | null = newParentId
while (currentId) {
if (currentId === nodeId) return true
if (seen.has(currentId)) return true
}
return false
}
Then the generic reparent path can reject the mutation before updating either side of the relationship:
if (
'parentId' in data &&
data.parentId !== current.parentId
) {
const newParentId =
typeof data.parentId === 'string'
? (data.parentId as AnyNodeId)
: null
if (
newParentId &&
wouldCreateHierarchyCycle(
current.id as AnyNodeId,
newParentId,
scene.nodes,
)
) {
throw new PatchRefusedError(
'invalid_parent',
index,
current.id,
reparenting "${current.id}" under "${newParentId}" would create a hierarchy cycle,)
}
// existing reparent logic...
}
Ideally the same invariant should live in a shared core hierarchy helper so it is reused by:
apply_patch
updateNode / updateNodes
createNode / createNodes
any future reparent API
rather than being enforced only in the MCP layer.
Scene-level validation
It would also be useful for scene validation to verify global hierarchy properties that per-node Zod validation cannot prove.
At minimum:
A simple DFS/parent-chain pass with a visiting/visited set would be enough to detect cycles.
For example:
function validateHierarchyAcyclic(
nodes: Record<AnyNodeId, AnyNode>,
): string[] {
const visiting = new Set()
const visited = new Set()
const issues: string[] = []
const visit = (id: AnyNodeId) => {
if (visiting.has(id)) {
issues.push(
hierarchy cycle detected at "${id}")return
}
}
for (const id of Object.keys(nodes) as AnyNodeId[]) {
visit(id)
}
return issues
}
This would complement, rather than replace, mutation-time prevention.
Defensive traversal guards
Even after the mutation path is fixed, some consumers should probably remain defensive because scenes can also enter through:
persisted older files;
imports;
migrations;
plugins;
external tooling.
In particular, resolveLevelId() could use the same pattern already present in findLevelAncestorId():
const seen = new Set()
while (current) {
if (seen.has(current.id as AnyNodeId)) {
return 'default'
}
seen.add(current.id as AnyNodeId)
if (current.type === 'level') {
return current.id
}
current = current.parentId
? nodes[current.parentId]
: undefined
}
Likewise, recursive render/traversal utilities could optionally keep an ancestor set and stop on repeated node IDs rather than relying on the hierarchy always being well-formed.
These defensive checks should be secondary safeguards; the main fix should still prevent invalid hierarchy mutations from committing in the first place.
Regression tests
A regression test for the mutation boundary could use a minimal scene:
Site_A
Site_B
Then attempt:
Site_A.parentId = Site_B
Site_B.parentId = Site_A
Expected:
second mutation → invalid_parent
and assert that the failed operation does not partially modify:
Site_A.parentId
Site_B.parentId
Site_A.children
Site_B.children
rootNodeIds
It would also be useful to cover a deeper descendant case:
A
└── B
└── C
Attempt:
A.parentId = C
Expected:
reject
Finally, a scene-level validation test could construct a cyclic graph directly and verify that the validator reports it even though every individual node still passes its own schema.
Additional context
Relevant paths on current main:
packages/mcp/src/tools/patch-guards.ts
packages/mcp/src/bridge/scene-bridge.ts
packages/core/src/store/actions/node-actions.ts
packages/core/src/hooks/spatial-grid/spatial-grid-sync.ts
packages/core/src/utils/heal-scene-graph.ts
A few details make this potentially more than a malformed-state-only issue:
apply_patch permits parentId changes when the target exists and exposes children, but there is no apparent generic ancestor/descendant cycle check.
validate_scene is node-local and therefore does not establish that the scene hierarchy is globally acyclic.
publishLiveSceneSnapshot() runs after a successful apply_patch; when a scene is bound, the current graph is exported and saved as a draft.
The existing scene healer repairs stale child links and missing reverse parent links, but a symmetric cycle such as:
A.parentId = B
B.children contains A
B.parentId = A
A.children contains B
does not look locally inconsistent and is therefore not naturally removed by those repairs.
There are consumers that assume the parent chain terminates. In particular, the core resolveLevelId() walks parentId using an unbounded while (current) loop. The adjacent findLevelAncestorId() already has a 16-step loop guard explicitly intended to prevent a corrupt parent-chain cycle from hanging a frame loop.
The rendering hierarchy is also recursively driven through children using NodeRenderer, so a reachable child cycle is worth guarding even if mutation-time prevention is added.
I have not claimed an observed browser hang or stack overflow here; the concrete issue is that the mutation path can admit a schema-valid cyclic hierarchy and allow it to proceed into downstream scene consumers/persistence.
The safest repair seems to be:
mutation-time acyclicity enforcement
+
scene-level hierarchy validation
+
defensive traversal guards
with the first item treated as the primary correctness boundary.