Comfy-Org / Comfy-Org/ComfyUI_frontend

Subgraph instances added from the node search share one promoted widget value

Open
#15,565 0 comments 0 reactions 1 assignee Claimed by @DrJKL View on GitHub
area:subgraph Potential Bug
Dominant language
TypeScript
Stars
2k
Forks
699
Avg merge
1d 7h
Merged PRs (30d)
490

Description

## Summary

Every `SubgraphNode` instance created through the node search / node library shares a single promoted-widget entry in `widgetValueStore`, keyed `:-1:`. Editing a promoted widget on one instance changes it on every other instance of the same subgraph, and deleting one can delete the other's state.

## Root cause

`SubgraphNode._setWidget` computes the widget id from the node's id at construction time:

```ts
// src/lib/litegraph/src/subgraph/SubgraphNode.ts:660
const id = widgetId(this.rootGraph.id, this.id, subgraphInput.name)
input.widgetId = id
```

The registered node class for a subgraph type is built with `instanceData.id = -1`:

```ts
// src/services/subgraphService.ts:22-32
const instanceData: ExportedSubgraphInstance = {
id: -1,
type: exportedSubgraph.id,
...
}
```

So `LiteGraph.createNode(subgraphId)` produces a node whose id is `-1`, promotion runs during construction, and `input.widgetId` is baked as `:-1:`. `LGraph.add()` then assigns the real node id, but nothing re-keys `input.widgetId`.

`SubgraphNode.configure()` does re-key (via `_internalConfigureAfterSlots` -> `_resolveInputWidget` -> `_setWidget`), which is why loading and pasting a workflow are unaffected. The node-search path never calls `configure`:

```ts
// src/services/litegraphService.ts:930-951 (addNodeOnGraph)
const node = LiteGraph.createNode(nodeDef.name, nodeDef.display_name, options)
...
graph.add(node, addOptions)
return node
```

## Reproduction

Add the same subgraph twice from the node library / node search, then change a promoted widget value on one copy. The other copy changes too.

Unit reproduction (green on `main` today, pinned as a tripwire in `src/lib/litegraph/src/subgraph/SubgraphDuplicateDeleteOrder.test.ts`):

```ts
const first = addWithoutConfigure()
const second = addWithoutConfigure()
expect(first.id).not.toBe(second.id)
expect(promotedId(first)).toBe(promotedId(second)) // both `:-1:value`
useWidgetValueStore().setValue(promotedId(first), 999)
expect(promotedValueOf(second)).toBe(999)
```

## Impact

Violates the ECS migration invariant I4 (no ambiguous entity ownership across graphs): a duplicate id lets one instance overwrite another's entity. Two instances of one shared definition are indistinguishable to the widget store.

## Suggested fix

Re-key promoted widget ids when the node id is assigned (an `onAdded` rebind, or `rebuildInputWidgetBindings()` after `LGraph.add`), or refuse to register promoted widget state while the node id is `UNASSIGNED_NODE_ID`.

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.