Comfy-Org / Comfy-Org/ComfyUI_frontend

ECS branch: nodeDataStore drops a colliding node with no diagnostic, and the caller silently re-mints its id

Open Beginner friendly
#15,701 8 comments 0 reactions 0 assignees View on GitHub
area:nodes Potential Bug
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

Open the contributing 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

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.