[backport core/1.40] fix: spin out workflow tab/load stability regressions (#9345) (#9576)

Backport of #9345 to core/1.40. Stability fix for workflow tab loading.

Conflicts: import additions and new test block in workflowService —
accepted incoming.

**Original PR:** https://github.com/Comfy-Org/ComfyUI_frontend/pull/9345
**Pipeline ticket:** 15e1f241-efaa-4fe5-88ca-4ccc7bfb3345

┆Issue is synchronized with this [Notion
page](https://www.notion.so/PR-9576-backport-core-1-40-fix-spin-out-workflow-tab-load-stability-regressions-9345-31d6d73d36508126a4a6f7cccf592272)
by [Unito](https://www.unito.io)

Co-authored-by: Alexander Brown <drjkl@comfy.org>
Co-authored-by: Amp <amp@ampcode.com>
This commit is contained in:
Christian Byrne
2026-03-07 18:30:50 -08:00
committed by GitHub
parent e181ec95b0
commit 22eefc4222
3 changed files with 615 additions and 17 deletions

View File

@@ -253,4 +253,575 @@ describe('useWorkflowService', () => {
expect(mockShowMissingNodes).toHaveBeenCalledTimes(1)
})
})
describe('saveWorkflow', () => {
let workflowStore: ReturnType<typeof useWorkflowStore>
beforeEach(() => {
setActivePinia(createTestingPinia())
workflowStore = useWorkflowStore()
})
it('should delegate to workflowStore.saveWorkflow for persisted workflows', async () => {
const workflow = createModeTestWorkflow({
path: 'workflows/persisted.json'
})
vi.mocked(workflowStore.saveWorkflow).mockResolvedValue()
await useWorkflowService().saveWorkflow(workflow)
expect(workflowStore.saveWorkflow).toHaveBeenCalledWith(workflow)
})
it('should call saveWorkflowAs for temporary workflows', async () => {
const workflow = createModeTestWorkflow({
path: 'workflows/Unsaved Workflow.json'
})
Object.defineProperty(workflow, 'isTemporary', { get: () => true })
vi.spyOn(workflow, 'promptSave').mockResolvedValue(null)
await useWorkflowService().saveWorkflow(workflow)
expect(workflowStore.saveWorkflow).not.toHaveBeenCalled()
})
})
describe('saveWorkflowAs', () => {
let workflowStore: ReturnType<typeof useWorkflowStore>
beforeEach(() => {
setActivePinia(createTestingPinia())
workflowStore = useWorkflowStore()
})
it('should rename then save when workflow is temporary', async () => {
const workflow = createModeTestWorkflow({
path: 'workflows/Unsaved Workflow.json'
})
Object.defineProperty(workflow, 'isTemporary', { get: () => true })
vi.mocked(workflowStore.getWorkflowByPath).mockReturnValue(null)
vi.mocked(workflowStore.renameWorkflow).mockResolvedValue()
vi.mocked(workflowStore.saveWorkflow).mockResolvedValue()
const result = await useWorkflowService().saveWorkflowAs(workflow, {
filename: 'my-workflow'
})
expect(result).toBe(true)
expect(workflowStore.renameWorkflow).toHaveBeenCalledWith(
workflow,
'workflows/my-workflow.json'
)
expect(workflowStore.saveWorkflow).toHaveBeenCalledWith(workflow)
})
it('should return false when no filename is provided', async () => {
const workflow = createModeTestWorkflow({
path: 'workflows/test.json'
})
vi.spyOn(workflow, 'promptSave').mockResolvedValue(null)
const result = await useWorkflowService().saveWorkflowAs(workflow)
expect(result).toBe(false)
expect(workflowStore.saveWorkflow).not.toHaveBeenCalled()
})
})
describe('afterLoadNewGraph', () => {
let workflowStore: ReturnType<typeof useWorkflowStore>
let existingWorkflow: LoadedComfyWorkflow
beforeEach(() => {
setActivePinia(createTestingPinia())
workflowStore = useWorkflowStore()
existingWorkflow = createModeTestWorkflow({
path: 'workflows/repeat.json'
})
vi.mocked(workflowStore.getWorkflowByPath).mockReturnValue(
existingWorkflow
)
vi.mocked(workflowStore.isActive).mockReturnValue(true)
vi.mocked(workflowStore.openWorkflow).mockResolvedValue(existingWorkflow)
})
it('should reuse the active workflow when loading the same path repeatedly', async () => {
const workflowId = 'repeat-workflow-id'
existingWorkflow.changeTracker.activeState.id = workflowId
await useWorkflowService().afterLoadNewGraph('repeat', {
id: workflowId,
nodes: [{ id: 1, type: 'TestNode', pos: [0, 0], size: [100, 100] }]
} as never)
expect(workflowStore.getWorkflowByPath).toHaveBeenCalledWith(
'workflows/repeat.json'
)
expect(workflowStore.openWorkflow).toHaveBeenCalledWith(existingWorkflow)
expect(existingWorkflow.changeTracker.reset).toHaveBeenCalled()
expect(existingWorkflow.changeTracker.restore).toHaveBeenCalled()
expect(workflowStore.createNewTemporary).not.toHaveBeenCalled()
})
it('should reuse active workflow for repeated same-path loads without ids', async () => {
await useWorkflowService().afterLoadNewGraph('repeat', {
nodes: [{ id: 1, type: 'TestNode', pos: [0, 0], size: [100, 100] }]
} as never)
expect(workflowStore.getWorkflowByPath).toHaveBeenCalledWith(
'workflows/repeat.json'
)
expect(workflowStore.openWorkflow).toHaveBeenCalledWith(existingWorkflow)
expect(existingWorkflow.changeTracker.reset).toHaveBeenCalled()
expect(existingWorkflow.changeTracker.restore).toHaveBeenCalled()
expect(workflowStore.createNewTemporary).not.toHaveBeenCalled()
})
it('should reuse active workflow when only one side has an id', async () => {
existingWorkflow.changeTracker.activeState.id = 'existing-id'
await useWorkflowService().afterLoadNewGraph('repeat', {
nodes: [{ id: 1, type: 'TestNode', pos: [0, 0], size: [100, 100] }]
} as never)
expect(workflowStore.openWorkflow).toHaveBeenCalledWith(existingWorkflow)
expect(existingWorkflow.changeTracker.reset).toHaveBeenCalled()
expect(existingWorkflow.changeTracker.restore).toHaveBeenCalled()
expect(workflowStore.createNewTemporary).not.toHaveBeenCalled()
})
it('should reuse active workflow when only workflowData has an id', async () => {
await useWorkflowService().afterLoadNewGraph('repeat', {
id: 'incoming-id',
nodes: [{ id: 1, type: 'TestNode', pos: [0, 0], size: [100, 100] }]
} as never)
expect(workflowStore.openWorkflow).toHaveBeenCalledWith(existingWorkflow)
expect(existingWorkflow.changeTracker.reset).toHaveBeenCalled()
expect(existingWorkflow.changeTracker.restore).toHaveBeenCalled()
expect(workflowStore.createNewTemporary).not.toHaveBeenCalled()
})
it('should create new temporary when ids differ', async () => {
existingWorkflow.changeTracker.activeState.id = 'existing-id'
const tempWorkflow = createModeTestWorkflow({
path: 'workflows/repeat (2).json'
})
vi.mocked(workflowStore.createNewTemporary).mockReturnValue(tempWorkflow)
vi.mocked(workflowStore.openWorkflow).mockResolvedValue(tempWorkflow)
await useWorkflowService().afterLoadNewGraph('repeat', {
id: 'different-id',
nodes: [{ id: 1, type: 'TestNode', pos: [0, 0], size: [100, 100] }]
} as never)
expect(workflowStore.createNewTemporary).toHaveBeenCalled()
})
})
describe('per-workflow mode switching', () => {
let appMode: ReturnType<typeof useAppMode>
let workflowStore: ReturnType<typeof useWorkflowStore>
let service: ReturnType<typeof useWorkflowService>
function mockOpenWorkflow() {
vi.spyOn(workflowStore, 'openWorkflow').mockImplementation(async (wf) => {
// Simulate load() setting changeTracker on first open
if (!wf.changeTracker) {
wf.changeTracker = createMockChangeTracker()
wf.content = '{}'
wf.originalContent = '{}'
}
const loaded = wf as LoadedComfyWorkflow
workflowStore.activeWorkflow = loaded
return loaded
})
}
beforeEach(() => {
appMode = useAppMode()
workflowStore = useWorkflowStore()
service = useWorkflowService()
})
describe('mode derivation from active workflow', () => {
it('reflects initialMode of the active workflow', () => {
const workflow = createModeTestWorkflow({ initialMode: 'app' })
workflowStore.activeWorkflow = workflow
expect(appMode.mode.value).toBe('app')
})
it('activeMode takes precedence over initialMode', () => {
const workflow = createModeTestWorkflow({
initialMode: 'app',
activeMode: 'graph'
})
workflowStore.activeWorkflow = workflow
expect(appMode.mode.value).toBe('graph')
})
it('defaults to graph when no active workflow', () => {
expect(appMode.mode.value).toBe('graph')
})
it('updates when activeWorkflow changes', () => {
const workflow1 = createModeTestWorkflow({
path: 'workflows/one.json',
initialMode: 'app'
})
const workflow2 = createModeTestWorkflow({
path: 'workflows/two.json',
activeMode: 'builder:inputs'
})
workflowStore.activeWorkflow = workflow1
expect(appMode.mode.value).toBe('app')
workflowStore.activeWorkflow = workflow2
expect(appMode.mode.value).toBe('builder:inputs')
})
})
describe('setMode writes to active workflow', () => {
it('writes activeMode without changing initialMode', () => {
const workflow = createModeTestWorkflow({ initialMode: 'graph' })
workflowStore.activeWorkflow = workflow
appMode.setMode('builder:arrange')
expect(workflow.activeMode).toBe('builder:arrange')
expect(workflow.initialMode).toBe('graph')
expect(appMode.mode.value).toBe('builder:arrange')
})
})
describe('afterLoadNewGraph initializes initialMode', () => {
beforeEach(() => {
mockOpenWorkflow()
})
it('sets initialMode from extra.linearMode on first load', async () => {
const workflow = createModeTestWorkflow({ loaded: false })
await service.afterLoadNewGraph(
workflow,
makeWorkflowData({ linearMode: true })
)
expect(workflow.initialMode).toBe('app')
})
it('leaves initialMode null when extra.linearMode is absent', async () => {
const workflow = createModeTestWorkflow({ loaded: false })
await service.afterLoadNewGraph(workflow, makeWorkflowData())
expect(workflow.initialMode).toBeNull()
})
it('sets initialMode to graph when extra.linearMode is false', async () => {
const workflow = createModeTestWorkflow({ loaded: false })
await service.afterLoadNewGraph(
workflow,
makeWorkflowData({ linearMode: false })
)
expect(workflow.initialMode).toBe('graph')
})
it('does not set initialMode on tab switch even if data has linearMode', async () => {
const workflow = createModeTestWorkflow({ loaded: false })
// First load — no linearMode in data
await service.afterLoadNewGraph(workflow, makeWorkflowData())
expect(workflow.initialMode).toBeNull()
// User switches to app mode at runtime
workflow.activeMode = 'app'
// Tab switch / reload — data now has linearMode (leaked from graph)
await service.afterLoadNewGraph(
workflow,
makeWorkflowData({ linearMode: true })
)
// initialMode should NOT have been updated — only builder save sets it
expect(workflow.initialMode).toBeNull()
})
it('preserves existing initialMode on tab switch', async () => {
const workflow = createModeTestWorkflow({
initialMode: 'app'
})
await service.afterLoadNewGraph(workflow, makeWorkflowData())
expect(workflow.initialMode).toBe('app')
})
it('sets initialMode to app for fresh string-based loads with linearMode', async () => {
vi.spyOn(workflowStore, 'createNewTemporary').mockReturnValue(
createModeTestWorkflow()
)
await service.afterLoadNewGraph(
'test.json',
makeWorkflowData({ linearMode: true })
)
expect(appMode.mode.value).toBe('app')
})
it('reads initialMode from file when draft lacks linearMode (restoration)', async () => {
const filePath = 'workflows/saved-app.json'
const fileInitialState = makeWorkflowData({ linearMode: true })
const mockTracker = createMockChangeTracker()
mockTracker.initialState = fileInitialState
// Persisted, not-loaded workflow in the store
const persistedWorkflow = new ComfyWorkflowClass({
path: filePath,
modified: Date.now(),
size: 100
})
vi.spyOn(workflowStore, 'getWorkflowByPath').mockReturnValue(
persistedWorkflow
)
vi.spyOn(workflowStore, 'openWorkflow').mockImplementation(
async (wf) => {
wf.changeTracker = mockTracker
wf.content = JSON.stringify(fileInitialState)
wf.originalContent = wf.content
workflowStore.activeWorkflow = wf as LoadedComfyWorkflow
return wf as LoadedComfyWorkflow
}
)
// Draft data has NO linearMode (simulates rootGraph serialization)
const draftData = makeWorkflowData()
await service.afterLoadNewGraph('saved-app.json', draftData)
// initialMode should come from the file, not the draft
expect(persistedWorkflow.initialMode).toBe('app')
})
})
describe('round-trip mode preservation', () => {
it('each workflow retains its own mode across tab switches', () => {
const workflow1 = createModeTestWorkflow({
path: 'workflows/one.json',
activeMode: 'builder:inputs'
})
const workflow2 = createModeTestWorkflow({
path: 'workflows/two.json',
initialMode: 'app'
})
workflowStore.activeWorkflow = workflow1
expect(appMode.mode.value).toBe('builder:inputs')
workflowStore.activeWorkflow = workflow2
expect(appMode.mode.value).toBe('app')
workflowStore.activeWorkflow = workflow1
expect(appMode.mode.value).toBe('builder:inputs')
})
})
})
describe('saveWorkflowAs', () => {
let workflowStore: ReturnType<typeof useWorkflowStore>
let service: ReturnType<typeof useWorkflowService>
beforeEach(() => {
workflowStore = useWorkflowStore()
service = useWorkflowService()
vi.spyOn(workflowStore, 'saveWorkflow').mockResolvedValue()
vi.spyOn(workflowStore, 'renameWorkflow').mockResolvedValue()
})
function createTemporaryWorkflow(
directory: string = 'workflows'
): LoadedComfyWorkflow {
const workflow = new ComfyWorkflowClass({
path: directory + '/temp.json',
modified: Date.now(),
size: 100
})
workflow.changeTracker = createMockChangeTracker()
workflow.content = '{}'
workflow.originalContent = '{}'
Object.defineProperty(workflow, 'isTemporary', { get: () => true })
return workflow as LoadedComfyWorkflow
}
it('appends .app.json extension when initialMode is app', async () => {
const workflow = createTemporaryWorkflow()
workflow.initialMode = 'app'
await service.saveWorkflowAs(workflow, { filename: 'my-workflow' })
expect(workflowStore.renameWorkflow).toHaveBeenCalledWith(
workflow,
'workflows/my-workflow.app.json'
)
})
it('appends .json extension when initialMode is graph', async () => {
const workflow = createTemporaryWorkflow()
workflow.initialMode = 'graph'
await service.saveWorkflowAs(workflow, { filename: 'my-workflow' })
expect(workflowStore.renameWorkflow).toHaveBeenCalledWith(
workflow,
'workflows/my-workflow.json'
)
})
it('appends .json extension when initialMode is not set', async () => {
const workflow = createTemporaryWorkflow()
await service.saveWorkflowAs(workflow, { filename: 'my-workflow' })
expect(workflowStore.renameWorkflow).toHaveBeenCalledWith(
workflow,
'workflows/my-workflow.json'
)
})
})
describe('saveWorkflow', () => {
let workflowStore: ReturnType<typeof useWorkflowStore>
let toastStore: ReturnType<typeof useToastStore>
let service: ReturnType<typeof useWorkflowService>
beforeEach(() => {
workflowStore = useWorkflowStore()
toastStore = useToastStore()
service = useWorkflowService()
vi.spyOn(workflowStore, 'saveWorkflow').mockResolvedValue()
vi.spyOn(workflowStore, 'renameWorkflow').mockResolvedValue()
})
function createSaveableWorkflow(path: string): LoadedComfyWorkflow {
const workflow = new ComfyWorkflowClass({
path,
modified: Date.now(),
size: 100
})
workflow.changeTracker = createMockChangeTracker()
workflow.content = '{}'
workflow.originalContent = '{}'
return workflow as LoadedComfyWorkflow
}
it('renames .json to .app.json when initialMode is app', async () => {
const workflow = createSaveableWorkflow('workflows/test.json')
workflow.initialMode = 'app'
await service.saveWorkflow(workflow)
expect(workflowStore.renameWorkflow).toHaveBeenCalledWith(
workflow,
'workflows/test.app.json'
)
expect(workflowStore.saveWorkflow).toHaveBeenCalledWith(workflow)
})
it('renames .app.json to .json when initialMode is graph', async () => {
const workflow = createSaveableWorkflow('workflows/test.app.json')
workflow.initialMode = 'graph'
await service.saveWorkflow(workflow)
expect(workflowStore.renameWorkflow).toHaveBeenCalledWith(
workflow,
'workflows/test.json'
)
expect(workflowStore.saveWorkflow).toHaveBeenCalledWith(workflow)
})
it('does not rename when extension already matches', async () => {
const workflow = createSaveableWorkflow('workflows/test.app.json')
workflow.initialMode = 'app'
await service.saveWorkflow(workflow)
expect(workflowStore.renameWorkflow).not.toHaveBeenCalled()
expect(workflowStore.saveWorkflow).toHaveBeenCalledWith(workflow)
})
it('shows toast only when rename occurs', async () => {
const addSpy = vi.spyOn(toastStore, 'add')
const workflow = createSaveableWorkflow('workflows/test.json')
workflow.initialMode = 'app'
await service.saveWorkflow(workflow)
expect(addSpy).toHaveBeenCalledWith(
expect.objectContaining({ severity: 'info' })
)
})
it('does not show toast when no rename occurs', async () => {
const addSpy = vi.spyOn(toastStore, 'add')
const workflow = createSaveableWorkflow('workflows/test.app.json')
workflow.initialMode = 'app'
await service.saveWorkflow(workflow)
expect(addSpy).not.toHaveBeenCalled()
})
it('does not rename when initialMode is not set', async () => {
const workflow = createSaveableWorkflow('workflows/test.json')
await service.saveWorkflow(workflow)
expect(workflowStore.renameWorkflow).not.toHaveBeenCalled()
})
it('prompts for overwrite when target path already exists', async () => {
const workflow = createSaveableWorkflow('workflows/test.json')
workflow.initialMode = 'app'
const existing = createSaveableWorkflow('workflows/test.app.json')
vi.spyOn(workflowStore, 'getWorkflowByPath').mockReturnValue(existing)
vi.spyOn(workflowStore, 'deleteWorkflow').mockResolvedValue()
mockConfirm.mockResolvedValue(true)
await service.saveWorkflow(workflow)
expect(mockConfirm).toHaveBeenCalled()
expect(workflowStore.renameWorkflow).toHaveBeenCalledWith(
workflow,
'workflows/test.app.json'
)
expect(workflowStore.saveWorkflow).toHaveBeenCalledWith(workflow)
})
it('saves without renaming when user declines overwrite', async () => {
const workflow = createSaveableWorkflow('workflows/test.json')
workflow.initialMode = 'app'
const existing = createSaveableWorkflow('workflows/test.app.json')
vi.spyOn(workflowStore, 'getWorkflowByPath').mockReturnValue(existing)
mockConfirm.mockResolvedValue(false)
await service.saveWorkflow(workflow)
expect(mockConfirm).toHaveBeenCalled()
expect(workflowStore.renameWorkflow).not.toHaveBeenCalled()
expect(workflowStore.saveWorkflow).toHaveBeenCalledWith(workflow)
})
})
})

