Comfy-Org / Comfy-Org/ComfyUI_frontend

ECS branch: output.links mutations are half-honoured — additions discarded, removals applied (breaks rgthree link repair)

Open
#15,620 3 comments 0 reactions 1 assignee Claimed by @DrJKL View on GitHub
Custom Node Potential Bug Public API
Dominant language
TypeScript
Stars
2k
Forks
699
Avg merge
1d 7h
Merged PRs (30d)
490

Description

On `feature/ecs-migration` (verified at `5002fae1b1`), loading a previously-saved workflow that
contains a connected **rgthree Power Prompt** in the old output format **deletes the node's output
wiring** instead of migrating it. On `main` the same load migrates correctly.

This is not the mirror-only idiom of #15577. The cause is that the legacy accessor honours
*removals* and discards *additions*, so any pack idiom that **moves** links through the mirror
loses them.

### The mechanism

`NodeOutputSlot`'s own docstring states the asymmetry:

> Reads return a stable store-derived view. **Removing ids from the view disconnects them;
> additions are discarded.**

And the setter is genuinely destructive — `commitLegacyLinks()` disconnects every link not in the
assigned set:

```ts
for (const link of outputLinks(graph, this._node.id, slot)) {
if (desired.has(link.id)) continue
graph.getNodeById(link.target_id)?.disconnectInput(link.target_slot)
}
```

### The pack code, and why the guards make it worse

`rgthree-comfy/web/comfyui/power_prompt.js`, inside **`loadedGraphNode`** — so it runs on every
workflow load, not at registration:

```js
if (node.outputs[0].type === "STRING") { // an OLD-FORMAT saved workflow
if (node.outputs[0].links) { // that HAS connected outputs
node.outputs[3].links = node.outputs[3].links || [] // assignment #1 -> setter fires
for (const link of node.outputs[0].links) {
node.outputs[3].links.push(link) // addition -> DISCARDED
app.graph.links[link].origin_slot = 3
}
node.outputs[0].links = null // removal -> DISCONNECTS EVERY CONSUMER
}
```

Additions discarded + removal honoured = the links are destroyed rather than moved to slot 3. The
guards select precisely the users being migrated from an older save format.

### Reachability, measured

Corpus of 29 installed packs / 157 frontend files. Control `/registerExtension/` -> **84 files**
(instrument live). `\.links\s*=\s*(\[|null)` -> **1 file, 1 site, 1 pack**.
`\.link\s*=\s*null` -> **0 files** — a non-vacuous zero, given the live control.

One pack, but rgthree is among the most-installed, and the path is workflow load.

### The governing record states the inverse

`docs/adr/0008-entity-component-system.md:197`:

> The `input.link` / `output.links` properties are deprecated, read-only compatibility accessors
> derived from the store; **assignments are ignored.**

If assignments really were ignored this pack would be a harmless no-op — links would stay on slot 0,
cosmetically wrong but intact. **The half-honoured design is what creates the data loss.**
`interfaces.ts:373/395` also carry no `readonly`, so the type permits the write.

Related: #15618 (the seven-document divergence), #15577 (mirror-only idiom, same pack).

### Suggested resolution

Either make the setter fully inert (matching the ADR), or make additions commit as well as
removals. A half-honoured accessor is the only variant that loses data.

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.