Comfy-Org / Comfy-Org/ComfyUI_frontend

Removing a SubgraphNode leaves its promoted WidgetState registered (pinned it.fails, untracked)

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

Description

Removing a SubgraphNode instance leaves its promoted WidgetState registered in useWidgetValueStore. The pin already exists: `it.fails('releases promoted widget state when an instance is removed')` at `src/lib/litegraph/src/subgraph/SubgraphDuplicateDeleteOrder.test.ts:202-212` (added in #15568), but no issue tracked it. Its sibling pins cite #15565; this one cited nothing.

Mechanism, at `f1bfb313d6`:

- `SubgraphNode.onRemoved` aborts listeners and calls the host widget's `onRemove` (`SubgraphNode.ts:887-900`); `_clearPromotedWidget` clears `input._widget` but never deletes the `WidgetState` (`SubgraphNode.ts:707-715`).
- `LGraph.removeNode`'s released-subgraph teardown unregisters link topologies, reroute chains, node states, and layouts (`LGraph.ts:1293-1306`). Widget values are absent from that list.

Bounded impact: `LGraph.resetAfterClear` clears all six stores with the pre-reassignment root id (`LGraph.ts:575-582`), so retention lasts until the next workflow switch or undo, not forever. Within a session, ids are monotonic (`idAllocation.ts:27-62`), so stale entries are a leak rather than a wrong-value inheritance; after an undo the root clear removes them before ids can be reused.

Pre-existing at the merge base (`a2603c59a6`), not introduced by #14246. Provenance: #12617 / #12197.

Related, distinct: #15665 (replacement path loses the promoted widgetId), #11739 / #11741 (keying design).

Suggested fix shape: the released-subgraph teardown in `LGraph.removeNode` (or `_clearPromotedWidget`) deletes the promoted `WidgetState` via the existing `deleteWidget` path that `demotePromotedInput` already uses (`promotionUtils.ts:436`). Flipping the `it.fails` at `SubgraphDuplicateDeleteOrder.test.ts:202` to `it` is the acceptance test.

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.