Fix composite node duplication placement (#542)

Route composite duplication through the fresh-subtree placement lifecycle, preserve descendant geometry and IDs, clear stale selection, and guarantee failed drafts restore history tracking.

packages/editor/src/components/editor-2d/floorplan-registry-action-menu.tsx

@@ -24,6 +24,7 @@ import { getFloorplanNodeExtension } from '../../lib/floorplan/floorplan-extensi
 import {
   createFreshPlacementSubtree,
   duplicatesAsFreshSubtree,
+  prepareFreshPlacementRootDuplicate,
 } from '../../lib/fresh-planar-placement'
 import { curveReshapeScope } from '../../lib/interaction/scope'
 import { playBlockedQuickActionFeedback } from '../../lib/quick-action-feedback'
@@ -116,8 +117,8 @@ function collectQuickActionNodes(
  *  - Add hole (slab + ceiling only): inserts a small default-square
  *    hole at the polygon centroid via `updateNode`. Mirrors the legacy
  *    `handleAddHole` in `floating-action-menu.tsx`.
- *  - Duplicate: deep-clones the node, marks it new, sets it as the
- *    movingNode (placement cursor) — same UX pattern as 3D duplicate.
+ *  - Duplicate: creates a fresh subtree when the kind opts in, otherwise a
+ *    root-only copy, then hands that real draft to the placement cursor.
  *  - Delete: calls `deleteNode(id)`. Cascade is handled by the registry's
  *    `relations.cascadeDelete` if declared on the def.
  *
@@ -320,35 +321,31 @@ export function FloorplanRegistryActionMenu() {
     if (!node.parentId) return
     sfxEmitter.emit('sfx:item-pick')
     useScene.temporal.getState().pause()
+    let draftId: AnyNodeId | null = null
+    try {
       if (duplicatesAsFreshSubtree(node as AnyNode)) {
-      const draftId = createFreshPlacementSubtree(node.id as AnyNodeId)
+        draftId = createFreshPlacementSubtree(node.id as AnyNodeId)
         const draft = draftId ? useScene.getState().nodes[draftId] : null
-      if (draft) {
+        if (!draft) return
         setMovingNode(draft as never)
-        setMovingNodeOrigin('2d')
-        useScene.temporal.getState().resume()
-        return
-      }
-      useScene.temporal.getState().resume()
-      return
-    }
-    const cloned = structuredClone(node) as AnyNode & { id?: AnyNodeId }
-    delete (cloned as { id?: AnyNodeId }).id
-    const prevMeta =
-      cloned.metadata && typeof cloned.metadata === 'object' && !Array.isArray(cloned.metadata)
-        ? (cloned.metadata as Record<string, unknown>)
-        : {}
-    // Mark fresh + hand to the placement cursor so the copy follows the
-    // pointer and only lands on the next click — same gesture for every
-    // kind. Polyline runs (duct / pipe / lineset) ride the same path:
-    // `FloorplanRegistryMoveOverlay` translates their whole `path`, so they
-    // no longer need the old "offset + drop already-placed" special case.
-    cloned.metadata = { ...prevMeta, isNew: true }
+      } else {
+        const cloned = prepareFreshPlacementRootDuplicate(node as AnyNode)
         const parsed = def.schema.parse(cloned) as AnyNode
+        draftId = parsed.id as AnyNodeId
         useScene.getState().createNode(parsed, node.parentId as AnyNodeId)
         setMovingNode(parsed as never)
+      }
+      setMovingNodeOrigin('2d')
+      useViewer.getState().setSelection({ selectedIds: [] })
+    } catch (error) {
+      if (draftId && useScene.getState().nodes[draftId]) {
+        useScene.getState().deleteNode(draftId)
+      }
+      console.error('Failed to duplicate node', error)
+    } finally {
       useScene.temporal.getState().resume()
     }
+  }
 
   const handleDelete = () => {
     sfxEmitter.emit('sfx:item-delete')

packages/editor/src/components/editor/floating-action-menu.tsx

@@ -27,7 +27,6 @@ import {
   runAsSingleSceneHistoryStep,
   type SlabNode,
   SpawnNode,
-  StairNode,
   StairSegmentNode,
   sceneRegistry,
   summarizeSystemFor,
@@ -47,6 +46,7 @@ import { resolveMoveActionNode } from '../../lib/direct-manipulation'
 import {
   createFreshPlacementSubtree,
   duplicatesAsFreshSubtree,
+  prepareFreshPlacementRootDuplicate,
 } from '../../lib/fresh-planar-placement'
 import { resolveOverlayPolicy } from '../../lib/interaction/overlay-policy'
 import { curveReshapeScope, holeEditScope } from '../../lib/interaction/scope'
@@ -54,7 +54,6 @@ import { playBlockedQuickActionFeedback } from '../../lib/quick-action-feedback'
 import { collectQuickActionNodeScope } from '../../lib/quick-action-nodes'
 import { duplicateRoofSubtree } from '../../lib/roof-duplication'
 import { emitDeleteSFX, sfxEmitter } from '../../lib/sfx-bus'
-import { duplicateStairSubtree } from '../../lib/stair-duplication'
 import { cn } from '../../lib/utils'
 import useEditor from '../../store/use-editor'
 import useInteractionScope, {
@@ -530,20 +529,26 @@ export function FloatingActionMenu() {
       useScene.temporal.getState().pause()
 
       if (duplicatesAsFreshSubtree(node as AnyNode)) {
-        const draftId = createFreshPlacementSubtree(node.id as AnyNodeId)
+        let draftId: AnyNodeId | null = null
+        try {
+          draftId = createFreshPlacementSubtree(node.id as AnyNodeId)
           const draft = draftId ? useScene.getState().nodes[draftId] : null
           if (draft) {
             setMovingNode(draft as any)
             setSelection({ selectedIds: [] })
             return
           }
+        } catch (error) {
+          if (draftId && useScene.getState().nodes[draftId]) {
+            useScene.getState().deleteNode(draftId)
+          }
+          console.error('Failed to duplicate node subtree', error)
+        }
         useScene.temporal.getState().resume()
         return
       }
 
-      let duplicateInfo = structuredClone(node) as any
-      delete duplicateInfo.id
-      duplicateInfo.metadata = { ...duplicateInfo.metadata, isNew: true }
+      const duplicateInfo = prepareFreshPlacementRootDuplicate(node as AnyNode) as any
 
       let duplicate: AnyNode | null = null
       try {
@@ -566,11 +571,6 @@ export function FloatingActionMenu() {
         } else if (node.type === 'roof-segment') {
           duplicateInfo.id = generateId('rseg')
           duplicate = RoofSegmentNode.parse(duplicateInfo)
-        } else if (node.type === 'stair') {
-          duplicateInfo.children = []
-          duplicateInfo.metadata = { ...duplicateInfo.metadata }
-          delete duplicateInfo.metadata?.isNew
-          duplicate = StairNode.parse(duplicateInfo)
         } else if (node.type === 'stair-segment') {
           duplicate = StairSegmentNode.parse(duplicateInfo)
         } else if (node.type === 'spawn') {
@@ -608,11 +608,7 @@ export function FloatingActionMenu() {
           useScene.getState().createNode(duplicate, duplicate.parentId as AnyNodeId)
         } else if (duplicate.type === 'fence') {
           useScene.getState().createNode(duplicate, duplicate.parentId as AnyNodeId)
-        } else if (
-          duplicate.type === 'roof-segment' ||
-          duplicate.type === 'stair' ||
-          duplicate.type === 'stair-segment'
-        ) {
+        } else if (duplicate.type === 'roof-segment' || duplicate.type === 'stair-segment') {
           // Add small offset to make it visible
           if ('position' in duplicate) {
             duplicate.position = [
@@ -621,13 +617,7 @@ export function FloatingActionMenu() {
               duplicate.position[2] + 1,
             ]
           }
-          if (node.type === 'stair' && duplicate.type === 'stair') {
-            duplicateStairSubtree(node.id as AnyNodeId, { mode: 'move' })
-          } else {
           useScene.getState().createNode(duplicate, duplicate.parentId as AnyNodeId)
-          }
-
-          // Duplicate children for stair nodes
         } else if (
           duplicate.type === 'item' ||
           duplicate.type === 'chimney' ||
@@ -696,13 +686,9 @@ export function FloatingActionMenu() {
           nodeRegistry.has(duplicate.type)
         ) {
           setMovingNode(duplicate as any)
-        } else if (duplicate.type === 'stair') {
-          setSelection({ selectedIds: [duplicate.id as AnyNodeId] })
         }
-        if (duplicate.type !== 'stair') {
         setSelection({ selectedIds: [] })
       }
-      }
     },
     [node, setMovingNode, setSelection],
   )

packages/editor/src/lib/fresh-planar-placement.test.ts

@@ -13,7 +13,12 @@ import {
   useScene,
 } from '@pascal-app/core'
 import { z } from 'zod'
-import { commitFreshPlacementSubtree, createFreshPlacementSubtree } from './fresh-planar-placement'
+import {
+  commitFreshPlacementSubtree,
+  createFreshPlacementSubtree,
+  duplicatesAsFreshSubtree,
+  prepareFreshPlacementRootDuplicate,
+} from './fresh-planar-placement'
 
 type RafFn = (cb: (time: number) => void) => number
 ;(globalThis as { requestAnimationFrame?: RafFn }).requestAnimationFrame ??= ((
@@ -212,6 +217,33 @@ describe('commitFreshPlacementSubtree', () => {
     expect((useScene.getState().nodes[LEVEL_ID] as { children: AnyNodeId[] }).children).toEqual([])
   })
 
+  test('uses the subtree contract for childless variants and never aliases root-only children', () => {
+    registerCabinetClonePrepTestKind()
+    const childlessCabinet = {
+      ...shelf(),
+      type: 'cabinet',
+      children: [],
+      metadata: { isTransient: true, label: 'source' },
+    } as AnyNode
+    expect(duplicatesAsFreshSubtree(childlessCabinet)).toBe(true)
+
+    const source = {
+      ...shelf(),
+      children: ['item_original' as AnyNodeId],
+      metadata: { isTransient: true, label: 'source' },
+    } as AnyNode
+    const duplicate = prepareFreshPlacementRootDuplicate(source) as AnyNode & {
+      children: AnyNodeId[]
+      id?: AnyNodeId
+      metadata?: Record<string, unknown>
+    }
+
+    expect(duplicate.id).toBeUndefined()
+    expect(duplicate.children).toEqual([])
+    expect(duplicate.metadata).toEqual({ isNew: true, label: 'source' })
+    expect((source as AnyNode & { children: AnyNodeId[] }).children).toEqual(['item_original'])
+  })
+
   test('commits a duplicated cabinet draft without deleting the original modules', () => {
     seedCabinetRun()
     useScene.temporal.getState().clear()

packages/editor/src/lib/fresh-planar-placement.ts

@@ -27,10 +27,27 @@ function duplicableConfigFor(node: AnyNode): DuplicableConfig | null {
 }
 
 export function duplicatesAsFreshSubtree(node: AnyNode): boolean {
-  const children = (node as { children?: unknown }).children
-  return (
-    duplicableConfigFor(node)?.subtree === true && Array.isArray(children) && children.length > 0
-  )
+  return duplicableConfigFor(node)?.subtree === true
+}
+
+/**
+ * Prepares a non-subtree duplicate without retaining ownership of the
+ * original node's children. Subtree-capable kinds take the path above and
+ * receive fresh descendant IDs; every other kind duplicates only its root.
+ */
+export function prepareFreshPlacementRootDuplicate(node: AnyNode): AnyNode {
+  const duplicate = structuredClone(node) as unknown as Record<string, unknown> & {
+    id?: AnyNodeId
+    children?: unknown
+    metadata?: unknown
+  }
+  delete duplicate.id
+  if (Array.isArray(duplicate.children)) duplicate.children = []
+  duplicate.metadata = {
+    ...getPlacementMetadataRecord(stripPlacementMetadataFlags(duplicate.metadata)),
+    isNew: true,
+  }
+  return duplicate as unknown as AnyNode
 }
 
 /**

packages/editor/src/lib/stair-duplication.test.ts

@@ -0,0 +1,103 @@
+import { afterEach, beforeEach, describe, expect, test } from 'bun:test'
+import {
+  type AnyNode,
+  type AnyNodeId,
+  LevelNode,
+  StairNode,
+  StairSegmentNode,
+  useScene,
+} from '@pascal-app/core'
+import { useViewer } from '@pascal-app/viewer'
+import useInteractionScope, { getMovingNode } from '../store/use-interaction-scope'
+import { duplicateStairSubtree } from './stair-duplication'
+
+const LEVEL_ID = 'level_stair-duplicate' as AnyNodeId
+const STAIR_ID = 'stair_original' as AnyNodeId
+const SEGMENT_ID = 'sseg_original' as AnyNodeId
+
+function seedStraightStair() {
+  const level = LevelNode.parse({
+    id: LEVEL_ID,
+    type: 'level',
+    children: [STAIR_ID],
+  })
+  const stair = StairNode.parse({
+    id: STAIR_ID,
+    type: 'stair',
+    parentId: LEVEL_ID,
+    children: [SEGMENT_ID],
+    position: [2, 0, 3],
+  })
+  const segment = StairSegmentNode.parse({
+    id: SEGMENT_ID,
+    type: 'stair-segment',
+    parentId: STAIR_ID,
+  })
+  useScene.setState({
+    nodes: {
+      [LEVEL_ID]: level as AnyNode,
+      [STAIR_ID]: stair as AnyNode,
+      [SEGMENT_ID]: segment as AnyNode,
+    },
+    rootNodeIds: [LEVEL_ID],
+    collections: {},
+    dirtyNodes: new Set(),
+  } as never)
+}
+
+describe('duplicateStairSubtree', () => {
+  beforeEach(() => {
+    useInteractionScope.getState().end()
+    useViewer.getState().setSelection({ selectedIds: [STAIR_ID] })
+    useScene.temporal.getState().clear()
+    useScene.temporal.getState().resume()
+    seedStraightStair()
+  })
+
+  afterEach(() => {
+    useInteractionScope.getState().end()
+    useScene.temporal.getState().resume()
+    useViewer.getState().setSelection({ selectedIds: [] })
+  })
+
+  test('moves the exact fresh scene subtree and clears the original selection', () => {
+    const result = duplicateStairSubtree(STAIR_ID, { mode: 'move' })
+    const nodes = useScene.getState().nodes
+    const draft = nodes[result.stair.id as AnyNodeId]
+
+    expect(result.stair.id).not.toBe(STAIR_ID)
+    expect(draft).toBe(result.stair)
+    expect(getMovingNode()?.id).toBe(result.stair.id)
+    expect(useViewer.getState().selection.selectedIds).toEqual([])
+    expect((result.stair.metadata as Record<string, unknown>)?.isNew).toBe(true)
+    expect(result.stair.position).toEqual([3, 0, 4])
+    expect(result.segmentIds).toHaveLength(1)
+    expect(result.segmentIds[0]).not.toBe(SEGMENT_ID)
+    expect(nodes[result.segmentIds[0] as AnyNodeId]?.parentId).toBe(result.stair.id)
+    expect(nodes[STAIR_ID]).toBeDefined()
+    expect(nodes[SEGMENT_ID]).toBeDefined()
+    expect(useScene.temporal.getState().isTracking).toBe(false)
+  })
+
+  test('keeps childless curved stairs on the same real draft path', () => {
+    const curved = StairNode.parse({
+      ...(useScene.getState().nodes[STAIR_ID] as AnyNode),
+      children: [],
+      stairType: 'curved',
+    })
+    useScene.setState((state) => ({
+      nodes: {
+        ...state.nodes,
+        [LEVEL_ID]: { ...state.nodes[LEVEL_ID], children: [STAIR_ID] } as AnyNode,
+        [STAIR_ID]: curved as AnyNode,
+      },
+    }))
+
+    const result = duplicateStairSubtree(STAIR_ID, { mode: 'move', offset: [0, 0, 0] })
+
+    expect(result.segmentIds).toEqual([])
+    expect(useScene.getState().nodes[result.stair.id as AnyNodeId]).toBe(result.stair)
+    expect(getMovingNode()?.id).toBe(result.stair.id)
+    expect((result.stair.metadata as Record<string, unknown>)?.isNew).toBe(true)
+  })
+})

packages/editor/src/lib/stair-duplication.ts

@@ -1,15 +1,13 @@
 import {
+  type AnyNode,
   type AnyNodeId,
-  generateId,
   type StairNode,
-  StairNode as StairNodeSchema,
   type StairSegmentNode,
-  StairSegmentNode as StairSegmentNodeSchema,
-  sceneRegistry,
   useScene,
 } from '@pascal-app/core'
 import { useViewer } from '@pascal-app/viewer'
 import useEditor from '../store/use-editor'
+import { commitFreshPlacementSubtree, createFreshPlacementSubtree } from './fresh-planar-placement'
 
 type DuplicateStairOptions = {
   mode?: 'select' | 'move'
@@ -22,47 +20,19 @@ type DuplicateStairResult = {
   segmentIds: StairSegmentNode['id'][]
 }
 
-const MOVE_REGISTRY_RETRY_LIMIT = 12
-
-function stripDuplicateFlags(metadata: unknown) {
-  if (typeof metadata !== 'object' || metadata === null || Array.isArray(metadata)) {
-    return metadata
-  }
-
-  const nextMeta = { ...(metadata as Record<string, unknown>) }
-  delete nextMeta.isNew
-  delete nextMeta.isTransient
-  return nextMeta
-}
-
-function moveStairWhenRegistered(stairId: StairNode['id'], attempt = 0) {
-  const latestStair = useScene.getState().nodes[stairId as AnyNodeId]
-  if (latestStair?.type !== 'stair') {
-    return
-  }
-
-  if (sceneRegistry.nodes.has(stairId)) {
-    useEditor.getState().setMovingNode(latestStair)
-    useViewer.getState().setSelection({ selectedIds: [] })
-    return
-  }
-
-  if (attempt >= MOVE_REGISTRY_RETRY_LIMIT) {
-    console.warn(`Duplicated stair "${stairId}" did not register before move mode started`)
-    return
-  }
-
-  requestAnimationFrame(() => moveStairWhenRegistered(stairId, attempt + 1))
-}
-
+/**
+ * Duplicates a stair through the shared fresh-subtree placement path.
+ *
+ * The draft passed to the move tool is the exact node stored in the scene,
+ * so its geometry, descendants, selection identity, and eventual commit all
+ * use one fresh ID graph.
+ */
 export function duplicateStairSubtree(
   sourceStairId: AnyNodeId,
   options: DuplicateStairOptions = {},
 ): DuplicateStairResult {
   const { mode = 'move', offset = [1, 0, 1], parentId: explicitParentId } = options
-
-  const scene = useScene.getState()
-  const sourceStair = scene.nodes[sourceStairId]
+  const sourceStair = useScene.getState().nodes[sourceStairId]
 
   if (sourceStair?.type !== 'stair') {
     throw new Error(`Node "${sourceStairId}" is not a stair`)
@@ -73,54 +43,56 @@ export function duplicateStairSubtree(
     throw new Error(`Stair "${sourceStairId}" is missing a parent level`)
   }
 
-  const stairClone = StairNodeSchema.parse({
-    ...structuredClone(sourceStair),
-    id: generateId('stair'),
+  const temporal = useScene.temporal.getState()
+  const wasTracking = (temporal as { isTracking?: boolean }).isTracking !== false
+  if (wasTracking) temporal.pause()
+
+  let draftId: AnyNodeId | null = null
+  try {
+    draftId = createFreshPlacementSubtree(sourceStairId, {
       parentId,
-    children: [],
       position: [
         sourceStair.position[0] + offset[0],
         sourceStair.position[1] + offset[1],
         sourceStair.position[2] + offset[2],
-    ] as StairNode['position'],
-    metadata: stripDuplicateFlags(sourceStair.metadata),
-  })
-
-  const segmentClones: StairSegmentNode[] = []
-  for (const childId of sourceStair.children ?? []) {
-    const childNode = scene.nodes[childId as AnyNodeId]
-    if (childNode?.type !== 'stair-segment') {
-      continue
-    }
-
-    const childClone = StairSegmentNodeSchema.parse({
-      ...structuredClone(childNode),
-      id: generateId('sseg'),
-      parentId: stairClone.id,
-      metadata: stripDuplicateFlags(childNode.metadata),
-    })
-    segmentClones.push(childClone)
-  }
-
-  scene.createNodes([
-    { node: stairClone, parentId },
-    ...segmentClones.map((segment) => ({ node: segment, parentId: stairClone.id as AnyNodeId })),
-  ])
-
-  const createdStair = useScene.getState().nodes[stairClone.id as AnyNodeId]
-  if (createdStair?.type !== 'stair') {
-    throw new Error(`Duplicated stair "${stairClone.id}" was not created`)
+      ],
+    } as Partial<AnyNode>)
+    const draft = draftId ? useScene.getState().nodes[draftId] : null
+    if (draft?.type !== 'stair') {
+      throw new Error(`Duplicated stair "${sourceStairId}" was not created`)
     }
 
     if (mode === 'select') {
-    useViewer.getState().setSelection({ selectedIds: [createdStair.id] })
-  } else {
-    useViewer.getState().setSelection({ selectedIds: [createdStair.id] })
-    requestAnimationFrame(() => moveStairWhenRegistered(createdStair.id))
+      const committedId = commitFreshPlacementSubtree(draft.id as AnyNodeId, {})
+      const committed = committedId ? useScene.getState().nodes[committedId] : null
+      if (committed?.type !== 'stair') {
+        throw new Error(`Duplicated stair "${sourceStairId}" could not be committed`)
+      }
+      if (wasTracking) temporal.resume()
+      useViewer.getState().setSelection({ selectedIds: [committed.id] })
+      return {
+        stair: committed,
+        segmentIds: stairSegmentIds(committed),
+      }
     }
 
+    useViewer.getState().setSelection({ selectedIds: [] })
+    useEditor.getState().setMovingNode(draft)
     return {
-    stair: createdStair,
-    segmentIds: segmentClones.map((segment) => segment.id),
+      stair: draft,
+      segmentIds: stairSegmentIds(draft),
+    }
+  } catch (error) {
+    if (draftId && useScene.getState().nodes[draftId]) {
+      useScene.getState().deleteNode(draftId)
+    }
+    if (wasTracking) temporal.resume()
+    throw error
   }
 }
+
+function stairSegmentIds(stair: StairNode): StairSegmentNode['id'][] {
+  return (stair.children ?? []).filter((childId): childId is StairSegmentNode['id'] => {
+    return useScene.getState().nodes[childId as AnyNodeId]?.type === 'stair-segment'
+  })
+}

packages/nodes/src/stair/definition.ts

@@ -468,7 +468,7 @@ export const stairDefinition: NodeDefinition<typeof StairNode> = {
       const aabb = stairFootprintAABB(node as StairNodeType, nodes)
       return aabb ? { shape: 'aabb', ...aabb } : null
     },
-    duplicable: true,
+    duplicable: { subtree: true },
     deletable: true,
     floorPlaced: {
       footprints: (node, ctx) =>