Comfy-Org / Comfy-Org/ComfyUI_frontend

ECS: same-id SubgraphNode replacement loses the promoted input's widgetId, orphaning its stored value

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

Description

On `feature/ecs-migration`, replacing a `SubgraphNode` with a new instance carrying the **same node id** leaves the replacement's promoted input with `widgetId: undefined`. On `main` it carries the original `widgetId`. The promoted widget's stored value is orphaned in `widgetValueStore`.

Same-id host replacement is what node replacement and workflow reload do, so this is user-reachable: a promoted subgraph widget loses its value.

## Repro

Standalone vitest probe, run at both refs:

```ts
const subgraph = createTestSubgraph()
const host = createTestSubgraphNode(subgraph, { id: 7 })
subgraph.rootGraph.add(host)

const interior = new LGraphNode('TestNode')
subgraph.add(interior)
const valueInput = interior.addInput('value', 'STRING')
const valueWidget = interior.addWidget('text', 'value', 'initial', () => {})
valueInput.widget = { name: valueWidget.name }
promoteValueWidgetViaSubgraphInput(host, interior, valueWidget)

const originalId = host.inputs.find((i) => i.name === 'value')?.widgetId

subgraph.rootGraph.remove(host)
await Promise.resolve()

const replacement = createTestSubgraphNode(subgraph, { id: 7 })
subgraph.rootGraph.add(replacement)

expect(replacement.inputs.find((i) => i.name === 'value')?.widgetId).toBe(originalId)
```

## Result

| ref | replacement promoted input `widgetId` | |
| --- | --- | --- |
| `main` `f9ec97a7ca` | `'00000000-0000-4000-8000-000000000001:7:value'` | PASS |
| `feature/ecs-migration` `907ca2b147` | `undefined` | FAIL |

## How it was found

Running the tests from #15550 (written against `main`, not yet merged) at branch head. 22 of its 51 tests fail there; this is the only one of the 22 that is a behaviour regression rather than a removed-API or characterisation mismatch. Reproduced above with a probe that does not use that PR's test file.

## Where

`SubgraphNode.ts` sets `input.widgetId = widgetId(this.rootGraph.id, this.id, subgraphInput.name)` in the promotion path. On the branch that path does not appear to run for a replacement instance that re-adopts an existing subgraph input. `git blame` lands on `13a302eadd` (`refactor!: migrate entity state to dedicated stores`), authored by `Amp `; the last human author of this region is @DrJKL.

Related: the re-promotion guard at `SubgraphNode.ts:364` has no test covering its removal (workspace note `cr-fu-06`).

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.