Skip to content

Undo/redo leaves stale missing-node rows because loadGraphData(clean=false) skips clean() #14969

Description

@jaeone94

Summary

Undo and redo restore a previous graph without clearing missing-node state, so rows describing nodes that are no longer in the graph persist in the Errors tab.

Detail

ChangeTracker.updateState restores through loadGraphData with clean = false (src/scripts/changeTracker.ts:472):

await app.loadGraphData(prevState, false, false, this.workflow, {
  checkForRerouteMigration: false,
  silentAssetErrors: true
})

clean = false skips ComfyApp.clean(), which is where setMissingNodeTypes([]) lives. Nothing else on that path re-derives the list, so whatever the pre-undo graph contributed stays.

Reroute migration reaches the same branch — src/components/toast/RerouteMigrationToast.vue:34 also calls loadGraphData(migrated, false, false, ...).

Why now

Pre-existing, and not a regression from #14900 — but that PR is what makes it observable. Until then, clearAllErrors ran on every prompt submission and cleared missing-node state as a side effect, so a stale row from an undo was wiped the next time the user pressed Run. #14900 stops submissions from clearing it (a submission proves nothing about whether a pack got installed), so the staleness now persists until a genuine load or a Clear Workflow.

The A1111 import had the identical shape — a graph-replacing path that never reaches clean(), papered over by the submit-time clear. #14900 compensates for that one explicitly; this is the same gap on the undo path, left out to keep that PR narrow.

Suggested fix

Re-derive rather than blank. Undo can legitimately restore a graph that does have missing nodes, so setMissingNodeTypes([]) alone would under-report; rescanAndSurfaceMissingNodes(app.rootGraph) after the restore gives the correct list in both directions.

Worth checking whether it belongs in updateState or in the clean === false branch of loadGraphData — the latter covers reroute migration in the same stroke.

Scope note

Missing models and media have the same gap on this path, plus several others, but they carry async verification that outlives the graph, a per-workflow pendingWarnings cache and a two-tier repair state, so they need a different mechanism (a graph generation token and a single discard entry point). That is being planned separately. This issue is missing nodes only, which are synchronous and derived and can be fixed independently.

Raised by @christian-byrne in review of #14900.

Metadata

Metadata

Assignees

Type

No type

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions