From c766f9624c1dfd5282dd00ea2ea150e8520e520c Mon Sep 17 00:00:00 2001 From: Christian Byrne Date: Mon, 6 Apr 2026 15:09:05 -0700 Subject: [PATCH] [backport cloud/1.42] fix: App mode - Save as not persisting mode on change (#10679) (#10913) Backport of #10679. Conflicts resolved same as core/1.42. Co-authored-by: pythongosssss <125205205+pythongosssss@users.noreply.github.com> --- browser_tests/tests/builderSaveFlow.spec.ts | 262 ++++++++++++++ src/components/builder/useBuilderSave.test.ts | 341 ++++++++++++++++++ src/components/builder/useBuilderSave.ts | 135 +++++++ src/composables/useAppMode.ts | 2 - .../core/services/workflowService.test.ts | 298 ++++++++++++--- .../workflow/core/services/workflowService.ts | 30 +- .../validation/schemas/workflowSchema.ts | 1 + 7 files changed, 1013 insertions(+), 56 deletions(-) create mode 100644 browser_tests/tests/builderSaveFlow.spec.ts create mode 100644 src/components/builder/useBuilderSave.test.ts create mode 100644 src/components/builder/useBuilderSave.ts diff --git a/browser_tests/tests/builderSaveFlow.spec.ts b/browser_tests/tests/builderSaveFlow.spec.ts new file mode 100644 index 0000000000..6bee2a2913 --- /dev/null +++ b/browser_tests/tests/builderSaveFlow.spec.ts @@ -0,0 +1,262 @@ +import { + comfyPageFixture as test, + comfyExpect as expect +} from '../fixtures/ComfyPage' +import { setupSubgraphBuilder } from '../helpers/builderTestUtils' +import { fitToViewInstant } from '../helpers/fitToView' + +test.describe('Builder save flow', { tag: ['@ui', '@subgraph'] }, () => { + test.beforeEach(async ({ comfyPage }) => { + await comfyPage.page.evaluate(() => { + window.app!.api.serverFeatureFlags.value = { + ...window.app!.api.serverFeatureFlags.value, + linear_toggle_enabled: true + } + }) + await comfyPage.settings.setSetting('Comfy.UseNewMenu', 'Top') + await comfyPage.settings.setSetting( + 'Comfy.AppBuilder.VueNodeSwitchDismissed', + true + ) + }) + + test('Save as dialog appears for unsaved workflow', async ({ comfyPage }) => { + const { page, appMode } = comfyPage + await setupSubgraphBuilder(comfyPage) + await appMode.goToPreview() + await appMode.clickSave() + + // The save-as dialog should appear with filename input and view type selection + const dialog = page.getByRole('dialog') + await expect(dialog).toBeVisible({ timeout: 5000 }) + await expect(dialog.getByRole('textbox')).toBeVisible() + await expect(dialog.getByText('Save as')).toBeVisible() + + // View type radio group should be present + const radioGroup = dialog.getByRole('radiogroup') + await expect(radioGroup).toBeVisible() + }) + + test('Save as dialog allows entering filename and saving', async ({ + comfyPage + }) => { + const { page, appMode } = comfyPage + await setupSubgraphBuilder(comfyPage) + await appMode.goToPreview() + await appMode.clickSave() + + const dialog = page.getByRole('dialog') + await expect(dialog).toBeVisible({ timeout: 5000 }) + + const workflowName = `${Date.now()} builder-save-test` + const input = dialog.getByRole('textbox') + await input.fill(workflowName) + + // Save button should be enabled now + const saveButton = dialog.getByRole('button', { name: 'Save' }) + await expect(saveButton).toBeEnabled() + await saveButton.click() + + // Success dialog should appear + const successDialog = page.getByRole('dialog') + await expect(successDialog.getByText('Successfully saved')).toBeVisible({ + timeout: 5000 + }) + }) + + test('Save as dialog disables save when filename is empty', async ({ + comfyPage + }) => { + const { page, appMode } = comfyPage + await setupSubgraphBuilder(comfyPage) + await appMode.goToPreview() + await appMode.clickSave() + + const dialog = page.getByRole('dialog') + await expect(dialog).toBeVisible({ timeout: 5000 }) + + // Clear the filename input + const input = dialog.getByRole('textbox') + await input.fill('') + + // Save button should be disabled + const saveButton = dialog.getByRole('button', { name: 'Save' }) + await expect(saveButton).toBeDisabled() + }) + + test('Builder step navigation works correctly', async ({ comfyPage }) => { + const { appMode } = comfyPage + await setupSubgraphBuilder(comfyPage) + + // Should start at outputs (we ended there in setup) + // Navigate to inputs + await appMode.goToInputs() + + // Back button should be disabled on first step + const backButton = appMode.getFooterButton('Back') + await expect(backButton).toBeDisabled() + + // Next button should be enabled + const nextButton = appMode.getFooterButton('Next') + await expect(nextButton).toBeEnabled() + + // Navigate forward + await appMode.next() + + // Back button should now be enabled + await expect(backButton).toBeEnabled() + + // Navigate to preview (last step) + await appMode.next() + + // Next button should be disabled on last step + await expect(nextButton).toBeDisabled() + }) + + test('Escape key exits builder mode', async ({ comfyPage }) => { + const { page } = comfyPage + await setupSubgraphBuilder(comfyPage) + + // Verify builder toolbar is visible + const toolbar = page.getByRole('navigation', { name: 'App Builder' }) + await expect(toolbar).toBeVisible() + + // Press Escape + await page.keyboard.press('Escape') + await comfyPage.nextFrame() + + // Builder toolbar should be gone + await expect(toolbar).not.toBeVisible() + }) + + test('Exit builder button exits builder mode', async ({ comfyPage }) => { + const { page, appMode } = comfyPage + await setupSubgraphBuilder(comfyPage) + + const toolbar = page.getByRole('navigation', { name: 'App Builder' }) + await expect(toolbar).toBeVisible() + + await appMode.exitBuilder() + + await expect(toolbar).not.toBeVisible() + }) + + test('Save button directly saves for previously saved workflow', async ({ + comfyPage + }) => { + const { page, appMode } = comfyPage + await setupSubgraphBuilder(comfyPage) + await appMode.goToPreview() + + // First save via builder save-as to make it non-temporary + await appMode.clickSave() + const saveAsDialog = page.getByRole('dialog') + await expect(saveAsDialog).toBeVisible({ timeout: 5000 }) + const workflowName = `${Date.now()} builder-direct-save` + await saveAsDialog.getByRole('textbox').fill(workflowName) + await saveAsDialog.getByRole('button', { name: 'Save' }).click() + + // Dismiss the success dialog + const successDialog = page.getByRole('dialog') + await expect(successDialog.getByText('Successfully saved')).toBeVisible({ + timeout: 5000 + }) + await successDialog.getByText('Close', { exact: true }).click() + await comfyPage.nextFrame() + + // Modify the workflow so the save button becomes enabled + await appMode.goToInputs() + const seedMenu = appMode.getBuilderInputItemMenu('seed') + await seedMenu.click() + await page.getByText('Delete', { exact: true }).click() + await comfyPage.nextFrame() + await appMode.goToPreview() + await expect(appMode.getFooterButton(/^Save$/)).toBeEnabled({ + timeout: 5000 + }) + + // Now click save — should save directly without dialog + await appMode.clickSave() + + await expect(page.getByRole('dialog')).not.toBeVisible({ timeout: 2000 }) + await expect(appMode.getFooterButton(/^Save$/)).toBeDisabled() + }) + + test('Split button chevron opens save-as for saved workflow', async ({ + comfyPage + }) => { + const { page, appMode } = comfyPage + await setupSubgraphBuilder(comfyPage) + await appMode.goToPreview() + + // First save via builder save-as to make it non-temporary + await appMode.clickSave() + const saveAsDialog = page.getByRole('dialog') + await expect(saveAsDialog).toBeVisible({ timeout: 5000 }) + const workflowName = `${Date.now()} builder-split-btn` + await saveAsDialog.getByRole('textbox').fill(workflowName) + await saveAsDialog.getByRole('button', { name: 'Save' }).click() + + // Dismiss the success dialog + const successDialog = page.getByRole('dialog') + await expect(successDialog.getByText('Successfully saved')).toBeVisible({ + timeout: 5000 + }) + await successDialog.getByText('Close', { exact: true }).click() + await comfyPage.nextFrame() + + // Click the chevron dropdown trigger + const chevronButton = appMode.getFooterButton('Save as') + await chevronButton.click() + + // "Save as" menu item should appear + const menuItem = page.getByRole('menuitem', { name: 'Save as' }) + await expect(menuItem).toBeVisible({ timeout: 5000 }) + await menuItem.click() + + // Save-as dialog should appear + const newSaveAsDialog = page.getByRole('dialog') + await expect(newSaveAsDialog.getByText('Save as')).toBeVisible({ + timeout: 5000 + }) + await expect(newSaveAsDialog.getByRole('textbox')).toBeVisible() + }) + + test('Connect output popover appears when no outputs selected', async ({ + comfyPage + }) => { + const { page, appMode } = comfyPage + await comfyPage.workflow.loadWorkflow('default') + await fitToViewInstant(comfyPage) + await appMode.enterBuilder() + + // Without selecting any outputs, click the save button + // It should trigger the connect-output popover + await appMode.clickSave() + + // The popover should show a message about connecting outputs + await expect( + page.getByText('Connect an output', { exact: false }) + ).toBeVisible({ timeout: 5000 }) + }) + + test('View type can be toggled in save-as dialog', async ({ comfyPage }) => { + const { page, appMode } = comfyPage + await setupSubgraphBuilder(comfyPage) + await appMode.goToPreview() + await appMode.clickSave() + + const dialog = page.getByRole('dialog') + await expect(dialog).toBeVisible({ timeout: 5000 }) + + // App should be selected by default + const appRadio = dialog.getByRole('radio', { name: /App/ }) + await expect(appRadio).toHaveAttribute('aria-checked', 'true') + + // Click Node graph option + const graphRadio = dialog.getByRole('radio', { name: /Node graph/ }) + await graphRadio.click() + await expect(graphRadio).toHaveAttribute('aria-checked', 'true') + await expect(appRadio).toHaveAttribute('aria-checked', 'false') + }) +}) diff --git a/src/components/builder/useBuilderSave.test.ts b/src/components/builder/useBuilderSave.test.ts new file mode 100644 index 0000000000..10768573e2 --- /dev/null +++ b/src/components/builder/useBuilderSave.test.ts @@ -0,0 +1,341 @@ +import { beforeEach, describe, expect, it, vi } from 'vitest' +import { ref } from 'vue' + +import { useBuilderSave } from './useBuilderSave' + +const mockSetMode = vi.hoisted(() => vi.fn()) +const mockToastErrorHandler = vi.hoisted(() => vi.fn()) +const mockTrackEnterLinear = vi.hoisted(() => vi.fn()) +const mockTrackDefaultViewSet = vi.hoisted(() => vi.fn()) +const mockSaveWorkflow = vi.hoisted(() => vi.fn<() => Promise>()) +const mockSaveWorkflowAs = vi.hoisted(() => + vi.fn<() => Promise>() +) +const mockShowLayoutDialog = vi.hoisted(() => vi.fn()) +const mockShowConfirmDialog = vi.hoisted(() => vi.fn()) +const mockCloseDialog = vi.hoisted(() => vi.fn()) +const mockExitBuilder = vi.hoisted(() => vi.fn()) + +const mockActiveWorkflow = ref<{ + filename: string + initialMode?: string | null +} | null>(null) + +vi.mock('@/composables/useAppMode', () => ({ + useAppMode: () => ({ setMode: mockSetMode }) +})) + +vi.mock('@/composables/useErrorHandling', () => ({ + useErrorHandling: () => ({ toastErrorHandler: mockToastErrorHandler }) +})) + +vi.mock('@/platform/telemetry', () => ({ + useTelemetry: () => ({ + trackEnterLinear: mockTrackEnterLinear, + trackDefaultViewSet: mockTrackDefaultViewSet + }) +})) + +vi.mock('@/platform/workflow/core/services/workflowService', () => ({ + useWorkflowService: () => ({ + saveWorkflow: mockSaveWorkflow, + saveWorkflowAs: mockSaveWorkflowAs + }) +})) + +vi.mock('@/platform/workflow/management/stores/workflowStore', () => ({ + useWorkflowStore: () => ({ + get activeWorkflow() { + return mockActiveWorkflow.value + } + }) +})) + +vi.mock('@/services/dialogService', () => ({ + useDialogService: () => ({ showLayoutDialog: mockShowLayoutDialog }) +})) + +vi.mock('@/stores/appModeStore', () => ({ + useAppModeStore: () => ({ exitBuilder: mockExitBuilder }) +})) + +vi.mock('@/stores/dialogStore', () => ({ + useDialogStore: () => ({ closeDialog: mockCloseDialog }) +})) + +vi.mock('@/components/dialog/confirm/confirmDialog', () => ({ + showConfirmDialog: mockShowConfirmDialog +})) + +vi.mock('@/i18n', () => ({ + t: (key: string, params?: Record) => { + if (params) return `${key}:${JSON.stringify(params)}` + return key + } +})) + +vi.mock('./BuilderSaveDialogContent.vue', () => ({ + default: { template: '
' } +})) + +const SAVE_DIALOG_KEY = 'builder-save' +const SUCCESS_DIALOG_KEY = 'builder-save-success' + +describe('useBuilderSave', () => { + beforeEach(() => { + vi.clearAllMocks() + mockActiveWorkflow.value = null + }) + + describe('save()', () => { + it('does nothing when there is no active workflow', async () => { + const { save } = useBuilderSave() + + await save() + + expect(mockSaveWorkflow).not.toHaveBeenCalled() + }) + + it('saves workflow directly without showing a dialog', async () => { + mockActiveWorkflow.value = { filename: 'my-workflow', initialMode: 'app' } + mockSaveWorkflow.mockResolvedValueOnce(undefined) + const { save } = useBuilderSave() + + await save() + + expect(mockSaveWorkflow).toHaveBeenCalledOnce() + expect(mockShowConfirmDialog).not.toHaveBeenCalled() + }) + + it('toasts error on failure', async () => { + mockActiveWorkflow.value = { filename: 'my-workflow', initialMode: 'app' } + const error = new Error('save failed') + mockSaveWorkflow.mockRejectedValueOnce(error) + const { save } = useBuilderSave() + + await save() + + expect(mockToastErrorHandler).toHaveBeenCalledWith(error) + expect(mockShowConfirmDialog).not.toHaveBeenCalled() + }) + + it('prevents concurrent saves', async () => { + mockActiveWorkflow.value = { filename: 'my-workflow', initialMode: 'app' } + let resolveSave!: () => void + mockSaveWorkflow.mockReturnValueOnce( + new Promise((r) => { + resolveSave = r + }) + ) + const { save, isSaving } = useBuilderSave() + + const firstSave = save() + expect(isSaving.value).toBe(true) + + await save() + expect(mockSaveWorkflow).toHaveBeenCalledOnce() + + resolveSave() + await firstSave + expect(isSaving.value).toBe(false) + }) + }) + + describe('saveAs()', () => { + it('does nothing when there is no active workflow', () => { + mockActiveWorkflow.value = null + const { saveAs } = useBuilderSave() + + saveAs() + + expect(mockShowLayoutDialog).not.toHaveBeenCalled() + }) + + it('opens save dialog with correct defaultFilename and defaultOpenAsApp', () => { + mockActiveWorkflow.value = { filename: 'my-workflow', initialMode: 'app' } + const { saveAs } = useBuilderSave() + + saveAs() + + expect(mockShowLayoutDialog).toHaveBeenCalledOnce() + const { key, props } = mockShowLayoutDialog.mock.calls[0][0] + expect(key).toBe(SAVE_DIALOG_KEY) + expect(props.defaultFilename).toBe('my-workflow') + expect(props.defaultOpenAsApp).toBe(true) + }) + + it('passes defaultOpenAsApp: false when initialMode is graph', () => { + mockActiveWorkflow.value = { + filename: 'my-workflow', + initialMode: 'graph' + } + const { saveAs } = useBuilderSave() + + saveAs() + + const { props } = mockShowLayoutDialog.mock.calls[0][0] + expect(props.defaultOpenAsApp).toBe(false) + }) + }) + + describe('save dialog callbacks', () => { + function getSaveDialogProps() { + mockActiveWorkflow.value = { filename: 'my-workflow', initialMode: 'app' } + const { saveAs } = useBuilderSave() + saveAs() + return mockShowLayoutDialog.mock.calls[0][0].props as { + onSave: (filename: string, openAsApp: boolean) => Promise + onClose: () => void + } + } + + it('onSave calls saveWorkflowAs with isApp and tracks telemetry', async () => { + mockSaveWorkflowAs.mockResolvedValueOnce(true) + const { onSave } = getSaveDialogProps() + + await onSave('new-name', true) + + expect(mockSaveWorkflowAs).toHaveBeenCalledWith( + mockActiveWorkflow.value, + { + filename: 'new-name', + isApp: true + } + ) + expect(mockTrackDefaultViewSet).toHaveBeenCalledWith({ + default_view: 'app' + }) + }) + + it('onSave passes isApp: false when saving as graph', async () => { + mockSaveWorkflowAs.mockResolvedValueOnce(true) + const { onSave } = getSaveDialogProps() + + await onSave('new-name', false) + + expect(mockSaveWorkflowAs).toHaveBeenCalledWith( + mockActiveWorkflow.value, + { + filename: 'new-name', + isApp: false + } + ) + expect(mockTrackDefaultViewSet).toHaveBeenCalledWith({ + default_view: 'graph' + }) + }) + + it('onSave does not track or close when saveWorkflowAs returns falsy', async () => { + mockSaveWorkflowAs.mockResolvedValueOnce(null) + const { onSave } = getSaveDialogProps() + + await onSave('new-name', false) + + expect(mockTrackDefaultViewSet).not.toHaveBeenCalled() + expect(mockCloseDialog).not.toHaveBeenCalled() + }) + + it('onSave closes dialog and shows success dialog after successful save', async () => { + mockSaveWorkflowAs.mockResolvedValueOnce(true) + const { onSave } = getSaveDialogProps() + + await onSave('new-name', true) + + expect(mockCloseDialog).toHaveBeenCalledWith({ key: SAVE_DIALOG_KEY }) + expect(mockShowConfirmDialog).toHaveBeenCalledOnce() + const successCall = mockShowConfirmDialog.mock.calls[0][0] + expect(successCall.key).toBe(SUCCESS_DIALOG_KEY) + }) + + it('shows app success message when openAsApp is true', async () => { + mockSaveWorkflowAs.mockResolvedValueOnce(true) + const { onSave } = getSaveDialogProps() + + await onSave('new-name', true) + + const successCall = mockShowConfirmDialog.mock.calls[0][0] + expect(successCall.props.promptText).toBe('builderSave.successBodyApp') + }) + + it('shows graph success message with exit builder button when openAsApp is false', async () => { + mockSaveWorkflowAs.mockResolvedValueOnce(true) + const { onSave } = getSaveDialogProps() + + await onSave('new-name', false) + + const successCall = mockShowConfirmDialog.mock.calls[0][0] + expect(successCall.props.promptText).toBe('builderSave.successBodyGraph') + expect(successCall.footerProps.confirmText).toBe( + 'linearMode.builder.exit' + ) + expect(successCall.footerProps.cancelText).toBe('builderToolbar.viewApp') + }) + + it('onSave toasts error and closes dialog on failure', async () => { + const error = new Error('save-as failed') + mockSaveWorkflowAs.mockRejectedValueOnce(error) + const { onSave } = getSaveDialogProps() + + await onSave('new-name', false) + + expect(mockToastErrorHandler).toHaveBeenCalledWith(error) + expect(mockCloseDialog).toHaveBeenCalledWith({ key: SAVE_DIALOG_KEY }) + }) + + it('prevents concurrent handleSaveAs calls', async () => { + let resolveSaveAs!: (v: boolean) => void + mockSaveWorkflowAs.mockReturnValueOnce( + new Promise((r) => { + resolveSaveAs = r + }) + ) + const { onSave } = getSaveDialogProps() + + const firstSave = onSave('new-name', true) + + await onSave('other-name', true) + expect(mockSaveWorkflowAs).toHaveBeenCalledOnce() + + resolveSaveAs(true) + await firstSave + }) + }) + + describe('graph success dialog callbacks', () => { + async function getGraphSuccessDialogProps() { + mockActiveWorkflow.value = { filename: 'my-workflow', initialMode: 'app' } + mockSaveWorkflowAs.mockResolvedValueOnce(true) + const { saveAs } = useBuilderSave() + saveAs() + const { onSave } = mockShowLayoutDialog.mock.calls[0][0].props as { + onSave: (filename: string, openAsApp: boolean) => Promise + } + await onSave('new-name', false) + return mockShowConfirmDialog.mock.calls[0][0].footerProps as { + onConfirm: () => void + onCancel: () => void + } + } + + it('onConfirm closes dialog and exits builder', async () => { + const { onConfirm } = await getGraphSuccessDialogProps() + + onConfirm() + + expect(mockCloseDialog).toHaveBeenCalledWith({ key: SUCCESS_DIALOG_KEY }) + expect(mockExitBuilder).toHaveBeenCalledOnce() + }) + + it('onCancel closes dialog and switches to app mode', async () => { + const { onCancel } = await getGraphSuccessDialogProps() + + onCancel() + + expect(mockCloseDialog).toHaveBeenCalledWith({ key: SUCCESS_DIALOG_KEY }) + expect(mockTrackEnterLinear).toHaveBeenCalledWith({ + source: 'app_builder' + }) + expect(mockSetMode).toHaveBeenCalledWith('app') + }) + }) +}) diff --git a/src/components/builder/useBuilderSave.ts b/src/components/builder/useBuilderSave.ts new file mode 100644 index 0000000000..d3ee33e32d --- /dev/null +++ b/src/components/builder/useBuilderSave.ts @@ -0,0 +1,135 @@ +import { useAppMode } from '@/composables/useAppMode' +import { useErrorHandling } from '@/composables/useErrorHandling' +import { showConfirmDialog } from '@/components/dialog/confirm/confirmDialog' +import { t } from '@/i18n' +import { useTelemetry } from '@/platform/telemetry' +import { useWorkflowService } from '@/platform/workflow/core/services/workflowService' +import { useWorkflowStore } from '@/platform/workflow/management/stores/workflowStore' +import { useDialogService } from '@/services/dialogService' +import { useAppModeStore } from '@/stores/appModeStore' +import { useDialogStore } from '@/stores/dialogStore' +import { ref } from 'vue' + +import BuilderSaveDialogContent from './BuilderSaveDialogContent.vue' + +const SAVE_DIALOG_KEY = 'builder-save' +const SUCCESS_DIALOG_KEY = 'builder-save-success' + +const isSaving = ref(false) + +export function useBuilderSave() { + const { toastErrorHandler } = useErrorHandling() + const { setMode } = useAppMode() + const workflowStore = useWorkflowStore() + const workflowService = useWorkflowService() + const dialogService = useDialogService() + const appModeStore = useAppModeStore() + const dialogStore = useDialogStore() + + function closeDialog(key: string) { + dialogStore.closeDialog({ key }) + } + + async function save() { + if (isSaving.value) return + const workflow = workflowStore.activeWorkflow + if (!workflow) return + + isSaving.value = true + try { + await workflowService.saveWorkflow(workflow) + } catch (e) { + toastErrorHandler(e) + } finally { + isSaving.value = false + } + } + + function saveAs() { + if (isSaving.value) return + const workflow = workflowStore.activeWorkflow + if (!workflow) return + + dialogService.showLayoutDialog({ + key: SAVE_DIALOG_KEY, + component: BuilderSaveDialogContent, + props: { + defaultFilename: workflow.filename, + defaultOpenAsApp: workflow.initialMode !== 'graph', + onSave: handleSaveAs, + onClose: () => closeDialog(SAVE_DIALOG_KEY) + } + }) + } + + async function handleSaveAs(filename: string, openAsApp: boolean) { + if (isSaving.value) return + isSaving.value = true + try { + const workflow = workflowStore.activeWorkflow + if (!workflow) return + + const saved = await workflowService.saveWorkflowAs(workflow, { + filename, + isApp: openAsApp + }) + + if (!saved) return + useTelemetry()?.trackDefaultViewSet({ + default_view: openAsApp ? 'app' : 'graph' + }) + closeDialog(SAVE_DIALOG_KEY) + showSuccessDialog(openAsApp ? 'app' : 'graph') + } catch (e) { + toastErrorHandler(e) + closeDialog(SAVE_DIALOG_KEY) + } finally { + isSaving.value = false + } + } + + function showSuccessDialog(viewType: 'app' | 'graph') { + const promptText = + viewType === 'app' + ? t('builderSave.successBodyApp') + : t('builderSave.successBodyGraph') + + showConfirmDialog({ + key: SUCCESS_DIALOG_KEY, + headerProps: { + title: t('builderSave.successTitle'), + icon: 'icon-[lucide--circle-check-big] text-green-500' + }, + props: { promptText, preserveNewlines: true }, + footerProps: + viewType === 'graph' + ? { + cancelText: t('builderToolbar.viewApp'), + confirmText: t('linearMode.builder.exit'), + confirmVariant: 'secondary' as const, + onCancel: () => { + closeDialog(SUCCESS_DIALOG_KEY) + useTelemetry()?.trackEnterLinear({ source: 'app_builder' }) + setMode('app') + }, + onConfirm: () => { + closeDialog(SUCCESS_DIALOG_KEY) + appModeStore.exitBuilder() + } + } + : { + cancelText: t('g.close'), + confirmText: t('builderToolbar.viewApp'), + confirmVariant: 'secondary' as const, + onCancel: () => closeDialog(SUCCESS_DIALOG_KEY), + onConfirm: () => { + closeDialog(SUCCESS_DIALOG_KEY) + useTelemetry()?.trackEnterLinear({ source: 'app_builder' }) + setMode('app') + } + } + }) + } + + return { save, saveAs, isSaving } +} diff --git a/src/composables/useAppMode.ts b/src/composables/useAppMode.ts index 8f9ff0d9c3..e589e7c4ef 100644 --- a/src/composables/useAppMode.ts +++ b/src/composables/useAppMode.ts @@ -37,8 +37,6 @@ export function useAppMode() { ) function setMode(newMode: AppMode) { - if (newMode === mode.value) return - const workflow = workflowStore.activeWorkflow if (workflow) workflow.activeMode = newMode } diff --git a/src/platform/workflow/core/services/workflowService.test.ts b/src/platform/workflow/core/services/workflowService.test.ts index 7e9f76b73b..b0c6d7fb61 100644 --- a/src/platform/workflow/core/services/workflowService.test.ts +++ b/src/platform/workflow/core/services/workflowService.test.ts @@ -75,7 +75,7 @@ vi.mock('@/services/dialogService', () => ({ vi.mock('@/scripts/app', () => ({ app: { canvas: { ds: { offset: [0, 0], scale: 1 } }, - rootGraph: { serialize: vi.fn(() => ({})) }, + rootGraph: { serialize: vi.fn(() => ({})), extra: {} }, loadGraphData: vi.fn() } })) @@ -97,7 +97,11 @@ vi.mock('@/renderer/core/thumbnail/useWorkflowThumbnail', () => ({ })) vi.mock('@/platform/telemetry', () => ({ - useTelemetry: () => null + useTelemetry: () => ({ + trackDefaultViewSet: vi.fn(), + trackWorkflowSaved: vi.fn(), + trackEnterLinear: vi.fn() + }) })) vi.mock('@/platform/workflow/persistence/stores/workflowDraftStore', () => ({ @@ -317,48 +321,6 @@ describe('useWorkflowService', () => { }) }) - describe('saveWorkflowAs', () => { - let workflowStore: ReturnType - - 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 let existingWorkflow: LoadedComfyWorkflow @@ -527,6 +489,20 @@ describe('useWorkflowService', () => { expect(workflow.initialMode).toBe('graph') expect(appMode.mode.value).toBe('builder:arrange') }) + + it('sets activeMode even when initialMode already matches', () => { + const workflow = createModeTestWorkflow({ + initialMode: 'app', + activeMode: null + }) + workflowStore.activeWorkflow = workflow + + // mode.value is 'app' via initialMode fallback, but activeMode + // must still be set so the UI transitions to app view + appMode.setMode('app') + + expect(workflow.activeMode).toBe('app') + }) }) describe('afterLoadNewGraph initializes initialMode', () => { @@ -675,6 +651,7 @@ describe('useWorkflowService', () => { service = useWorkflowService() vi.spyOn(workflowStore, 'saveWorkflow').mockResolvedValue() vi.spyOn(workflowStore, 'renameWorkflow').mockResolvedValue() + app.rootGraph.extra = {} }) function createTemporaryWorkflow( @@ -692,6 +669,34 @@ describe('useWorkflowService', () => { return workflow as LoadedComfyWorkflow } + it('should rename then save when workflow is temporary', async () => { + const workflow = createTemporaryWorkflow() + vi.mocked(workflowStore.getWorkflowByPath).mockReturnValue(null) + + const result = await service.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 service.saveWorkflowAs(workflow) + + expect(result).toBe(false) + expect(workflowStore.saveWorkflow).not.toHaveBeenCalled() + }) + it('appends .app.json extension when initialMode is app', async () => { const workflow = createTemporaryWorkflow() workflow.initialMode = 'app' @@ -726,6 +731,211 @@ describe('useWorkflowService', () => { 'workflows/my-workflow.json' ) }) + + it('uses isApp option over initialMode when provided (graph -> app)', async () => { + const workflow = createTemporaryWorkflow() + workflow.initialMode = 'graph' + + await service.saveWorkflowAs(workflow, { + filename: 'my-workflow', + isApp: true + }) + + expect(workflowStore.renameWorkflow).toHaveBeenCalledWith( + workflow, + 'workflows/my-workflow.app.json' + ) + }) + + it('uses isApp option over initialMode when provided (app -> graph)', async () => { + const workflow = createTemporaryWorkflow() + workflow.initialMode = 'app' + + await service.saveWorkflowAs(workflow, { + filename: 'my-workflow', + isApp: false + }) + + expect(workflowStore.renameWorkflow).toHaveBeenCalledWith( + workflow, + 'workflows/my-workflow.json' + ) + }) + + it('creates a copy when saving same name with different mode (not self-overwrite)', async () => { + const source = createModeTestWorkflow({ + path: 'workflows/test.json', + initialMode: 'graph' + }) + + const copy = createModeTestWorkflow({ + path: 'workflows/test.app.json' + }) + vi.spyOn(workflowStore, 'saveAs').mockReturnValue(copy) + vi.spyOn(workflowStore, 'openWorkflow').mockResolvedValue(copy) + + await service.saveWorkflowAs(source, { + filename: 'test', + isApp: true + }) + + // Different extension means different path, so it's not a self-overwrite + // — a new copy is created instead of modifying the source in place + expect(source.initialMode).toBe('graph') + expect(workflowStore.saveAs).toHaveBeenCalledWith( + source, + 'workflows/test.app.json' + ) + expect(workflowStore.saveWorkflow).toHaveBeenCalledWith(copy) + }) + + it('self-overwrites when saving same name with same mode', async () => { + const source = createModeTestWorkflow({ + path: 'workflows/test.app.json', + initialMode: 'app' + }) + vi.spyOn(workflowStore, 'getWorkflowByPath').mockReturnValue(source) + mockConfirm.mockResolvedValue(true) + + await service.saveWorkflowAs(source, { + filename: 'test', + isApp: true + }) + + // Same path → self-overwrite: saves in place via saveWorkflow, no copy + expect(workflowStore.saveAs).not.toHaveBeenCalled() + expect(workflowStore.saveWorkflow).toHaveBeenCalledWith(source) + }) + + it('does not modify source workflow mode when saving persisted workflow as different mode', async () => { + const source = createModeTestWorkflow({ + path: 'workflows/original.json', + initialMode: 'graph' + }) + + const copy = createModeTestWorkflow({ + path: 'workflows/copy.app.json' + }) + vi.spyOn(workflowStore, 'saveAs').mockReturnValue(copy) + vi.spyOn(workflowStore, 'openWorkflow').mockResolvedValue(copy) + + await service.saveWorkflowAs(source, { + filename: 'copy', + isApp: true + }) + + expect(source.initialMode).toBe('graph') + expect(copy.initialMode).toBe('app') + expect(workflowStore.saveAs).toHaveBeenCalledWith( + source, + 'workflows/copy.app.json' + ) + expect(workflowStore.saveWorkflow).toHaveBeenCalledWith(copy) + }) + + it('does not modify source workflow mode when saving app as graph', async () => { + const source = createModeTestWorkflow({ + path: 'workflows/original.app.json', + initialMode: 'app' + }) + + const copy = createModeTestWorkflow({ + path: 'workflows/copy.json' + }) + vi.spyOn(workflowStore, 'saveAs').mockReturnValue(copy) + vi.spyOn(workflowStore, 'openWorkflow').mockResolvedValue(copy) + + await service.saveWorkflowAs(source, { + filename: 'copy', + isApp: false + }) + + expect(source.initialMode).toBe('app') + expect(copy.initialMode).toBe('graph') + expect(workflowStore.saveAs).toHaveBeenCalledWith( + source, + 'workflows/copy.json' + ) + expect(workflowStore.saveWorkflow).toHaveBeenCalledWith(copy) + }) + + function captureLinearModeAtSaveTime() { + let value: boolean | undefined + vi.mocked(workflowStore.saveWorkflow).mockImplementation(async () => { + value = app.rootGraph.extra?.linearMode as boolean | undefined + }) + return () => value + } + + it('sets linearMode in graph data before saving (graph -> app)', async () => { + const workflow = createTemporaryWorkflow() + workflow.initialMode = 'graph' + app.rootGraph.extra = { linearMode: false } + const getLinearMode = captureLinearModeAtSaveTime() + + await service.saveWorkflowAs(workflow, { + filename: 'my-workflow', + isApp: true + }) + + expect(getLinearMode()).toBe(true) + }) + + it('sets linearMode in graph data before saving (app -> graph)', async () => { + const workflow = createTemporaryWorkflow() + workflow.initialMode = 'app' + app.rootGraph.extra = { linearMode: true } + const getLinearMode = captureLinearModeAtSaveTime() + + await service.saveWorkflowAs(workflow, { + filename: 'my-workflow', + isApp: false + }) + + expect(getLinearMode()).toBe(false) + }) + + it('sets linearMode before saving persisted workflow copy', async () => { + const source = createModeTestWorkflow({ + path: 'workflows/original.json', + initialMode: 'graph' + }) + app.rootGraph.extra = { linearMode: false } + + const copy = createModeTestWorkflow({ + path: 'workflows/original.app.json' + }) + vi.spyOn(workflowStore, 'saveAs').mockReturnValue(copy) + vi.spyOn(workflowStore, 'openWorkflow').mockResolvedValue(copy) + const getLinearMode = captureLinearModeAtSaveTime() + + await service.saveWorkflowAs(source, { + filename: 'original', + isApp: true + }) + + expect(getLinearMode()).toBe(true) + }) + + it('does not change initialMode when isApp is omitted (persisted copy)', async () => { + const source = createModeTestWorkflow({ + path: 'workflows/original.app.json', + initialMode: 'app' + }) + + // Real saveAs copies initialMode from source; replicate that here + const copy = createModeTestWorkflow({ + path: 'workflows/copy.app.json', + initialMode: 'app' + }) + vi.spyOn(workflowStore, 'saveAs').mockReturnValue(copy) + vi.spyOn(workflowStore, 'openWorkflow').mockResolvedValue(copy) + + await service.saveWorkflowAs(source, { filename: 'copy' }) + + // saveWorkflowAs should not change initialMode when isApp is omitted + expect(copy.initialMode).toBe('app') + }) }) describe('saveWorkflow', () => { diff --git a/src/platform/workflow/core/services/workflowService.ts b/src/platform/workflow/core/services/workflowService.ts index 4a1941ed41..1d21b72d94 100644 --- a/src/platform/workflow/core/services/workflowService.ts +++ b/src/platform/workflow/core/services/workflowService.ts @@ -114,12 +114,12 @@ export const useWorkflowService = () => { */ const saveWorkflowAs = async ( workflow: ComfyWorkflow, - options: { filename?: string } = {} + options: { filename?: string; isApp?: boolean } = {} ): Promise => { const newFilename = options.filename ?? (await workflow.promptSave()) if (!newFilename) return false - const isApp = workflow.initialMode === 'app' + const isApp = options.isApp ?? workflow.initialMode === 'app' const newPath = workflow.directory + '/' + appendWorkflowJsonExt(newFilename, isApp) const existingWorkflow = workflowStore.getWorkflowByPath(newPath) @@ -136,17 +136,27 @@ export const useWorkflowService = () => { } } - if (workflowStore.isActive(workflow)) workflow.changeTracker?.checkState() - if (isSelfOverwrite) { + workflow.changeTracker?.checkState() await saveWorkflow(workflow) - } else if (workflow.isTemporary) { - await renameWorkflow(workflow, newPath) - await workflowStore.saveWorkflow(workflow) } else { - const tempWorkflow = workflowStore.saveAs(workflow, newPath) - await openWorkflow(tempWorkflow) - await workflowStore.saveWorkflow(tempWorkflow) + let target: ComfyWorkflow + if (workflow.isTemporary) { + await renameWorkflow(workflow, newPath) + target = workflow + } else { + target = workflowStore.saveAs(workflow, newPath) + await openWorkflow(target) + } + + if (options.isApp !== undefined) { + app.rootGraph.extra ??= {} + app.rootGraph.extra.linearMode = isApp + target.initialMode = isApp ? 'app' : 'graph' + } + target.changeTracker?.checkState() + + await workflowStore.saveWorkflow(target) } useTelemetry()?.trackWorkflowSaved({ is_app: isApp, is_new: true }) diff --git a/src/platform/workflow/validation/schemas/workflowSchema.ts b/src/platform/workflow/validation/schemas/workflowSchema.ts index 656b278196..f7aec7a672 100644 --- a/src/platform/workflow/validation/schemas/workflowSchema.ts +++ b/src/platform/workflow/validation/schemas/workflowSchema.ts @@ -282,6 +282,7 @@ const zExtra = z workflowRendererVersion: zRendererType.optional(), BlueprintDescription: z.string().optional(), BlueprintSearchAliases: z.array(z.string()).optional(), + linearMode: z.boolean().optional(), linearData: z .object({ inputs: z.array(z.tuple([zNodeId, z.string()])).optional(),