Comfy-Org / Comfy-Org/ComfyUI_frontend
ECS branch: rejected topology updates degrade silently at all three updateEndpoints call sites
- Dominant language
- TypeScript
- Stars
- 2k
- Forks
- 699
- Avg merge
- 1d 7h
- Merged PRs (30d)
- 490
Description
Found while writing the repros for #15577 and #15581 (PR #15592). Those two are instances of a class, and this is the third and worst instance. Branch `feature/ecs-migration`, head `9f6ab9adda`.
## The class
Every `updateEndpoints` call site on the branch swallows a rejected topology update into `console.error` and continues with degraded state. There are three, and each degrades differently:
| Site | On rejection | Result |
| --- | --- | --- |
| `linkDeduplication.ts:158` | logs, `break` | abandons the node's remaining moves (#15581) |
| `LLink.ts:115` | logs, falls through | caller proceeds as if the endpoint moved |
| `node/slotLinks.ts:192` | logs, `return []` | node keeps its **old** input layout; `[]` is indistinguishable from "nothing to replace" |
None marks `has_errors`. None uses the repo's unified `reportError`. Raw `console.error` is auto-captured by Datadog RUM but untagged, so a load that silently re-wires a graph produces no searchable signal.
## Why `slotLinks.ts:192` is the worst of the three
```ts
const result = store.updateEndpoints(graphScopeOf(node.graph), updates, removals.map(...))
if (!result.ok) {
console.error('Failed to replace node inputs', result.error)
return []
}
node.inputs.splice(0, node.inputs.length, ...finalInputs)
```
The early return happens **before** the splice. So on a rejected batch the node keeps its previous input layout while the caller believes `replaceNodeInputs` ran, and the empty return value is the same value a successful no-op replacement produces. There is no way for a caller to tell the two apart.
## Why this may already be a known break
`replaceNodeInputs` is the indexed-slot-replacement path. comfyui-promptchain (14,814 installs) does exactly `node.inputs[i] = {...slot}`, and it is the confirmed pack break on this branch. If promptchain's failure is a rejected `replaceNodeInputs` returning `[]`, then the shim currently being scoped for it is being designed against a symptom rather than the cause.
Worth testing that before the shim is specified.
## Asks
1. Decide what a rejected topology update should do. One decision for the class, not three patches.
2. At minimum, make `replaceNodeInputs` distinguishable: return a result type, or throw, rather than an empty array that means two different things.
3. Route these through `reportError` so they are searchable, and mark the node `has_errors` where a graph has actually been left mis-wired.
## Related
- #15577, #15581 — the other two instances, both with repros in #15592
Contributor guide
Assessment
This issue has not been assessed yet.