Comfy-Org / Comfy-Org/ComfyUI_frontend

ECS branch: store-level rejection is atomic, graph-level rejection is not; enumerate the gap

Open
#15,703 2 comments 0 reactions 0 assignees View on GitHub
Dominant language
TypeScript
Stars
2k
Forks
699
Avg merge
1d 7h
Merged PRs (30d)
490

Description

Rejection inside a store is side-effect free. Rejection at the graph level is not, and nobody has listed the paths where it is not.

Repo `Comfy-Org/ComfyUI_frontend`, branch `feature/ecs-migration` (PR #14246), measured at `907ca2b147`.

## What is verified atomic

`replaceLink` (`src/stores/linkStore.ts:198-234`) returns before any mutation on all three rejection paths: expected-not-owned `:204`, id already held `:209`, target occupied `:222`. `rootBucket()`, the only bucket-creating call, is at `:228`, after validation.

`updateEndpoints` (`:330-349`) runs `validateEndpointUpdates` (`:275-327`) as a read-only pass over the whole batch before the first `displace()`. `docs/architecture/link-topology-store.md` Decision 4 records the consequence: swaps and rotations commit without transient eviction.

## What is not

`docs/architecture/ecs/` risk list states that direct extension insertion which bypasses import normalization can be "rejected after partial surrounding work". That is the graph-level case, and it is one sentence in a risk list rather than a tested or enumerated property.

## Ask

1. List the graph-level operations where a store rejection leaves surrounding work committed. Start from the callers of `registerNodeState`, `replaceLink` and `updateEndpoints`.
2. For each, decide: make it atomic, or declare it non-atomic in the compatibility docs.
3. Cover the declared-non-atomic ones with tests in the shape `src/lib/litegraph/src/node/legacySlotLinkMutations.test.ts` already uses for known-broken ecosystem patterns, so the contract is pinned rather than described.

## Provenance

Found while answering Christian Byrne's questions in Slack `#p-frontend-graph-improvements`
on 2026-08-22 (threads `p1787385648421529`, `p1787386156545179`, `p1787388764721129`).
Every code claim above was re-derived against `origin/feature/ecs-migration` at `907ca2b1479381eff7a617e656c9002806b5fa15`
and `origin/main` at `a08a7598aa` on 2026-08-23T06:30Z with `git show :`, not
read from a working tree.

---
_Mirrored from Linear [FE-1812](https://linear.app/comfyorg/issue/FE-1812/ecs-branch-store-level-rejection-is-atomic-graph-level-rejection-is)._

Contributor guide

Open the contributing guide

Research direction

Start with callers of registerNodeState, replaceLink, and updateEndpoints, then read src/stores/linkStore.ts and docs/architecture/ecs/. Compare those paths with src/lib/litegraph/src/node/legacySlotLinkMutations.test.ts. Done means every graph-level rejection path is enumerated, each is either made atomic or documented as non-atomic, and the latter paths have contract tests.

Written by the indexing model from the issue text.

Assessment

Tech stack
typescript
Domain
documentation, frontend, testing
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Active
Clarity
Mostly clear
Newbie friendliness
45/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.