feat(nodeShell): assert node registration-identity invariants (#14443)

**STACKED — merging lands on `matt/be-5050-node-shell-state` (#14257,
which itself targets `feature/ecs-migration`), NOT `main`.**

## ELI-5

When a node joins or leaves a graph, the app files its "shell state"
into a store so the renderer can draw it. Two ways that filing can go
wrong today fail silently: filing a node that is already filed under a
*different* workflow strands the old entry, and un-filing a state the
store never held leaves a ghost the renderer keeps drawing. Both now
trip an assertion (throws in DEV, reports via the assert reporter in
prod) instead of drifting quietly. No behavior changes when the
invariants hold.

## What changed

`src/core/graph/nodeShell/nodeShellState.ts`

- `registerNodeState` asserts `node._graphId` is unset or already equals
`graph.rootGraph.id` before overwriting it. Membership is by state
identity, so a node re-registered under a second root graph leaves its
first bucket entry behind with nothing to remove it.
- `unregisterNodeState` captures `deleteNode`'s boolean instead of
discarding it and asserts it. `false` means the bucket did not contain
`node._state` — identity drift, e.g. `_state` reassigned after
registration.

Both messages name the node id. `_graphId` is cleared before the
assertion so a failing teardown still leaves the node detached rather
than half-registered.

## Deliberately not shipped: the duplicate-`NodeState` assertion

The third invariant asked for — asserting the store never holds two
distinct `NodeState` objects for one `(graphId, id)` — was implemented
(an auxiliary forward `Map<string, NodeState>` plus a `Map<NodeState,
string>` reverse map per bucket, so a renumbered node still de-indexes
by identity) and then **reverted**, because it is not a regression net
on this base: it fires on legitimate existing behavior.

Evidence, all from the suite on this branch:

- `LGraph.serialise.test.ts > can (de)serialise node / group titles` —
`new LGraph(data)` (`LGraph.ts:476` → `configure:2636` → `add:1092`)
keeps the *persisted* graph id, so a round-tripped copy shares a bucket
with the graph it was serialised from and collides on every node id.
- `LGraph.test.ts > Link serialization goldens` (4 tests) and
`LGraph.test.ts > deduplicateSubgraphNodeIds (via configure) > warns
when configuring a host with legacy proxyWidgets…` fail the same way.

That is 6 tests, green on this base and green again with the assertion
removed — so this is the assertion's premise being wrong, not a latent
bug those tests were hiding. `subgraphDeduplication.ts` guarantees
id-uniqueness *within* one root graph; it says nothing about two live
`LGraph` instances that carry the same id. Making duplicate ids
assertable means giving each `LGraph` instance its own bucket identity
rather than keying buckets by graph id — a separate change with real
blast radius (`getGraphNodesFor` callers, `clearGraph`, the
`canvasStore.rootGraphId` readers), not a rider on this one.
`docs/architecture/node-data-store.md` records both the two shipped
invariants and this gap.

## Tests

`nodeShellState.test.ts` gains the two cases; both are red without the
production change (verified by stashing it). The cross-root case needs
an explicit `second.id = createUuidv4()` — two fresh `LGraph`s both sit
at `zeroUuid` until `configure`, so they share a bucket and the
invariant genuinely holds.

## Verification

Targeted: `nodeShellState.test.ts`, `nodeDataStore.test.ts`,
`useNodeReplacement.test.ts`, `LGraph.test.ts`,
`LGraph.serialise.test.ts` — 82 tests pass, 0 fail. `oxfmt --check`
clean.

Full `vitest run` was executed and is the reason the third assertion was
reverted (it caught the 6 failures above). That run is **not** a
clean-suite claim: this worktree borrows the parent clone's
`node_modules`, which predates the `minisearch` dependency this branch
adds, so 222 suites failed to resolve imports for reasons unrelated to
the diff. Of the tests that did run, the only failures attributable to
the diff were the 6 named above; the rest (`assetService`,
`useAssetGridSelection`, `onboardingCloudRoutes`, …) are the same
load-dependent set #14257 reported. A reviewer should treat repo-wide CI
as the authority here, not this local run. Repo-wide `pnpm typecheck`
was likewise not run locally — the diff is 11 lines of source plus
tests, and both changed files typecheck under vitest's transform.

Author: mattmillerai

docs/architecture/node-data-store.md

@@ -132,6 +132,24 @@ Follows the shipped trio convention (`LLink` / `Reroute`):
 - Chokepoints: `LGraph.add` / `LGraph.remove` (the canonical sites),
   `unregisterAllNodeStates(graph)` on graph `clear()`, identity-checked
   delete (`toRaw` compare) so only the registered state vacates its key.
+- Two ways that lifecycle can silently drift are asserted rather than left to
+  the renderer to expose: re-registering an already-registered node under a
+  different root graph (its old bucket entry would strand), and unregistering a
+  state the bucket does not hold (`deleteNode` returning `false` — a ghost the
+  renderer keeps drawing). Re-registering the _same_ state object under the
+  _same_ root stays legal: `reactive()` returns a cached proxy, so
+  unregister→register sequences (`useNodeReplacement`) are idempotent.
+- `assert` throws in DEV and reports (Sentry) elsewhere, so both paths repair
+  the store before reporting rather than relying on the throw to stop them:
+  `registerNodeState` deletes the stale entry from the previous root's bucket
+  and drops `node._graphScope` with it; `unregisterNodeState` clears
+  `_graphScope` regardless of the outcome. A production build is left
+  consistent, and a DEV throw cannot strand the node it names.
+- Not asserted: a second `NodeState` object for a `(graphId, id)` the bucket
+  already holds. Deserialising a graph keeps its persisted id, so two live
+  `LGraph` instances round-tripped from one workflow share a bucket and collide
+  on every node id by design. Catching duplicate-id regressions needs bucket
+  identity to be per-instance rather than per-id — a separate change.
 - The lifecycle coordination itself is app-owned and lives in
   `src/core/graph/nodeShell/`: `nodeShellState.ts` (`createNodeShellState`,
   `setTrackedNodeState`, `registerNodeState`, `unregisterNodeState`,

src/core/graph/nodeShell/nodeShellState.test.ts

@@ -1,18 +1,18 @@
 import { createTestingPinia } from '@pinia/testing'
 import { setActivePinia } from 'pinia'
-import { beforeEach, describe, expect, it } from 'vitest'
+import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
 import { toRaw } from 'vue'
 
-import { LGraphNode } from '@/lib/litegraph/src/litegraph'
+import { LGraph, LGraphNode } from '@/lib/litegraph/src/litegraph'
 import type { Subgraph } from '@/lib/litegraph/src/litegraph'
 import { createTestSubgraph } from '@/lib/litegraph/src/subgraph/__fixtures__/subgraphHelpers'
 import { useNodeDataStore } from '@/stores/nodeDataStore'
 import { graphScopeOf } from '@/types/graphScopeId'
 import { UNASSIGNED_NODE_ID } from '@/types/nodeId'
 import type { NodeState } from '@/types/nodeState'
-import { zeroUuid } from '@/utils/uuid'
+import { createUuidv4, zeroUuid } from '@/utils/uuid'
 
-import { createNodeShellState } from './nodeShellState'
+import { createNodeShellState, unregisterNodeState } from './nodeShellState'
 
 describe('node shell state', () => {
   beforeEach(() => {
@@ -71,3 +71,55 @@ describe('node shell state', () => {
     expect(node._graphScope).toBeUndefined()
   })
 })
+
+describe('node registration invariants', () => {
+  beforeEach(() => {
+    setActivePinia(createTestingPinia({ stubActions: false }))
+    vi.stubEnv('DEV', true)
+    vi.spyOn(console, 'error').mockImplementation(() => {})
+  })
+
+  afterEach(() => {
+    vi.unstubAllEnvs()
+    vi.restoreAllMocks()
+  })
+
+  it('refuses to register a node under a second root graph', () => {
+    const first = new LGraph()
+    const second = new LGraph()
+    second.id = createUuidv4()
+    const node = new LGraphNode('Node')
+    first.add(node)
+
+    expect(() => second.add(node)).toThrow(/different root graph/)
+    expect(node._graphScope).toBeUndefined()
+    expect(useNodeDataStore().getGraphNodesFor(first.id, first.id)).toEqual([])
+  })
+
+  it('drops the previous root entry rather than stranding it', () => {
+    vi.stubEnv('DEV', false)
+    const first = new LGraph()
+    first.id = createUuidv4()
+    const second = new LGraph()
+    second.id = createUuidv4()
+    const node = new LGraphNode('Node')
+    first.add(node)
+
+    second.add(node)
+
+    const store = useNodeDataStore()
+    const owningGraphId = node._state.graphId
+    expect(store.getGraphNodesFor(first.id, owningGraphId)).toEqual([])
+    expect(store.getGraphNodesFor(second.id, owningGraphId)).toHaveLength(1)
+  })
+
+  it('reports a state that drifted out of its bucket before unregistering', () => {
+    const graph = new LGraph()
+    const node = new LGraphNode('Node')
+    graph.add(node)
+    node._state = createNodeShellState('Node', 'test', undefined)
+
+    expect(() => unregisterNodeState(node)).toThrow(/identity drift/)
+    expect(node._graphScope).toBeUndefined()
+  })
+})

src/core/graph/nodeShell/nodeShellState.ts

@@ -1,5 +1,6 @@
 import { shallowReactive } from 'vue'
 
+import { assert } from '@/base/assert'
 import {
   canTransferLayoutAttachment,
   transferLayoutAttachment
@@ -70,8 +71,24 @@ export function registerNodeState(
   node: LGraphNode
 ): boolean {
   const graphScope = graphScopeOf(graph)
+  const store = useNodeDataStore()
+  const strandedScope =
+    node._graphScope === undefined ||
+    node._graphScope.rootGraphId === graphScope.rootGraphId
+      ? undefined
+      : node._graphScope
+
+  if (strandedScope !== undefined) {
+    store.deleteNode(strandedScope, node._state)
+    node._graphScope = undefined
+  }
+  assert(
+    strandedScope === undefined,
+    `registerNodeState: node ${node.id} already registered under a different root graph (${strandedScope?.rootGraphId})`
+  )
+
   node._state.graphId = graph.id
-  const registered = useNodeDataStore().registerNode(graphScope, node._state)
+  const registered = store.registerNode(graphScope, node._state)
   if (!registered) return false
   node._state = registered
   node._graphScope = graphScope
@@ -85,8 +102,12 @@ export function registerNodeState(
  */
 export function unregisterNodeState(node: LGraphNode): void {
   if (!node._graphScope) return
-  useNodeDataStore().deleteNode(node._graphScope, node._state)
+  const deleted = useNodeDataStore().deleteNode(node._graphScope, node._state)
   node._graphScope = undefined
+  assert(
+    deleted,
+    `unregisterNodeState: state for node ${node.id} not found in bucket (identity drift)`
+  )
 }
 
 /**