diff --git a/browser_tests/fixtures/utils/builderTestUtils.ts b/browser_tests/fixtures/utils/builderTestUtils.ts index 720b0685e7..b45978cd5f 100644 --- a/browser_tests/fixtures/utils/builderTestUtils.ts +++ b/browser_tests/fixtures/utils/builderTestUtils.ts @@ -122,3 +122,19 @@ export async function saveAndReopenInAppMode( await comfyPage.appMode.toggleAppMode() } + +export async function saveCloseAndReopenInBuilder( + comfyPage: ComfyPage, + appMode: AppModeHelper, + workflowName: string +) { + await appMode.steps.goToPreview() + await builderSaveAs(appMode, workflowName) + await appMode.saveAs.closeButton.click() + await expect(appMode.saveAs.successDialog).toBeHidden() + + await appMode.footer.exitBuilder() + await openWorkflowFromSidebar(comfyPage, workflowName) + await appMode.enterBuilder() + await appMode.steps.goToInputs() +} diff --git a/browser_tests/tests/appModeInputPersistence.spec.ts b/browser_tests/tests/appModeInputPersistence.spec.ts new file mode 100644 index 0000000000..e5d81aa6bf --- /dev/null +++ b/browser_tests/tests/appModeInputPersistence.spec.ts @@ -0,0 +1,44 @@ +import { + comfyPageFixture as test, + comfyExpect as expect +} from '@e2e/fixtures/ComfyPage' +import { + saveCloseAndReopenInBuilder, + setupBuilder +} from '@e2e/fixtures/utils/builderTestUtils' + +const WIDGETS = ['seed', 'steps', 'cfg'] + +test.describe( + 'App builder input persistence after reload', + { tag: '@ui' }, + () => { + test.beforeEach(async ({ comfyPage }) => { + await comfyPage.appMode.enableLinearMode() + await comfyPage.settings.setSetting( + 'Comfy.AppBuilder.VueNodeSwitchDismissed', + true + ) + }) + + test('persists selected inputs after save and reopen without visibility errors', async ({ + comfyPage + }) => { + const { appMode } = comfyPage + await setupBuilder(comfyPage, undefined, WIDGETS) + + await appMode.steps.goToInputs() + await expect(appMode.select.inputItemTitles).toHaveText(WIDGETS) + + const workflowName = `${Date.now()} input-persistence` + await saveCloseAndReopenInBuilder(comfyPage, appMode, workflowName) + + await expect(appMode.select.inputItemTitles).toHaveText(WIDGETS) + for (const widget of WIDGETS) { + await expect( + appMode.select.getInputItemSubtitle(widget) + ).not.toContainText('Widget not visible') + } + }) + } +) diff --git a/browser_tests/tests/builderReorder.spec.ts b/browser_tests/tests/builderReorder.spec.ts index feb73e7e74..27d5e54cb8 100644 --- a/browser_tests/tests/builderReorder.spec.ts +++ b/browser_tests/tests/builderReorder.spec.ts @@ -5,26 +5,18 @@ import { import type { AppModeHelper } from '@e2e/fixtures/helpers/AppModeHelper' import type { ComfyPage } from '@e2e/fixtures/ComfyPage' import { - builderSaveAs, - openWorkflowFromSidebar, + saveCloseAndReopenInBuilder, setupBuilder } from '@e2e/fixtures/utils/builderTestUtils' const WIDGETS = ['seed', 'steps', 'cfg'] -/** Save as app, close it by loading default, reopen from sidebar, enter app mode. */ async function saveCloseAndReopenAsApp( comfyPage: ComfyPage, appMode: AppModeHelper, workflowName: string ) { - await appMode.steps.goToPreview() - await builderSaveAs(appMode, workflowName) - await appMode.saveAs.closeButton.click() - await expect(appMode.saveAs.successDialog).toBeHidden() - - await appMode.footer.exitBuilder() - await openWorkflowFromSidebar(comfyPage, workflowName) + await saveCloseAndReopenInBuilder(comfyPage, appMode, workflowName) await appMode.toggleAppMode() } diff --git a/src/stores/appModeStore.test.ts b/src/stores/appModeStore.test.ts index d3a22987fc..1d64bbaa08 100644 --- a/src/stores/appModeStore.test.ts +++ b/src/stores/appModeStore.test.ts @@ -5,11 +5,16 @@ import { nextTick } from 'vue' import { beforeEach, describe, expect, it, vi } from 'vitest' import type { LGraphNode, NodeId } from '@/lib/litegraph/src/LGraphNode' -import type { LoadedComfyWorkflow } from '@/platform/workflow/management/stores/comfyWorkflow' +import type { IBaseWidget } from '@/lib/litegraph/src/types/widgets' +import type { + InputWidgetConfig, + LinearInput, + LoadedComfyWorkflow +} from '@/platform/workflow/management/stores/comfyWorkflow' import { ComfyWorkflow as ComfyWorkflowClass } from '@/platform/workflow/management/stores/comfyWorkflow' import { useWorkflowStore } from '@/platform/workflow/management/stores/workflowStore' import { app } from '@/scripts/app' -import type { ChangeTracker } from '@/scripts/changeTracker' +import { ChangeTracker } from '@/scripts/changeTracker' import { createMockChangeTracker } from '@/utils/__tests__/litegraphTestUtils' const mockEmptyWorkflowDialog = vi.hoisted(() => { @@ -100,6 +105,29 @@ function createBuilderWorkflowWithOutputs( return workflow } +function createWorkflowWithLinearData( + activeMode: string, + inputs: LinearInput[], + outputs: NodeId[] +): LoadedComfyWorkflow { + const workflow = createBuilderWorkflow(activeMode) + workflow.changeTracker = createMockChangeTracker( + fromPartial>({ + activeState: { + last_node_id: 0, + last_link_id: 0, + nodes: [], + links: [], + groups: [], + config: {}, + version: 0.4, + extra: { linearData: fromAny({ inputs, outputs }) } + } + }) + ) + return workflow +} + describe('appModeStore', () => { let workflowStore: ReturnType let store: ReturnType @@ -107,6 +135,7 @@ describe('appModeStore', () => { beforeEach(() => { setActivePinia(createTestingPinia({ stubActions: false })) vi.mocked(app.rootGraph).extra = {} + ChangeTracker.isLoadingGraph = false mockResolveNode.mockReturnValue(undefined) mockSettings.reset() vi.mocked(app.rootGraph).nodes = [{ id: 1 } as LGraphNode] @@ -163,6 +192,28 @@ describe('appModeStore', () => { ) expect(workflowStore.activeWorkflow!.activeMode).toBe('graph') }) + + it('prunes selections from workflow state on entry', () => { + const node1 = { id: 1 } + mockResolveNode.mockImplementation((id) => + id == 1 ? fromAny(node1) : undefined + ) + workflowStore.activeWorkflow = createWorkflowWithLinearData( + 'graph', + [ + [1, 'seed'], + [99, 'steps'] + ], + [1, 99] + ) + store.selectedInputs = [[42, 'prompt']] + store.selectedOutputs = [42] + + store.enterBuilder() + + expect(store.selectedInputs).toEqual([[1, 'seed']]) + expect(store.selectedOutputs).toEqual([1]) + }) }) describe('empty workflow dialog callbacks', () => { @@ -202,33 +253,36 @@ describe('appModeStore', () => { }) }) + describe('exitBuilder', () => { + it('prunes selections from workflow state on exit', () => { + const node1 = { id: 1 } + mockResolveNode.mockImplementation((id) => + id == 1 ? fromAny(node1) : undefined + ) + workflowStore.activeWorkflow = createWorkflowWithLinearData( + 'builder:inputs', + [ + [1, 'seed'], + [99, 'steps'] + ], + [1, 99] + ) + store.selectedInputs = [[42, 'prompt']] + store.selectedOutputs = [42] + + store.exitBuilder() + + expect(store.selectedInputs).toEqual([[1, 'seed']]) + expect(store.selectedOutputs).toEqual([1]) + expect(workflowStore.activeWorkflow!.activeMode).toBe('graph') + }) + }) + describe('loadSelections pruning', () => { function mockNode(id: number) { return { id } } - function workflowWithLinearData( - inputs: [number, string][], - outputs: number[] - ) { - const workflow = createBuilderWorkflow('app') - workflow.changeTracker = createMockChangeTracker( - fromPartial>({ - activeState: { - last_node_id: 0, - last_link_id: 0, - nodes: [], - links: [], - groups: [], - config: {}, - version: 0.4, - extra: { linearData: { inputs, outputs } } - } - }) - ) - return workflow - } - it('removes inputs referencing deleted nodes on load', async () => { const node1 = mockNode(1) mockResolveNode.mockImplementation((id) => @@ -294,7 +348,11 @@ describe('appModeStore', () => { // Initially nodes are not resolvable — pruning removes them mockResolveNode.mockReturnValue(undefined) const inputs: [number, string][] = [[1, 'seed']] - workflowStore.activeWorkflow = workflowWithLinearData(inputs, [1]) + workflowStore.activeWorkflow = createWorkflowWithLinearData( + 'app', + inputs, + [1] + ) store.loadSelections({ inputs }) await nextTick() @@ -324,6 +382,141 @@ describe('appModeStore', () => { }) }) + describe('loadSelections edge cases', () => { + it('clears existing selections on undefined or empty data', () => { + store.selectedInputs = [[1, 'seed']] + store.selectedOutputs = [1] + + store.loadSelections(undefined) + + expect(store.selectedInputs).toEqual([]) + expect(store.selectedOutputs).toEqual([]) + + store.selectedInputs = [[1, 'seed']] + store.selectedOutputs = [1] + + store.loadSelections({}) + + expect(store.selectedInputs).toEqual([]) + expect(store.selectedOutputs).toEqual([]) + }) + }) + + describe('pruneLinearData', () => { + it('returns empty selections for undefined data', () => { + expect(store.pruneLinearData(undefined)).toEqual({ + inputs: [], + outputs: [] + }) + }) + + it('does not prune when rootGraph is empty', () => { + const originalRootGraph = app.rootGraph + Object.defineProperty(app, 'rootGraph', { value: null, writable: true }) + + try { + expect( + store.pruneLinearData({ + inputs: [[1, 'seed']], + outputs: [1] + }) + ).toEqual({ + inputs: [[1, 'seed']], + outputs: [1] + }) + } finally { + Object.defineProperty(app, 'rootGraph', { + value: originalRootGraph, + writable: true + }) + } + }) + }) + + describe('pruneLinearData during graph loading', () => { + it('preserves all entries when ChangeTracker.isLoadingGraph is true', () => { + ChangeTracker.isLoadingGraph = true + + store.loadSelections({ + inputs: [ + [1, 'seed'], + [999, 'steps'] + ], + outputs: [1, 999] + }) + + expect(store.selectedInputs).toEqual([ + [1, 'seed'], + [999, 'steps'] + ]) + expect(store.selectedOutputs).toEqual([1, 999]) + }) + + it('prunes entries for deleted nodes when not loading', () => { + const node1 = { id: 1 } + mockResolveNode.mockImplementation((id) => + id == 1 ? fromAny(node1) : undefined + ) + + store.loadSelections({ + inputs: [ + [1, 'seed'], + [999, 'steps'] + ], + outputs: [1, 999] + }) + + expect(store.selectedInputs).toEqual([[1, 'seed']]) + expect(store.selectedOutputs).toEqual([1]) + }) + }) + + describe('resetSelectedToWorkflow fallback', () => { + it('falls back to initialState when activeState has no linearData', () => { + const node1 = { id: 1 } + mockResolveNode.mockImplementation((id) => + id == 1 ? fromAny(node1) : undefined + ) + const workflow = createBuilderWorkflow('app') + workflow.changeTracker.activeState.extra = {} + workflow.changeTracker.initialState = fromAny({ + ...workflow.changeTracker.activeState, + extra: { + linearData: { inputs: [[1, 'seed']], outputs: [1] } + } + }) + workflowStore.activeWorkflow = workflow + + store.resetSelectedToWorkflow() + + expect(store.selectedInputs).toEqual([[1, 'seed']]) + expect(store.selectedOutputs).toEqual([1]) + }) + + it('prefers activeState linearData when available', () => { + const node1 = { id: 1 } + mockResolveNode.mockImplementation((id) => + id == 1 ? fromAny(node1) : undefined + ) + const workflow = createBuilderWorkflow('app') + workflow.changeTracker.activeState.extra = { + linearData: { inputs: [[1, 'steps']], outputs: [1] } + } + workflow.changeTracker.initialState = fromAny({ + ...workflow.changeTracker.activeState, + extra: { + linearData: { inputs: [[1, 'seed']], outputs: [1] } + } + }) + workflowStore.activeWorkflow = workflow + + store.resetSelectedToWorkflow() + + expect(store.selectedInputs).toEqual([[1, 'steps']]) + expect(store.selectedOutputs).toEqual([1]) + }) + }) + describe('linearData sync watcher', () => { it('writes linearData to rootGraph.extra when in builder mode', async () => { workflowStore.activeWorkflow = createBuilderWorkflow() @@ -353,15 +546,22 @@ describe('appModeStore', () => { await nextTick() const originalRootGraph = app.rootGraph + const dataBefore = JSON.parse( + JSON.stringify(originalRootGraph.extra.linearData) + ) Object.defineProperty(app, 'rootGraph', { value: null, writable: true }) - store.selectedOutputs.push(1) - await nextTick() + try { + store.selectedOutputs.push(1) + await nextTick() + } finally { + Object.defineProperty(app, 'rootGraph', { + value: originalRootGraph, + writable: true + }) + } - Object.defineProperty(app, 'rootGraph', { - value: originalRootGraph, - writable: true - }) + expect(originalRootGraph.extra.linearData).toEqual(dataBefore) }) it('calls captureCanvasState when input is selected', async () => { @@ -428,6 +628,18 @@ describe('appModeStore', () => { expect(store.selectedInputs[0][2]).toEqual({ height: 200 }) }) + it('merges existing config with new values', () => { + const existingConfig: InputWidgetConfig & { width: number } = { + height: 120, + width: 240 + } + store.selectedInputs.push([1, 'prompt', existingConfig]) + + store.updateInputConfig(1 as NodeId, 'prompt', { height: 300 }) + + expect(store.selectedInputs[0][2]).toEqual({ height: 300, width: 240 }) + }) + it('triggers linearData sync watcher', async () => { workflowStore.activeWorkflow = createBuilderWorkflow() store.selectedInputs.push([42, 'prompt']) @@ -443,6 +655,17 @@ describe('appModeStore', () => { }) }) + describe('removeSelectedInput', () => { + it('removes the matching input entry only', () => { + store.selectedInputs.push([1, 'prompt']) + store.selectedInputs.push([2, 'steps']) + + store.removeSelectedInput({ name: 'steps' } as IBaseWidget, { id: 2 }) + + expect(store.selectedInputs).toEqual([[1, 'prompt']]) + }) + }) + describe('autoEnableVueNodes', () => { it('enables Vue nodes when entering select mode with them disabled', async () => { mockSettings.store['Comfy.VueNodes.Enabled'] = false diff --git a/src/stores/appModeStore.ts b/src/stores/appModeStore.ts index 56a648d1f7..307049246f 100644 --- a/src/stores/appModeStore.ts +++ b/src/stores/appModeStore.ts @@ -49,14 +49,13 @@ export const useAppModeStore = defineStore('appMode', () => { function pruneLinearData(data: Partial | undefined): LinearData { const rawInputs = data?.inputs ?? [] const rawOutputs = data?.outputs ?? [] + if (!app.rootGraph || ChangeTracker.isLoadingGraph) { + return { inputs: rawInputs, outputs: rawOutputs } + } return { - inputs: app.rootGraph - ? rawInputs.filter(([nodeId]) => resolveNode(nodeId)) - : rawInputs, - outputs: app.rootGraph - ? rawOutputs.filter((nodeId) => resolveNode(nodeId)) - : rawOutputs + inputs: rawInputs.filter(([nodeId]) => resolveNode(nodeId)), + outputs: rawOutputs.filter((nodeId) => resolveNode(nodeId)) } } @@ -70,7 +69,10 @@ export const useAppModeStore = defineStore('appMode', () => { const { activeWorkflow } = workflowStore if (!activeWorkflow) return - loadSelections(activeWorkflow.changeTracker?.activeState?.extra?.linearData) + const source = + activeWorkflow.changeTracker?.activeState?.extra?.linearData ?? + activeWorkflow.initialState?.extra?.linearData + loadSelections(source) } useEventListener(