Comfy-Org / Comfy-Org/ComfyUI_frontend
ECS branch: two link-store guards/diagnostics are graph-scoped while the invariant they protect is root-scoped
- Dominant language
- TypeScript
- Stars
- 2k
- Forks
- 702
- Avg merge
- 1d 7h
- Merged PRs (30d)
- 490
Description
Two link-store diagnostics on `feature/ecs-migration` are scoped differently from the invariant they are protecting. Neither is a live bug at this head; both mislead the next reader.
Verified at PR #14246 head `907ca2b1479381eff7a617e656c9002806b5fa15`.
## 1. `replaceLink`'s ownership error fires on a plain duplicate id
`stores/linkStore.ts`, in `replaceLink`:
```ts
console.error(
`Link ${replacement.id} belongs to graph ${incumbent.graphId}; graph ${scope.owningGraphId} cannot overwrite it.`
)
```
This fires on every `byId` hit, including the same-graph duplicate-id case, where it reads *"Link 7 belongs to graph A; graph A cannot overwrite it."*
Branch on `incumbent.graphId === scope.owningGraphId` so a plain duplicate id does not look like a cross-graph ownership violation. Nitpick, non-blocking.
## 2. `addFloatingLink`'s guard is graph-scoped; the rejection it guards is root-scoped
`LGraph.ts:2762-2768`:
```ts
if (this.links.has(floatingLink.id) || this.floatingLinks.has(floatingLink.id))
```
`this.links` and `this.floatingLinks` are both `LinkMap`s filtered to `graphScopeOf(this)`. The rejection this is protecting against is `linkStore.replaceLink`'s `bucket.byId.get(replacement.id)`, which is keyed **per root graph**. An id already held by another owner in the same root passes this check, `registerLinkTopology` then fails, and `addFloatingLink` returns `undefined` with only the store's `console.error` to show for it.
I traced `collectReservedLinkIds` and `deduplicateSubgraphLinkIds` and could not construct a load path that produces a cross-graph collision here, so this is defence-in-depth rather than a live bug. `useLinkStore().getTopology(rootGraphId, floatingLink.id)` would make the guard match the invariant.
Raised originally as review threads on #14246; filed so the findings do not depend on those threads.
Related: #15593 (the same silent-degradation class at the `updateEndpoints` call sites).
Contributor guide
Assessment
This issue has not been assessed yet.