Comfy-Org / Comfy-Org/ComfyUI_frontend
ECS branch: nodeDataStore drops a colliding node with no diagnostic, and the caller silently re-mints its id
- Dominant language
- TypeScript
- Stars
- 2k
- Forks
- 699
- Avg merge
- 1d 7h
- Merged PRs (30d)
- 490
Description
`useNodeDataStore().registerNode` rejects a colliding node by returning `undefined` with no log, no assert and no import of `@/base/assert`. The two neighbouring stores log the same event.
Repo `Comfy-Org/ComfyUI_frontend`, branch `feature/ecs-migration` (PR #14246), measured at `907ca2b147`.
## The four registries on collision
| Store | Behaviour | Diagnostic |
| -- | -- | -- |
| `src/stores/nodeDataStore.ts:56` | rejects, returns `undefined` | **none** |
| `src/stores/linkStore.ts:198-234` | rejects, returns `undefined` | `console.error` at `:210` and `:223` |
| `src/stores/rerouteStore.ts` | rejects, returns `undefined` | `console.error` at `:131` |
| `src/stores/widgetValueStore.ts:103-106` | returns the incumbent when `existing.type === init.type`, overwrites otherwise | none |
`nodeDataStore.ts:44-56`:
```ts
function registerNode(graphScope: GraphScope, state: NodeState): NodeState | undefined {
const existingBucket = roots.get(graphScope.rootGraphId)
const incumbent = existingBucket?.byId.get(state.id)
if (incumbent && toRaw(incumbent) === toRaw(state) && incumbent.graphId === graphScope.owningGraphId)
return incumbent
if (incumbent) return undefined
```
## Why the silence costs more here than in the other stores
The only caller treats the rejection as a retry signal. `src/lib/litegraph/src/LGraph.ts:1171`:
```ts
while (!registerNodeState(this, node)) node.id = mintNodeId(state)
```
`registerNodeState` (`src/lib/litegraph/src/LGraphNode.ts:4460-4470`) forwards the `undefined` as `false`. So a genuine identity violation, two distinct `NodeState` objects claiming one id, is indistinguishable at runtime from an ordinary id collision: the node is silently renamed and the loop continues. Nothing reaches the console, and node ids are user-visible in serialized workflows.
This is the same corruption class as BE-5047 (duplicate node-instance asserts in `LGraph.add/remove`).
## Suggested fix
One `console.error` on the `if (incumbent) return undefined` branch, worded to match `linkStore.ts:210`. If BE-5046 lands, an `assert` from `src/base/assert.ts` instead, so it reaches Datadog RUM on cloud. Note that BE-5048 is already marked Done for exactly this file and no assert exists on either ref, so do not assume it is covered.
## How to verify
```bash
git show origin/feature/ecs-migration:src/stores/nodeDataStore.ts | sed -n '44,60p'
git grep -ln "from '@/base/assert'" origin/feature/ecs-migration -- 'src/**' | grep -v test
```
The second command returns exactly three files: `src/main.ts`, `src/schemas/nodeDef/inputSpecUtil.ts`, `src/scripts/changeTracker.ts`.
## Provenance
Found while answering Christian Byrne's questions in Slack `#p-frontend-graph-improvements`
on 2026-08-22 (threads `p1787385648421529`, `p1787386156545179`, `p1787388764721129`).
Every code claim above was re-derived against `origin/feature/ecs-migration` at `907ca2b1479381eff7a617e656c9002806b5fa15`
and `origin/main` at `a08a7598aa` on 2026-08-23T06:30Z with `git show :`, not
read from a working tree.
---
_Mirrored from Linear [FE-1810](https://linear.app/comfyorg/issue/FE-1810/ecs-branch-nodedatastore-drops-a-colliding-node-with-no-diagnostic-and)._
Contributor guide
Research direction
Start with src/stores/nodeDataStore.ts:44-56 and compare its collision handling with linkStore.ts:198-234, then trace the retry through src/lib/litegraph/src/LGraph.ts:1171 and LGraphNode.ts:4460-4470. Use the provided git show and git grep commands on feature/ecs-migration; done means a collision produces the intended diagnostic without changing ordinary id-retry behavior.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- typescript
- Domain
- frontend
- Issue type
- Bug
- Difficulty
- 2/5
- Estimated time
- 1-3 hours
- Activity status
- Active
- Clarity
- Clearly specified
- Newbie friendliness
- 78/100