mirror of
https://github.com/Comfy-Org/ComfyUI_frontend.git
synced 2026-04-26 09:19:43 +00:00
fix: Prevent corruption of workflow data due to checkState during graph loading (#9531)
## 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)
This commit is contained in:
@@ -9,6 +9,7 @@ import { layoutStore } from '@/renderer/core/layout/store/layoutStore'
|
|||||||
import { flushScheduledSlotLayoutSync } from '@/renderer/extensions/vueNodes/composables/useSlotElementTracking'
|
import { flushScheduledSlotLayoutSync } from '@/renderer/extensions/vueNodes/composables/useSlotElementTracking'
|
||||||
|
|
||||||
import { st, t } from '@/i18n'
|
import { st, t } from '@/i18n'
|
||||||
|
import { ChangeTracker } from '@/scripts/changeTracker'
|
||||||
import type { IContextMenuValue } from '@/lib/litegraph/src/interfaces'
|
import type { IContextMenuValue } from '@/lib/litegraph/src/interfaces'
|
||||||
import {
|
import {
|
||||||
LGraph,
|
LGraph,
|
||||||
@@ -1306,136 +1307,143 @@ export class ComfyApp {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
ChangeTracker.isLoadingGraph = true
|
||||||
try {
|
try {
|
||||||
// @ts-expect-error Discrepancies between zod and litegraph - in progress
|
try {
|
||||||
this.rootGraph.configure(graphData)
|
// @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)
|
// Save original renderer version before scaling (it gets modified during scaling)
|
||||||
const originalMainGraphRenderer =
|
const originalMainGraphRenderer =
|
||||||
this.rootGraph.extra.workflowRendererVersion
|
this.rootGraph.extra.workflowRendererVersion
|
||||||
|
|
||||||
// Scale main graph
|
// Scale main graph
|
||||||
ensureCorrectLayoutScale(originalMainGraphRenderer)
|
ensureCorrectLayoutScale(originalMainGraphRenderer)
|
||||||
|
|
||||||
// Scale all subgraphs that were loaded with the workflow
|
// Scale all subgraphs that were loaded with the workflow
|
||||||
// Use original main graph renderer as fallback (not the modified one)
|
// Use original main graph renderer as fallback (not the modified one)
|
||||||
for (const subgraph of this.rootGraph.subgraphs.values()) {
|
for (const subgraph of this.rootGraph.subgraphs.values()) {
|
||||||
ensureCorrectLayoutScale(
|
ensureCorrectLayoutScale(
|
||||||
subgraph.extra.workflowRendererVersion || originalMainGraphRenderer,
|
subgraph.extra.workflowRendererVersion || originalMainGraphRenderer,
|
||||||
subgraph
|
subgraph
|
||||||
)
|
)
|
||||||
|
}
|
||||||
|
|
||||||
|
if (canvasVisible) fitView()
|
||||||
|
} catch (error) {
|
||||||
|
useDialogService().showErrorDialog(error, {
|
||||||
|
title: t('errorDialog.loadWorkflowTitle'),
|
||||||
|
reportType: 'loadWorkflowError'
|
||||||
|
})
|
||||||
|
console.error(error)
|
||||||
|
return
|
||||||
}
|
}
|
||||||
|
forEachNode(this.rootGraph, (node) => {
|
||||||
if (canvasVisible) fitView()
|
const size = node.computeSize()
|
||||||
} catch (error) {
|
size[0] = Math.max(node.size[0], size[0])
|
||||||
useDialogService().showErrorDialog(error, {
|
size[1] = Math.max(node.size[1], size[1])
|
||||||
title: t('errorDialog.loadWorkflowTitle'),
|
node.setSize(size)
|
||||||
reportType: 'loadWorkflowError'
|
if (node.widgets) {
|
||||||
})
|
// If you break something in the backend and want to patch workflows in the frontend
|
||||||
console.error(error)
|
// This is the place to do this
|
||||||
return
|
for (let widget of node.widgets) {
|
||||||
}
|
if (node.type == 'KSampler' || node.type == 'KSamplerAdvanced') {
|
||||||
forEachNode(this.rootGraph, (node) => {
|
if (widget.name == 'sampler_name') {
|
||||||
const size = node.computeSize()
|
if (
|
||||||
size[0] = Math.max(node.size[0], size[0])
|
typeof widget.value === 'string' &&
|
||||||
size[1] = Math.max(node.size[1], size[1])
|
widget.value.startsWith('sample_')
|
||||||
node.setSize(size)
|
) {
|
||||||
if (node.widgets) {
|
widget.value = widget.value.slice(7)
|
||||||
// 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 (
|
if (
|
||||||
values &&
|
node.type == 'KSampler' ||
|
||||||
values.length > 0 &&
|
node.type == 'KSamplerAdvanced' ||
|
||||||
(widget.value == null ||
|
node.type == 'PrimitiveNode'
|
||||||
(reset_invalid_values &&
|
|
||||||
!values.includes(widget.value as string | number | boolean)))
|
|
||||||
) {
|
) {
|
||||||
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)
|
// Store pending warnings on the workflow for deferred display
|
||||||
})
|
const activeWf = useWorkspaceStore().workflow.activeWorkflow
|
||||||
|
if (activeWf) {
|
||||||
await useExtensionService().invokeExtensionsAsync(
|
const warnings: PendingWarnings = {}
|
||||||
'afterConfigureGraph',
|
if (missingNodeTypes.length && showMissingNodesDialog) {
|
||||||
missingNodeTypes
|
warnings.missingNodeTypes = missingNodeTypes
|
||||||
)
|
}
|
||||||
|
if (missingModels.length && showMissingModelsDialog) {
|
||||||
const telemetryPayload = {
|
const paths = await api.getFolderPaths()
|
||||||
missing_node_count: missingNodeTypes.length,
|
warnings.missingModels = { missingModels: missingModels, paths }
|
||||||
missing_node_types: missingNodeTypes.map((node) =>
|
}
|
||||||
typeof node === 'string' ? node : node.type
|
if (warnings.missingNodeTypes || warnings.missingModels) {
|
||||||
),
|
activeWf.pendingWarnings = warnings
|
||||||
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
|
|
||||||
}
|
}
|
||||||
if (missingModels.length && showMissingModelsDialog) {
|
|
||||||
const paths = await api.getFolderPaths()
|
|
||||||
warnings.missingModels = { missingModels: missingModels, paths }
|
|
||||||
}
|
|
||||||
if (warnings.missingNodeTypes || warnings.missingModels) {
|
|
||||||
activeWf.pendingWarnings = warnings
|
|
||||||
}
|
|
||||||
}
|
|
||||||
|
|
||||||
if (!deferWarnings) {
|
if (!deferWarnings) {
|
||||||
useWorkflowService().showPendingWarnings()
|
useWorkflowService().showPendingWarnings()
|
||||||
}
|
}
|
||||||
|
|
||||||
requestAnimationFrame(() => {
|
requestAnimationFrame(() => {
|
||||||
this.canvas.setDirty(true, true)
|
this.canvas.setDirty(true, true)
|
||||||
})
|
})
|
||||||
|
} finally {
|
||||||
|
ChangeTracker.isLoadingGraph = false
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
async graphToPrompt(graph = this.rootGraph) {
|
async graphToPrompt(graph = this.rootGraph) {
|
||||||
|
|||||||
@@ -28,6 +28,14 @@ logger.setLevel('info')
|
|||||||
|
|
||||||
export class ChangeTracker {
|
export class ChangeTracker {
|
||||||
static MAX_HISTORY = 50
|
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.
|
* The active state of the workflow.
|
||||||
*/
|
*/
|
||||||
@@ -131,7 +139,7 @@ export class ChangeTracker {
|
|||||||
}
|
}
|
||||||
|
|
||||||
checkState() {
|
checkState() {
|
||||||
if (!app.graph || this.changeCount) return
|
if (!app.graph || this.changeCount || ChangeTracker.isLoadingGraph) return
|
||||||
const currentState = clone(app.rootGraph.serialize()) as ComfyWorkflowJSON
|
const currentState = clone(app.rootGraph.serialize()) as ComfyWorkflowJSON
|
||||||
if (!this.activeState) {
|
if (!this.activeState) {
|
||||||
this.activeState = currentState
|
this.activeState = currentState
|
||||||
|
|||||||
@@ -9,6 +9,7 @@ import { useCanvasStore } from '@/renderer/core/canvas/canvasStore'
|
|||||||
import { useWorkflowStore } from '@/platform/workflow/management/stores/workflowStore'
|
import { useWorkflowStore } from '@/platform/workflow/management/stores/workflowStore'
|
||||||
import { useSidebarTabStore } from '@/stores/workspace/sidebarTabStore'
|
import { useSidebarTabStore } from '@/stores/workspace/sidebarTabStore'
|
||||||
import { app } from '@/scripts/app'
|
import { app } from '@/scripts/app'
|
||||||
|
import { ChangeTracker } from '@/scripts/changeTracker'
|
||||||
import { resolveNode } from '@/utils/litegraphUtil'
|
import { resolveNode } from '@/utils/litegraphUtil'
|
||||||
|
|
||||||
export const useAppModeStore = defineStore('appMode', () => {
|
export const useAppModeStore = defineStore('appMode', () => {
|
||||||
@@ -77,7 +78,7 @@ export const useAppModeStore = defineStore('appMode', () => {
|
|||||||
? { inputs: selectedInputs, outputs: selectedOutputs }
|
? { inputs: selectedInputs, outputs: selectedOutputs }
|
||||||
: null,
|
: null,
|
||||||
(data) => {
|
(data) => {
|
||||||
if (!data) return
|
if (!data || ChangeTracker.isLoadingGraph) return
|
||||||
const graph = app.rootGraph
|
const graph = app.rootGraph
|
||||||
if (!graph) return
|
if (!graph) return
|
||||||
const extra = (graph.extra ??= {})
|
const extra = (graph.extra ??= {})
|
||||||
|
|||||||
Reference in New Issue
Block a user