Comfy-Org / Comfy-Org/Comfy-Desktop

ComfyBuilder updates: define and implement a custom_nodes preservation policy

Open
#1,410 1 comment 1 reaction 0 assignees View on GitHub
comfy-builder
Dominant language
TypeScript
Stars
458
Forks
59
Avg merge
22h 18m
Merged PRs (30d)
45

Description

Split out of #1408. The urgent part of that issue (preserving `ComfyUI/user` across ComfyBuilder version updates, rollbacks, and crash recovery) is fixed on the PR #1404 branch. `ComfyUI/custom_nodes` is intentionally NOT preserved yet because the right policy needs a product decision from someone who knows what ComfyBuilder distributions are supposed to support.

## Current behavior

The ComfyBuilder update transaction (`src/main/sources/comfybuilder/index.ts`) replaces the whole `ComfyUI` tree. `ComfyUI/models` and `ComfyUI/user` are detached before the swap and restored afterward; `ComfyUI/custom_nodes` is not, so node packs the user installed through Manager v4 (reachable: default launch args include `--enable-manager`) are silently deleted on every successful version update.

## Why blind preservation is wrong

- Distributions can ship their own `custom_nodes` as part of the built artifact. Preserving the whole old directory would clobber or shadow distribution-shipped nodes on name collision, defeating the point of a curated build.
- User-installed node packs carry Python dependencies installed into the venv that the update just replaced. A preserved node pack may import against packages that no longer exist in the new venv, turning a clean update into a broken boot.
- A node pack may be incompatible with the new ComfyUI version itself.

## Proposed starting point (needs owner sign-off)

On name collision the new distribution wins; user node packs absent from the new artifact are retained. Then either re-run their dependency install steps against the new venv, or surface a post-update report listing retained packs that failed to import so the user can reinstall them through Manager.

Open questions for the ComfyBuilder owner:

- Are users supposed to install extra node packs into a ComfyBuilder-managed install at all, or is the distribution the sole source of nodes? If the latter, the fix is to disable Manager pack installation for this route instead of preserving anything.
- If user packs are supported, who owns dependency re-resolution after a venv swap - desktop, Manager v4, or the builder manifest?
- Should retained packs be disabled by default until they import successfully against the new environment?

## Acceptance criteria

- [ ] Policy decision recorded here by the ComfyBuilder owner
- [ ] Update/rollback/crash-recovery transaction implements the chosen policy with the same interleaving coverage as models/user preservation
- [ ] User-visible outcome (retained/dropped/disabled packs) is surfaced, not silent

Refs #1408, #1404.

Contributor guide

No contributing guide indexed for this repository

Research direction

Start with src/main/sources/comfybuilder/index.ts and compare its update transaction with the models/user preservation work on the PR #1404 branch. First get the ComfyBuilder owner’s policy decision; done means the chosen behavior covers update, rollback, and crash recovery, and surfaces whether node packs were retained, dropped, or disabled.

Written by the indexing model from the issue text.

Assessment

Tech stack
python, typescript
Domain
desktop, release
Issue type
Feature
Difficulty
5/5
Estimated time
Over a week
Activity status
Quiet
Clarity
Needs clarification
Newbie friendliness
25/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.