View File

@@ -22,7 +22,16 @@ import { useMissingNodesDialog } from '@/composables/useMissingNodesDialog'
import { useDialogService } from '@/services/dialogService'
import { useDomWidgetStore } from '@/stores/domWidgetStore'
import { useWorkspaceStore } from '@/stores/workspaceStore'
import { appendJsonExt } from '@/utils/formatUtil'
import {
appendJsonExt,
appendWorkflowJsonExt,
generateUUID
} from '@/utils/formatUtil'
function linearModeToAppMode(linearMode: unknown): AppMode | null {
if (typeof linearMode !== 'boolean') return null
return linearMode ? 'app' : 'graph'
}
export const useWorkflowService = () => {
const settingStore = useSettingStore()
@@ -361,10 +370,24 @@ export const useWorkflowService = () => {
const fullPath = ComfyWorkflow.basePath + appendJsonExt(path)
const existingWorkflow = workflowStore.getWorkflowByPath(fullPath)
// If the workflow exists and is NOT loaded yet (restoration case),
// use the existing workflow instead of creating a new one.
// If it IS loaded, this is a re-import case - create new with suffix.
if (existingWorkflow?.isPersisted && !existingWorkflow.isLoaded) {
// Reuse an existing workflow when this is a restoration case
// (persisted but currently unloaded) or an idempotent repeated load
// of the currently active same-path workflow.
//
// This prevents accidental duplicate tabs when startup/load flows
// invoke loadGraphData more than once for the same workflow name.
const isSameActiveWorkflowLoad =
!!existingWorkflow &&
workflowStore.isActive(existingWorkflow) &&
(existingWorkflow.activeState?.id === undefined ||
workflowData.id === undefined ||
existingWorkflow.activeState.id === workflowData.id)
if (
existingWorkflow &&
((existingWorkflow.isPersisted && !existingWorkflow.isLoaded) ||
isSameActiveWorkflowLoad)
) {
const loadedWorkflow =
await workflowStore.openWorkflow(existingWorkflow)
loadedWorkflow.changeTracker.reset(workflowData)
@@ -435,7 +458,10 @@ export const useWorkflowService = () => {
* Takes an existing workflow and duplicates it with a new name
*/
const duplicateWorkflow = async (workflow: ComfyWorkflow) => {
if (!workflow.isLoaded) await workflow.load()
const state = JSON.parse(JSON.stringify(workflow.activeState))
// Ensure duplicates are always treated as distinct workflows.
if (state) state.id = generateUUID()
const suffix = workflow.isPersisted ? ' (Copy)' : ''
// Remove the suffix `(2)` or similar
const filename = workflow.filename.replace(/\s*\(\d+\)$/, '') + suffix

View File

@@ -478,12 +478,15 @@ export const useWorkflowStore = defineStore('workflow', () => {
const wasBookmarked = bookmarkStore.isBookmarked(oldPath)
const draftStore = useWorkflowDraftStore()
const openIndex = detachWorkflow(workflow)
// Perform the actual rename operation first
try {
await workflow.rename(newPath)
} finally {
attachWorkflow(workflow, openIndex)
await workflow.rename(newPath)
// Synchronously swap old path for new path in lookup and open paths
// to avoid a tab flicker caused by an async gap between detach/attach.
delete workflowLookup.value[oldPath]
workflowLookup.value[workflow.path] = workflow
const openIndex = openWorkflowPaths.value.indexOf(oldPath)
if (openIndex !== -1) {
openWorkflowPaths.value.splice(openIndex, 1, workflow.path)
}
draftStore.moveDraft(oldPath, newPath, workflow.key)
@@ -524,13 +527,11 @@ export const useWorkflowStore = defineStore('workflow', () => {
const saveWorkflow = async (workflow: ComfyWorkflow) => {
isBusy.value = true
try {
// Detach the workflow and re-attach to force refresh the tree objects.
await workflow.save()
// Synchronously detach and re-attach to force refresh the tree objects
// without an async gap that would cause the tab to disappear.
const openIndex = detachWorkflow(workflow)
try {
await workflow.save()
} finally {
attachWorkflow(workflow, openIndex)
}
attachWorkflow(workflow, openIndex)
} finally {
isBusy.value = false
}