Comfy-Org / Comfy-Org/Comfy-Desktop

Make custom node restore transactional (roll back node state on cancelled/failed restore)

Open
#1,018 0 comments 0 reactions 0 assignees View on GitHub
Dominant language
TypeScript
Stars
458
Forks
59
Avg merge
22h 18m
Merged PRs (30d)
45

Description

## Background

#991 made snapshot **restore** (and update) all-or-nothing for the two pieces that previously got stranded:

- **ComfyUI core source** — git checkout is rolled back to the pre-restore commit on cancel/failure (in-process), and a surviving op-marker triggers rollback on the next launch after a hard process kill (`recoverInterruptedComfyOp`).
- **pip packages** — `restorePipPackages` reverts its own changes from a targeted backup.

This closed the "core code stranded from its pip deps" inconsistency.

## Gap: custom node state is still not transactional

`restoreCustomNodes` (`src/main/lib/snapshots/restore.ts`) mutates many independent node directories, and only one of those mutations is currently undoable on cancel/failure:

| Operation | What it does | Reversible today? |
|-----------|-------------|-------------------|
| Remove | `fs.rm` deletes the node dir | ❌ no backup — bytes gone |
| Switch | `git checkout ` / cnr version switch in place | ❌ prior commit/version not recorded |
| Enable/disable | move dir ↔ `custom_nodes/.disabled/` | ❌ not tracked |
| Install | git clone / cnr install | ⚠️ only *partial* clones are cleaned on abort; completed installs stay if a later step fails |
| Post-install | pip installs node deps into site-packages | ❌ not captured by the pip phase backup |

Result: cancelling a restore "in the wrong place" leaves core + pip rolled back to pre-restore, but custom nodes can be left half-removed / half-switched. The current code even surfaces this: cancellation messages say *"Custom node changes may be partial."*

## Proposed direction

Make the node phase reversible and wire it into the existing transactional guard in `src/main/sources/standalone/actions.ts` so the whole restore (core + nodes + pip) becomes all-or-nothing.

Preferred approach — **inverse-operation log** (cheaper than copying everything):
- remove → back up *that* dir only, restore on rollback
- git switch → record prior commit, `git checkout` back
- cnr switch → record prior version, switch back
- install → delete on rollback
- enable/disable → move back

The unavoidable cost is backing up the dirs that are genuinely **removed** (must keep the bytes to restore them); git switches revert essentially for free. Can reuse the existing `createTargetedBackup` / `restoreFromBackup` helpers.

### Open questions / scope to decide
- **In-process cancel/failure** rollback first (covers what users actually hit). ✅ primary scope
- **Hard-kill recovery for nodes** — bigger lift: the op-marker only stores a git HEAD today; node recovery would need a persisted backup dir + undo manifest replayed by `recoverInterruptedComfyOp` on next launch. Defer?
- **Node post-install pip deps** land in site-packages and aren't reverted by the pip backup — fold into the rollback or accept as residual?

## References
- Follow-up to #991
- `restoreCustomNodes`: `src/main/lib/snapshots/restore.ts`
- Transactional guard: `src/main/sources/standalone/actions.ts`
- Op-marker / launch recovery: `src/main/lib/opMarker.ts`

Contributor guide

No contributing guide indexed for this repository

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.