From ec129de63dc75e884a77ca2e2ecf53f6f05ef8f9 Mon Sep 17 00:00:00 2001 From: pythongosssss <125205205+pythongosssss@users.noreply.github.com> Date: Sat, 7 Mar 2026 20:44:12 +0000 Subject: [PATCH] fix: Prevent corruption of workflow data due to checkState during graph loading (#9531) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Summary During workflow loading, the workflow data & active workflow object can be out of sync, meaning any checkState calls will overwrite data into the wrong workflow. Recreation steps: * Open 2-3 workflows * Enter builder mode > select step * Select some different inputs on each * Quickly tap the shift key (this triggers checkState) while switching tabs * After a while, you'll see the wrong inputs on the workflows Alternatively, register an extension that guarantees to call checkState during the bad phase, run this in browser devtools and switch tabs: ``` window.app.registerExtension({ name: 'bad', async afterConfigureGraph() { window.app.extensionManager.workflow.activeWorkflow.changeTracker.checkState() } }) ``` ## Changes - **What**: - Add loading graph flag - Prevent checkState calls while loading - Prevent app mode data sync while loading ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-9531-fix-Prevent-corruption-of-workflow-data-due-to-checkState-during-graph-loading-31c6d73d365081e2ab91d9145bf1d025) by [Unito](https://www.unito.io) --- src/scripts/app.ts | 234 ++++++++++++++++++----------------- src/scripts/changeTracker.ts | 10 +- src/stores/appModeStore.ts | 3 +- 3 files changed, 132 insertions(+), 115 deletions(-) diff --git a/src/scripts/app.ts b/src/scripts/app.ts index 95eda43a4d..e16146c49a 100644 --- a/src/scripts/app.ts +++ b/src/scripts/app.ts @@ -9,6 +9,7 @@ import { layoutStore } from '@/renderer/core/layout/store/layoutStore' import { flushScheduledSlotLayoutSync } from '@/renderer/extensions/vueNodes/composables/useSlotElementTracking' import { st, t } from '@/i18n' +import { ChangeTracker } from '@/scripts/changeTracker' import type { IContextMenuValue } from '@/lib/litegraph/src/interfaces' import { LGraph, @@ -1306,136 +1307,143 @@ export class ComfyApp { } } + ChangeTracker.isLoadingGraph = true try { - // @ts-expect-error Discrepancies between zod and litegraph - in progress - this.rootGraph.configure(graphData) + try { + // @ts-expect-error Discrepancies between zod and litegraph - in progress + this.rootGraph.configure(graphData) - // Save original renderer version before scaling (it gets modified during scaling) - const originalMainGraphRenderer = - this.rootGraph.extra.workflowRendererVersion + // Save original renderer version before scaling (it gets modified during scaling) + const originalMainGraphRenderer = + this.rootGraph.extra.workflowRendererVersion - // Scale main graph - ensureCorrectLayoutScale(originalMainGraphRenderer) + // Scale main graph + ensureCorrectLayoutScale(originalMainGraphRenderer) - // Scale all subgraphs that were loaded with the workflow - // Use original main graph renderer as fallback (not the modified one) - for (const subgraph of this.rootGraph.subgraphs.values()) { - ensureCorrectLayoutScale( - subgraph.extra.workflowRendererVersion || originalMainGraphRenderer, - subgraph - ) + // Scale all subgraphs that were loaded with the workflow + // Use original main graph renderer as fallback (not the modified one) + for (const subgraph of this.rootGraph.subgraphs.values()) { + ensureCorrectLayoutScale( + subgraph.extra.workflowRendererVersion || originalMainGraphRenderer, + subgraph + ) + } + + if (canvasVisible) fitView() + } catch (error) { + useDialogService().showErrorDialog(error, { + title: t('errorDialog.loadWorkflowTitle'), + reportType: 'loadWorkflowError' + }) + console.error(error) + return } - - if (canvasVisible) fitView() - } catch (error) { - useDialogService().showErrorDialog(error, { - title: t('errorDialog.loadWorkflowTitle'), - reportType: 'loadWorkflowError' - }) - console.error(error) - return - } - forEachNode(this.rootGraph, (node) => { - const size = node.computeSize() - size[0] = Math.max(node.size[0], size[0]) - size[1] = Math.max(node.size[1], size[1]) - node.setSize(size) - if (node.widgets) { - // If you break something in the backend and want to patch workflows in the frontend - // This is the place to do this - for (let widget of node.widgets) { - if (node.type == 'KSampler' || node.type == 'KSamplerAdvanced') { - if (widget.name == 'sampler_name') { - if ( - typeof widget.value === 'string' && - widget.value.startsWith('sample_') - ) { - widget.value = widget.value.slice(7) + forEachNode(this.rootGraph, (node) => { + const size = node.computeSize() + size[0] = Math.max(node.size[0], size[0]) + size[1] = Math.max(node.size[1], size[1]) + node.setSize(size) + if (node.widgets) { + // If you break something in the backend and want to patch workflows in the frontend + // This is the place to do this + for (let widget of node.widgets) { + if (node.type == 'KSampler' || node.type == 'KSamplerAdvanced') { + if (widget.name == 'sampler_name') { + if ( + typeof widget.value === 'string' && + widget.value.startsWith('sample_') + ) { + widget.value = widget.value.slice(7) + } } } - } - if ( - node.type == 'KSampler' || - node.type == 'KSamplerAdvanced' || - node.type == 'PrimitiveNode' - ) { - if (widget.name == 'control_after_generate') { - if (widget.value === true) { - widget.value = 'randomize' - } else if (widget.value === false) { - widget.value = 'fixed' - } - } - } - if (widget.type == 'combo') { - const values = widget.options.values as - | (string | number | boolean)[] - | undefined if ( - values && - values.length > 0 && - (widget.value == null || - (reset_invalid_values && - !values.includes(widget.value as string | number | boolean))) + node.type == 'KSampler' || + node.type == 'KSamplerAdvanced' || + node.type == 'PrimitiveNode' ) { - widget.value = values[0] + if (widget.name == 'control_after_generate') { + if (widget.value === true) { + widget.value = 'randomize' + } else if (widget.value === false) { + widget.value = 'fixed' + } + } + } + if (widget.type == 'combo') { + const values = widget.options.values as + | (string | number | boolean)[] + | undefined + if ( + values && + values.length > 0 && + (widget.value == null || + (reset_invalid_values && + !values.includes( + widget.value as string | number | boolean + ))) + ) { + widget.value = values[0] + } } } } + + useExtensionService().invokeExtensions('loadedGraphNode', node) + }) + + await useExtensionService().invokeExtensionsAsync( + 'afterConfigureGraph', + missingNodeTypes + ) + + const telemetryPayload = { + missing_node_count: missingNodeTypes.length, + missing_node_types: missingNodeTypes.map((node) => + typeof node === 'string' ? node : node.type + ), + open_source: openSource ?? 'unknown' + } + useTelemetry()?.trackWorkflowOpened(telemetryPayload) + useTelemetry()?.trackWorkflowImported(telemetryPayload) + await useWorkflowService().afterLoadNewGraph( + workflow, + this.rootGraph.serialize() as unknown as ComfyWorkflowJSON + ) + + // If the canvas was not visible and we're a fresh load, resize the canvas and fit the view + // This fixes switching from app mode to a new graph mode workflow (e.g. load template) + if (!canvasVisible && (!workflow || typeof workflow === 'string')) { + this.canvas.resize() + requestAnimationFrame(() => fitView()) } - useExtensionService().invokeExtensions('loadedGraphNode', node) - }) - - await useExtensionService().invokeExtensionsAsync( - 'afterConfigureGraph', - missingNodeTypes - ) - - const telemetryPayload = { - missing_node_count: missingNodeTypes.length, - missing_node_types: missingNodeTypes.map((node) => - typeof node === 'string' ? node : node.type - ), - open_source: openSource ?? 'unknown' - } - useTelemetry()?.trackWorkflowOpened(telemetryPayload) - useTelemetry()?.trackWorkflowImported(telemetryPayload) - await useWorkflowService().afterLoadNewGraph( - workflow, - this.rootGraph.serialize() as unknown as ComfyWorkflowJSON - ) - - // If the canvas was not visible and we're a fresh load, resize the canvas and fit the view - // This fixes switching from app mode to a new graph mode workflow (e.g. load template) - if (!canvasVisible && (!workflow || typeof workflow === 'string')) { - this.canvas.resize() - requestAnimationFrame(() => fitView()) - } - - // Store pending warnings on the workflow for deferred display - const activeWf = useWorkspaceStore().workflow.activeWorkflow - if (activeWf) { - const warnings: PendingWarnings = {} - if (missingNodeTypes.length && showMissingNodesDialog) { - warnings.missingNodeTypes = missingNodeTypes + // Store pending warnings on the workflow for deferred display + const activeWf = useWorkspaceStore().workflow.activeWorkflow + if (activeWf) { + const warnings: PendingWarnings = {} + if (missingNodeTypes.length && showMissingNodesDialog) { + warnings.missingNodeTypes = missingNodeTypes + } + if (missingModels.length && showMissingModelsDialog) { + const paths = await api.getFolderPaths() + warnings.missingModels = { missingModels: missingModels, paths } + } + if (warnings.missingNodeTypes || warnings.missingModels) { + activeWf.pendingWarnings = warnings + } } - if (missingModels.length && showMissingModelsDialog) { - const paths = await api.getFolderPaths() - warnings.missingModels = { missingModels: missingModels, paths } - } - if (warnings.missingNodeTypes || warnings.missingModels) { - activeWf.pendingWarnings = warnings - } - } - if (!deferWarnings) { - useWorkflowService().showPendingWarnings() - } + if (!deferWarnings) { + useWorkflowService().showPendingWarnings() + } - requestAnimationFrame(() => { - this.canvas.setDirty(true, true) - }) + requestAnimationFrame(() => { + this.canvas.setDirty(true, true) + }) + } finally { + ChangeTracker.isLoadingGraph = false + } } async graphToPrompt(graph = this.rootGraph) { diff --git a/src/scripts/changeTracker.ts b/src/scripts/changeTracker.ts index 0d7d87c8e3..cdc3942cf1 100644 --- a/src/scripts/changeTracker.ts +++ b/src/scripts/changeTracker.ts @@ -28,6 +28,14 @@ logger.setLevel('info') export class ChangeTracker { static MAX_HISTORY = 50 + /** + * Guard flag to prevent checkState from running during loadGraphData. + * Between rootGraph.configure() and afterLoadNewGraph(), the rootGraph + * contains the NEW workflow's data while activeWorkflow still points to + * the OLD workflow. Any checkState call in that window would serialize + * the wrong graph into the old workflow's activeState, corrupting it. + */ + static isLoadingGraph = false /** * The active state of the workflow. */ @@ -131,7 +139,7 @@ export class ChangeTracker { } checkState() { - if (!app.graph || this.changeCount) return + if (!app.graph || this.changeCount || ChangeTracker.isLoadingGraph) return const currentState = clone(app.rootGraph.serialize()) as ComfyWorkflowJSON if (!this.activeState) { this.activeState = currentState diff --git a/src/stores/appModeStore.ts b/src/stores/appModeStore.ts index 5fec531556..a4eb7e8e7f 100644 --- a/src/stores/appModeStore.ts +++ b/src/stores/appModeStore.ts @@ -9,6 +9,7 @@ import { useCanvasStore } from '@/renderer/core/canvas/canvasStore' import { useWorkflowStore } from '@/platform/workflow/management/stores/workflowStore' import { useSidebarTabStore } from '@/stores/workspace/sidebarTabStore' import { app } from '@/scripts/app' +import { ChangeTracker } from '@/scripts/changeTracker' import { resolveNode } from '@/utils/litegraphUtil' export const useAppModeStore = defineStore('appMode', () => { @@ -77,7 +78,7 @@ export const useAppModeStore = defineStore('appMode', () => { ? { inputs: selectedInputs, outputs: selectedOutputs } : null, (data) => { - if (!data) return + if (!data || ChangeTracker.isLoadingGraph) return const graph = app.rootGraph if (!graph) return const extra = (graph.extra ??= {})