From b7bebf9f1d58b8faaa0aba101267ceed8e80a137 Mon Sep 17 00:00:00 2001 From: Mutasem Aldmour <4711238+mutdmour@users.noreply.github.com> Date: Wed, 14 Jan 2026 08:45:09 +0100 Subject: [PATCH] fix(ai-builder): Fix dirty state for nodes when importing workflows (#24165) --- .../frontend/@n8n/i18n/src/locales/en.json | 1 + .../composables/useCanvasOperations.test.ts | 39 +- .../app/composables/useCanvasOperations.ts | 38 +- .../src/app/composables/useErrorHandler.ts | 57 ++ .../app/composables/useWorkflowState.test.ts | 93 +++ .../src/app/composables/useWorkflowState.ts | 23 + .../app/composables/useWorkflowUpdate.test.ts | 725 ++++++++++++++++++ .../src/app/composables/useWorkflowUpdate.ts | 325 ++++++++ .../editor-ui/src/app/views/NodeView.vue | 9 +- .../ai/assistant/builder.store.test.ts | 652 ---------------- .../features/ai/assistant/builder.store.ts | 155 +--- .../Agent/AskAssistantBuild.test.ts | 525 +++++++++---- .../components/Agent/AskAssistantBuild.vue | 95 ++- .../features/workflows/canvas/canvas.types.ts | 8 +- .../workflows/canvas/components/Canvas.vue | 19 +- .../canvas/composables/useCanvasLayout.ts | 3 +- 16 files changed, 1757 insertions(+), 1010 deletions(-) create mode 100644 packages/frontend/editor-ui/src/app/composables/useErrorHandler.ts create mode 100644 packages/frontend/editor-ui/src/app/composables/useWorkflowUpdate.test.ts create mode 100644 packages/frontend/editor-ui/src/app/composables/useWorkflowUpdate.ts diff --git a/packages/frontend/@n8n/i18n/src/locales/en.json b/packages/frontend/@n8n/i18n/src/locales/en.json index 1035ff1cbba..93d55b101a8 100644 --- a/packages/frontend/@n8n/i18n/src/locales/en.json +++ b/packages/frontend/@n8n/i18n/src/locales/en.json @@ -234,6 +234,7 @@ "aiAssistant.builder.invalidPrompt": "Prompt validation failed. Please try again with a clearer description of your workflow requirements and supported integrations.", "aiAssistant.builder.workflowParsingError.title": "Unable to insert workflow", "aiAssistant.builder.workflowParsingError.content": "The workflow returned by AI could not be parsed. Please try again.", + "aiAssistant.builder.error.title": "Workflow update failed", "aiAssistant.builder.canvasPrompt.title": "What would you like to automate?", "aiAssistant.builder.canvasPrompt.confirmTitle": "Replace current prompt?", "aiAssistant.builder.canvasPrompt.confirmMessage": "This will replace your current prompt. Are you sure?", diff --git a/packages/frontend/editor-ui/src/app/composables/useCanvasOperations.test.ts b/packages/frontend/editor-ui/src/app/composables/useCanvasOperations.test.ts index 02c778e1fa1..c2fef3e1c19 100644 --- a/packages/frontend/editor-ui/src/app/composables/useCanvasOperations.test.ts +++ b/packages/frontend/editor-ui/src/app/composables/useCanvasOperations.test.ts @@ -1384,7 +1384,7 @@ describe('useCanvasOperations', () => { }); describe('renameNode', () => { - it('should rename node', async () => { + it('should rename node and return true on success', async () => { const workflowsStore = mockedStore(useWorkflowsStore); const ndvStore = mockedStore(useNDVStore); const oldName = 'Old Node'; @@ -1398,13 +1398,14 @@ describe('useCanvasOperations', () => { ndvStore.activeNodeName = oldName; const { renameNode } = useCanvasOperations(); - await renameNode(oldName, newName); + const result = await renameNode(oldName, newName); + expect(result).toBe(true); expect(workflowObject.renameNode).toHaveBeenCalledWith(oldName, newName); expect(ndvStore.setActiveNodeName).toHaveBeenCalledWith(newName, expect.any(String)); }); - it('should not rename node when new name is same as old name', async () => { + it('should return false when new name is same as old name', async () => { const workflowsStore = mockedStore(useWorkflowsStore); const ndvStore = mockedStore(useNDVStore); const oldName = 'Old Node'; @@ -1412,12 +1413,13 @@ describe('useCanvasOperations', () => { ndvStore.activeNodeName = oldName; const { renameNode } = useCanvasOperations(); - await renameNode(oldName, oldName); + const result = await renameNode(oldName, oldName); + expect(result).toBe(false); expect(ndvStore.activeNodeName).toBe(oldName); }); - it('should show error toast when renameNode throws an error', async () => { + it('should show error toast and return false when renameNode throws an error', async () => { const workflowsStore = mockedStore(useWorkflowsStore); const ndvStore = mockedStore(useNDVStore); const toast = useToast(); @@ -1437,8 +1439,9 @@ describe('useCanvasOperations', () => { ndvStore.activeNodeName = oldName; const { renameNode } = useCanvasOperations(); - await renameNode(oldName, newName); + const result = await renameNode(oldName, newName); + expect(result).toBe(false); expect(workflowObject.renameNode).toHaveBeenCalledWith(oldName, newName); expect(toast.showMessage).toHaveBeenCalledWith({ type: 'error', @@ -1446,6 +1449,30 @@ describe('useCanvasOperations', () => { message: errorDescription, }); }); + + it('should not show error toast when showErrorToast is false', async () => { + const workflowsStore = mockedStore(useWorkflowsStore); + const ndvStore = mockedStore(useNDVStore); + const toast = useToast(); + const oldName = 'Old Node'; + const newName = 'New Node'; + const errorMessage = 'Node name already exists'; + + const workflowObject = createTestWorkflowObject(); + workflowObject.renameNode = vi.fn().mockImplementation(() => { + throw new UserError(errorMessage); + }); + workflowsStore.workflowObject = workflowObject; + workflowsStore.cloneWorkflowObject = vi.fn().mockReturnValue(workflowObject); + workflowsStore.getNodeByName = vi.fn().mockReturnValue({ name: oldName }); + ndvStore.activeNodeName = oldName; + + const { renameNode } = useCanvasOperations(); + const result = await renameNode(oldName, newName, { showErrorToast: false }); + + expect(result).toBe(false); + expect(toast.showMessage).not.toHaveBeenCalled(); + }); }); describe('revertRenameNode', () => { diff --git a/packages/frontend/editor-ui/src/app/composables/useCanvasOperations.ts b/packages/frontend/editor-ui/src/app/composables/useCanvasOperations.ts index 3a697ba0312..c1bd6c8f720 100644 --- a/packages/frontend/editor-ui/src/app/composables/useCanvasOperations.ts +++ b/packages/frontend/editor-ui/src/app/composables/useCanvasOperations.ts @@ -211,11 +211,19 @@ export function useCanvasOperations() { function tidyUp( { result, source, target }: CanvasLayoutEvent, - { trackEvents = true }: { trackEvents?: boolean } = {}, + { + trackEvents = true, + trackHistory = true, + trackBulk = true, + }: { + trackEvents?: boolean; + trackHistory?: boolean; + trackBulk?: boolean; + } = {}, ) { updateNodesPosition( result.nodes.map(({ id, x, y }) => ({ id, position: { x, y } })), - { trackBulk: true, trackHistory: true }, + { trackBulk, trackHistory }, ); if (trackEvents) { @@ -315,13 +323,17 @@ export function useCanvasOperations() { replaceNodeParameters(nodeId, newParameters, currentParameters); } + /** + * Rename a node and update all references (connections, expressions, pinData, etc.) + * @returns `true` if rename succeeded, `false` if it failed or was skipped + */ async function renameNode( currentName: string, newName: string, - { trackHistory = false, trackBulk = true } = {}, - ) { + { trackHistory = false, trackBulk = true, showErrorToast = true } = {}, + ): Promise { if (currentName === newName) { - return; + return false; } if (trackHistory && trackBulk) { @@ -335,12 +347,14 @@ export function useCanvasOperations() { try { workflow.renameNode(currentName, newName); } catch (error) { - toast.showMessage({ - type: 'error', - title: error.message, - message: error.description, - }); - return; + if (showErrorToast) { + toast.showMessage({ + type: 'error', + title: error.message, + message: error.description, + }); + } + return false; } if (trackHistory) { @@ -361,6 +375,8 @@ export function useCanvasOperations() { if (trackHistory && trackBulk) { historyStore.stopRecordingUndo(); } + + return true; } async function revertRenameNode(currentName: string, previousName: string) { diff --git a/packages/frontend/editor-ui/src/app/composables/useErrorHandler.ts b/packages/frontend/editor-ui/src/app/composables/useErrorHandler.ts new file mode 100644 index 00000000000..76cd21a4238 --- /dev/null +++ b/packages/frontend/editor-ui/src/app/composables/useErrorHandler.ts @@ -0,0 +1,57 @@ +import { useToast } from '@/app/composables/useToast'; +import { useI18n } from '@n8n/i18n'; +import { captureException } from '@sentry/vue'; +import type { BaseTextKey } from '@n8n/i18n'; + +export interface ErrorHandlerOptions { + /** Source identifier for Sentry tags (e.g., 'ai-builder', 'workflow-editor') */ + source: string; + /** i18n key for the error toast title */ + titleKey?: BaseTextKey; +} + +export interface HandleErrorOptions { + /** Additional context for the error (used in console log and Sentry tags) */ + context?: string; + /** Override the default toast title */ + title?: string; + /** Override the default toast message (defaults to error.message) */ + message?: string; +} + +/** + * Composable for unified error handling. + * Shows error toast, logs to console, and captures exception to Sentry. + */ +export function useErrorHandler(options: ErrorHandlerOptions) { + const toast = useToast(); + const i18n = useI18n(); + + /** + * Handle an error by showing a toast, logging to console, and capturing to Sentry. + */ + function handleError(error: unknown, errorOptions?: HandleErrorOptions): void { + const errorMessage = error instanceof Error ? error.message : String(error); + const context = errorOptions?.context; + + console.error(`[${options.source}${context ? ` - ${context}` : ''}]`, error); + + captureException(error, { + tags: { + source: options.source, + context, + }, + }); + + const title = + errorOptions?.title ?? (options.titleKey ? i18n.baseText(options.titleKey) : 'Error'); + + toast.showMessage({ + type: 'error', + title, + message: errorOptions?.message ?? errorMessage, + }); + } + + return { handleError }; +} diff --git a/packages/frontend/editor-ui/src/app/composables/useWorkflowState.test.ts b/packages/frontend/editor-ui/src/app/composables/useWorkflowState.test.ts index 640993dbbbc..78e5887c452 100644 --- a/packages/frontend/editor-ui/src/app/composables/useWorkflowState.test.ts +++ b/packages/frontend/editor-ui/src/app/composables/useWorkflowState.test.ts @@ -241,4 +241,97 @@ describe('useWorkflowState', () => { }); }); }); + + describe('updateNodeById', () => { + beforeEach(() => { + workflowsStore.setNodes([ + createTestNode({ id: 'node-1', name: 'First Node', parameters: { key: 'value1' } }), + createTestNode({ id: 'node-2', name: 'Second Node', parameters: { key: 'value2' } }), + ]); + }); + + it('should update node by ID and return true', () => { + const result = workflowState.updateNodeById('node-1', { name: 'Updated First Node' }); + + expect(result).toBe(true); + expect(workflowsStore.workflow.nodes[0].name).toBe('Updated First Node'); + }); + + it('should return false if node ID is not found', () => { + const result = workflowState.updateNodeById('non-existent-id', { name: 'Updated Node' }); + + expect(result).toBe(false); + // Nodes should remain unchanged + expect(workflowsStore.workflow.nodes[0].name).toBe('First Node'); + expect(workflowsStore.workflow.nodes[1].name).toBe('Second Node'); + }); + + it('should update only specified properties', () => { + const result = workflowState.updateNodeById('node-2', { + name: 'Updated Second Node', + disabled: true, + }); + + expect(result).toBe(true); + expect(workflowsStore.workflow.nodes[1].name).toBe('Updated Second Node'); + expect(workflowsStore.workflow.nodes[1].disabled).toBe(true); + // Other properties should remain unchanged + expect(workflowsStore.workflow.nodes[1].id).toBe('node-2'); + }); + + it('should return false if node data is unchanged', () => { + const result = workflowState.updateNodeById('node-1', { name: 'First Node' }); + + expect(result).toBe(false); + }); + }); + + describe('resetParametersLastUpdatedAt', () => { + it('should set parametersLastUpdatedAt on existing metadata', () => { + const nodeName = 'Test Node'; + workflowsStore.addNode({ + parameters: {}, + id: 'test-node-id', + name: nodeName, + type: 'n8n-nodes-base.set', + position: [0, 0], + typeVersion: 1, + }); + + expect(workflowsStore.nodeMetadata[nodeName].parametersLastUpdatedAt).toBeUndefined(); + + workflowState.resetParametersLastUpdatedAt(nodeName); + + expect(workflowsStore.nodeMetadata[nodeName].parametersLastUpdatedAt).toEqual( + expect.any(Number), + ); + }); + + it('should create metadata if it does not exist', () => { + const nodeName = 'New Node Without Metadata'; + // Node metadata doesn't exist yet + expect(workflowsStore.nodeMetadata[nodeName]).toBeUndefined(); + + workflowState.resetParametersLastUpdatedAt(nodeName); + + expect(workflowsStore.nodeMetadata[nodeName]).toBeDefined(); + expect(workflowsStore.nodeMetadata[nodeName].parametersLastUpdatedAt).toEqual( + expect.any(Number), + ); + }); + + it('should preserve existing metadata properties when updating', () => { + const nodeName = 'Node With Existing Metadata'; + workflowsStore.nodeMetadata[nodeName] = { + pristine: true, + }; + + workflowState.resetParametersLastUpdatedAt(nodeName); + + expect(workflowsStore.nodeMetadata[nodeName].pristine).toBe(true); + expect(workflowsStore.nodeMetadata[nodeName].parametersLastUpdatedAt).toEqual( + expect.any(Number), + ); + }); + }); }); diff --git a/packages/frontend/editor-ui/src/app/composables/useWorkflowState.ts b/packages/frontend/editor-ui/src/app/composables/useWorkflowState.ts index e99aa4477ac..d6c858433b3 100644 --- a/packages/frontend/editor-ui/src/app/composables/useWorkflowState.ts +++ b/packages/frontend/editor-ui/src/app/composables/useWorkflowState.ts @@ -395,6 +395,27 @@ export function useWorkflowState() { setNodeValue({ name: node.name, key: 'position', value: position }); } + /** + * Update node by ID. Finds the node by ID and updates it with the provided data. + * @returns `true` if the node was found and updated + */ + function updateNodeById(nodeId: string, nodeData: Partial): boolean { + const nodeIndex = ws.workflow.nodes.findIndex((node) => node.id === nodeId); + if (nodeIndex === -1) return false; + return updateNodeAtIndex(nodeIndex, nodeData); + } + + /** + * Reset parametersLastUpdatedAt to current timestamp for a node. + * Used to mark a node as "dirty" when its parameters change. + */ + function resetParametersLastUpdatedAt(nodeName: string): void { + if (!ws.nodeMetadata[nodeName]) { + ws.nodeMetadata[nodeName] = { pristine: true }; + } + ws.nodeMetadata[nodeName].parametersLastUpdatedAt = Date.now(); + } + function updateNodeProperties( this: WorkflowState, updateInformation: INodeUpdatePropertiesInformation, @@ -484,7 +505,9 @@ export function useWorkflowState() { setNodePositionById, setNodeIssue, updateNodeAtIndex, + updateNodeById, updateNodeProperties, + resetParametersLastUpdatedAt, // reexport executingNode: workflowStateStore.executingNode, diff --git a/packages/frontend/editor-ui/src/app/composables/useWorkflowUpdate.test.ts b/packages/frontend/editor-ui/src/app/composables/useWorkflowUpdate.test.ts new file mode 100644 index 00000000000..b0c1e3fc2d3 --- /dev/null +++ b/packages/frontend/editor-ui/src/app/composables/useWorkflowUpdate.test.ts @@ -0,0 +1,725 @@ +import { describe, it, expect, vi, beforeEach } from 'vitest'; +import { setActivePinia } from 'pinia'; +import { createTestingPinia } from '@pinia/testing'; +import { useWorkflowUpdate } from './useWorkflowUpdate'; +import { useWorkflowsStore } from '@/app/stores/workflows.store'; +import { useCredentialsStore } from '@/features/credentials/credentials.store'; +import { useNodeTypesStore } from '@/app/stores/nodeTypes.store'; +import { useBuilderStore } from '@/features/ai/assistant/builder.store'; +import { mockedStore } from '@/__tests__/utils'; +import { createTestNode } from '@/__tests__/mocks'; +import type { INodeUi } from '@/Interface'; +import { DEFAULT_NEW_WORKFLOW_NAME } from '@/app/constants'; + +// Mock canvas event bus - using hoisted to ensure proper initialization order +const canvasEventBusEmitMock = vi.hoisted(() => vi.fn()); +vi.mock('@/features/workflows/canvas/canvas.eventBus', () => ({ + canvasEventBus: { + emit: canvasEventBusEmitMock, + }, +})); + +// Mock canvas utils - using hoisted for proper initialization +const mockMapLegacyConnectionsToCanvasConnections = vi.hoisted(() => vi.fn().mockReturnValue([])); +vi.mock('@/features/workflows/canvas/canvas.utils', () => ({ + mapLegacyConnectionsToCanvasConnections: mockMapLegacyConnectionsToCanvasConnections, +})); + +// Mock useWorkflowState - using hoisted for proper initialization +const mockWorkflowState = vi.hoisted(() => ({ + resetParametersLastUpdatedAt: vi.fn(), + setWorkflowName: vi.fn(), +})); +vi.mock('@/app/composables/useWorkflowState', () => ({ + injectWorkflowState: vi.fn(() => mockWorkflowState), +})); + +// Mock useCanvasOperations - using hoisted for proper initialization +const mockCanvasOperations = vi.hoisted(() => ({ + deleteNode: vi.fn(), + addNodes: vi.fn().mockResolvedValue([]), + deleteConnection: vi.fn(), + addConnections: vi.fn().mockResolvedValue(undefined), + renameNode: vi.fn().mockResolvedValue(true), +})); +vi.mock('@/app/composables/useCanvasOperations', () => ({ + useCanvasOperations: vi.fn(() => mockCanvasOperations), +})); + +// Mock nodeTypesUtils +vi.mock('@/app/utils/nodeTypesUtils', () => ({ + getMainAuthField: vi.fn(), + getAuthTypeForNodeCredential: vi.fn(), +})); + +describe('useWorkflowUpdate', () => { + let workflowsStore: ReturnType>; + let builderStore: ReturnType>; + let credentialsStore: ReturnType>; + let nodeTypesStore: ReturnType>; + + beforeEach(() => { + vi.clearAllMocks(); + + setActivePinia(createTestingPinia()); + + workflowsStore = mockedStore(useWorkflowsStore); + builderStore = mockedStore(useBuilderStore); + credentialsStore = mockedStore(useCredentialsStore); + nodeTypesStore = mockedStore(useNodeTypesStore); + + // Reset canvas utils mock to default behavior + mockMapLegacyConnectionsToCanvasConnections.mockReturnValue([]); + + // Setup default mocks + workflowsStore.allNodes = []; + workflowsStore.workflow = { + id: 'test-workflow', + name: DEFAULT_NEW_WORKFLOW_NAME, + nodes: [], + connections: {}, + } as unknown as ReturnType['workflow']; + workflowsStore.cloneWorkflowObject = vi.fn().mockReturnValue({ + nodes: {}, + connectionsBySourceNode: {}, + renameNode: vi.fn(), + }); + workflowsStore.setNodes = vi.fn(); + workflowsStore.setConnections = vi.fn(); + workflowsStore.nodesByName = {}; + + builderStore.setBuilderMadeEdits = vi.fn(); + + credentialsStore.getCredentialsByType = vi.fn().mockReturnValue([]); + nodeTypesStore.getNodeType = vi.fn(); + }); + + describe('updateWorkflow', () => { + it('should return success with empty newNodeIds when no new nodes are added', async () => { + const { updateWorkflow } = useWorkflowUpdate(); + + const result = await updateWorkflow({ + nodes: [], + connections: {}, + }); + + expect(result).toEqual({ success: true, newNodeIds: [] }); + }); + + it('should call builderStore.setBuilderMadeEdits(true)', async () => { + const { updateWorkflow } = useWorkflowUpdate(); + + await updateWorkflow({ + nodes: [], + connections: {}, + }); + + expect(builderStore.setBuilderMadeEdits).toHaveBeenCalledWith(true); + }); + + describe('node categorization', () => { + it('should add new nodes that do not exist in current workflow', async () => { + const newNode = createTestNode({ + id: 'new-node-1', + name: 'New Node', + type: 'n8n-nodes-base.httpRequest', + }); + + mockCanvasOperations.addNodes.mockResolvedValue([newNode as INodeUi]); + + const { updateWorkflow } = useWorkflowUpdate(); + + const result = await updateWorkflow({ + nodes: [newNode], + connections: {}, + }); + + expect(mockCanvasOperations.addNodes).toHaveBeenCalledWith( + [expect.objectContaining({ id: 'new-node-1' })], + { + trackHistory: true, + trackBulk: false, + forcePosition: true, + telemetry: false, + }, + ); + expect(result.success).toBe(true); + if (result.success) { + expect(result.newNodeIds).toEqual(['new-node-1']); + } + }); + + it('should remove nodes that are no longer in the update', async () => { + const existingNode = createTestNode({ + id: 'existing-node', + name: 'Existing Node', + }) as INodeUi; + + workflowsStore.allNodes = [existingNode]; + + const { updateWorkflow } = useWorkflowUpdate(); + + await updateWorkflow({ + nodes: [], // Empty - existing node should be removed + connections: {}, + }); + + expect(mockCanvasOperations.deleteNode).toHaveBeenCalledWith('existing-node', { + trackHistory: true, + trackBulk: false, + }); + }); + + it('should update existing nodes in place', async () => { + const existingNode = createTestNode({ + id: 'node-1', + name: 'Old Name', + parameters: { url: 'http://old.com' }, + position: [100, 200], + }) as INodeUi; + + workflowsStore.allNodes = [existingNode]; + + const mockWorkflowObject = { + nodes: { 'Old Name': { ...existingNode } }, + connectionsBySourceNode: {}, + renameNode: vi.fn(), + }; + workflowsStore.cloneWorkflowObject = vi.fn().mockReturnValue(mockWorkflowObject); + + const { updateWorkflow } = useWorkflowUpdate(); + + await updateWorkflow({ + nodes: [ + { + id: 'node-1', + name: 'Old Name', + type: 'n8n-nodes-base.httpRequest', + typeVersion: 1, + position: [300, 400], // New position should be ignored + parameters: { url: 'http://new.com' }, + }, + ], + connections: {}, + }); + + // Should not add or remove nodes + expect(mockCanvasOperations.addNodes).not.toHaveBeenCalled(); + expect(mockCanvasOperations.deleteNode).not.toHaveBeenCalled(); + + // Should sync state back to store + expect(workflowsStore.setNodes).toHaveBeenCalled(); + expect(workflowsStore.setConnections).toHaveBeenCalled(); + }); + }); + + describe('node renaming', () => { + it('should handle node renames via canvasOperations.renameNode()', async () => { + const existingNode = createTestNode({ + id: 'node-1', + name: 'Old Name', + }) as INodeUi; + + workflowsStore.allNodes = [existingNode]; + + // After rename, cloneWorkflowObject returns node with new name + const mockWorkflowObject = { + nodes: { 'New Name': { ...existingNode, name: 'New Name' } }, + connectionsBySourceNode: {}, + renameNode: vi.fn(), + }; + workflowsStore.cloneWorkflowObject = vi.fn().mockReturnValue(mockWorkflowObject); + + const { updateWorkflow } = useWorkflowUpdate(); + + await updateWorkflow({ + nodes: [ + { + id: 'node-1', + name: 'New Name', // Renamed + type: 'n8n-nodes-base.httpRequest', + typeVersion: 1, + position: [0, 0], + parameters: {}, + }, + ], + connections: {}, + }); + + expect(mockCanvasOperations.renameNode).toHaveBeenCalledWith('Old Name', 'New Name', { + trackHistory: true, + trackBulk: false, + showErrorToast: false, + }); + }); + + it('should not call renameNode when node name is unchanged', async () => { + const existingNode = createTestNode({ + id: 'node-1', + name: 'Same Name', + }) as INodeUi; + + workflowsStore.allNodes = [existingNode]; + + const mockWorkflowObject = { + nodes: { 'Same Name': { ...existingNode } }, + connectionsBySourceNode: {}, + renameNode: vi.fn(), + }; + workflowsStore.cloneWorkflowObject = vi.fn().mockReturnValue(mockWorkflowObject); + + const { updateWorkflow } = useWorkflowUpdate(); + + await updateWorkflow({ + nodes: [ + { + id: 'node-1', + name: 'Same Name', // No rename + type: 'n8n-nodes-base.httpRequest', + typeVersion: 1, + position: [0, 0], + parameters: {}, + }, + ], + connections: {}, + }); + + expect(mockCanvasOperations.renameNode).not.toHaveBeenCalled(); + }); + + it('should use old name when rename fails', async () => { + const existingNode = createTestNode({ + id: 'node-1', + name: 'Old Name', + parameters: { url: 'http://example.com' }, + }) as INodeUi; + + workflowsStore.allNodes = [existingNode]; + + // Rename fails + mockCanvasOperations.renameNode.mockResolvedValueOnce(false); + + // Workflow still has node under old name since rename failed + const mockWorkflowObject = { + nodes: { 'Old Name': { ...existingNode } }, + connectionsBySourceNode: {}, + renameNode: vi.fn(), + }; + workflowsStore.cloneWorkflowObject = vi.fn().mockReturnValue(mockWorkflowObject); + + const { updateWorkflow } = useWorkflowUpdate(); + + await updateWorkflow({ + nodes: [ + { + id: 'node-1', + name: 'New Name', // Attempted rename + type: 'n8n-nodes-base.httpRequest', + typeVersion: 1, + position: [0, 0], + parameters: { url: 'http://updated.com' }, + }, + ], + connections: {}, + }); + + // Should still update node properties under old name + expect(workflowsStore.setNodes).toHaveBeenCalled(); + const setNodesCall = workflowsStore.setNodes.mock.calls[0][0]; + expect(setNodesCall[0].name).toBe('Old Name'); + expect(setNodesCall[0].parameters.url).toBe('http://updated.com'); + }); + }); + + describe('parameter change tracking', () => { + it('should mark node as dirty when parameters change', async () => { + const existingNode = createTestNode({ + id: 'node-1', + name: 'HTTP Request', + parameters: { url: 'http://old.com' }, + }) as INodeUi; + + workflowsStore.allNodes = [existingNode]; + + const mockWorkflowObject = { + nodes: { 'HTTP Request': { ...existingNode } }, + connectionsBySourceNode: {}, + renameNode: vi.fn(), + }; + workflowsStore.cloneWorkflowObject = vi.fn().mockReturnValue(mockWorkflowObject); + + const { updateWorkflow } = useWorkflowUpdate(); + + await updateWorkflow({ + nodes: [ + { + id: 'node-1', + name: 'HTTP Request', + type: 'n8n-nodes-base.httpRequest', + typeVersion: 1, + position: [0, 0], + parameters: { url: 'http://new.com' }, // Changed + }, + ], + connections: {}, + }); + + expect(mockWorkflowState.resetParametersLastUpdatedAt).toHaveBeenCalledWith('HTTP Request'); + }); + + it('should not mark node as dirty when parameters are unchanged', async () => { + const existingNode = createTestNode({ + id: 'node-1', + name: 'HTTP Request', + parameters: { url: 'http://same.com' }, + }) as INodeUi; + + workflowsStore.allNodes = [existingNode]; + + const mockWorkflowObject = { + nodes: { 'HTTP Request': { ...existingNode } }, + connectionsBySourceNode: {}, + renameNode: vi.fn(), + }; + workflowsStore.cloneWorkflowObject = vi.fn().mockReturnValue(mockWorkflowObject); + + const { updateWorkflow } = useWorkflowUpdate(); + + await updateWorkflow({ + nodes: [ + { + id: 'node-1', + name: 'HTTP Request', + type: 'n8n-nodes-base.httpRequest', + typeVersion: 1, + position: [0, 0], + parameters: { url: 'http://same.com' }, // Same + }, + ], + connections: {}, + }); + + expect(mockWorkflowState.resetParametersLastUpdatedAt).not.toHaveBeenCalled(); + }); + }); + + describe('workflow name update', () => { + it('should update workflow name on initial generation when name starts with default', async () => { + workflowsStore.workflow.name = DEFAULT_NEW_WORKFLOW_NAME; + + const { updateWorkflow } = useWorkflowUpdate(); + + await updateWorkflow( + { + name: 'My Generated Workflow', + nodes: [], + connections: {}, + }, + { isInitialGeneration: true }, + ); + + expect(mockWorkflowState.setWorkflowName).toHaveBeenCalledWith({ + newName: 'My Generated Workflow', + setStateDirty: false, + }); + }); + + it('should not update workflow name when not initial generation', async () => { + workflowsStore.workflow.name = DEFAULT_NEW_WORKFLOW_NAME; + + const { updateWorkflow } = useWorkflowUpdate(); + + await updateWorkflow( + { + name: 'My Generated Workflow', + nodes: [], + connections: {}, + }, + { isInitialGeneration: false }, + ); + + expect(mockWorkflowState.setWorkflowName).not.toHaveBeenCalled(); + }); + + it('should not update workflow name when current name does not start with default', async () => { + workflowsStore.workflow.name = 'Custom Workflow Name'; + + const { updateWorkflow } = useWorkflowUpdate(); + + await updateWorkflow( + { + name: 'My Generated Workflow', + nodes: [], + connections: {}, + }, + { isInitialGeneration: true }, + ); + + expect(mockWorkflowState.setWorkflowName).not.toHaveBeenCalled(); + }); + }); + + describe('tidyUp behavior', () => { + it('should emit tidyUp event with new node IDs', async () => { + const newNode = createTestNode({ + id: 'new-node-1', + name: 'New Node', + }); + + mockCanvasOperations.addNodes.mockResolvedValue([newNode as INodeUi]); + + const { updateWorkflow } = useWorkflowUpdate(); + + await updateWorkflow({ + nodes: [newNode], + connections: {}, + }); + + expect(canvasEventBusEmitMock).toHaveBeenCalledWith('tidyUp', { + source: 'builder-update', + nodeIdsFilter: ['new-node-1'], + trackEvents: false, + trackHistory: true, + trackBulk: false, + }); + }); + + it('should combine new node IDs with passed nodeIdsToTidyUp', async () => { + const newNode = createTestNode({ + id: 'new-node-2', + name: 'New Node', + }); + + mockCanvasOperations.addNodes.mockResolvedValue([newNode as INodeUi]); + + const { updateWorkflow } = useWorkflowUpdate(); + + await updateWorkflow( + { + nodes: [newNode], + connections: {}, + }, + { nodeIdsToTidyUp: ['previous-node-1'] }, + ); + + expect(canvasEventBusEmitMock).toHaveBeenCalledWith('tidyUp', { + source: 'builder-update', + nodeIdsFilter: ['new-node-2', 'previous-node-1'], + trackEvents: false, + trackHistory: true, + trackBulk: false, + }); + }); + + it('should not emit tidyUp event when there are no node IDs to tidy up', async () => { + const { updateWorkflow } = useWorkflowUpdate(); + + await updateWorkflow({ + nodes: [], + connections: {}, + }); + + expect(canvasEventBusEmitMock).not.toHaveBeenCalled(); + }); + }); + + describe('default credentials', () => { + it('should apply default credentials to nodes without credentials', async () => { + const nodeWithoutCreds = createTestNode({ + id: 'node-1', + name: 'HTTP Request', + type: 'n8n-nodes-base.httpRequest', + credentials: undefined, + }); + + const mockCredential = { + id: 'cred-1', + name: 'My Credential', + type: 'httpBasicAuth', + }; + + nodeTypesStore.getNodeType = vi.fn().mockReturnValue({ + credentials: [{ name: 'httpBasicAuth' }], + }); + credentialsStore.getCredentialsByType = vi.fn().mockReturnValue([mockCredential]); + + mockCanvasOperations.addNodes.mockResolvedValue([nodeWithoutCreds as INodeUi]); + + const { updateWorkflow } = useWorkflowUpdate(); + + await updateWorkflow({ + nodes: [nodeWithoutCreds], + connections: {}, + }); + + // Credentials should be applied to the node before it's added to the store + expect(mockCanvasOperations.addNodes).toHaveBeenCalledWith( + [ + expect.objectContaining({ + credentials: { httpBasicAuth: { id: 'cred-1', name: 'My Credential' } }, + }), + ], + expect.any(Object), + ); + }); + + it('should not override existing credentials', async () => { + const existingCredentials = { httpBasicAuth: { id: 'existing-cred', name: 'Existing' } }; + const nodeWithCreds = createTestNode({ + id: 'node-1', + name: 'HTTP Request', + type: 'n8n-nodes-base.httpRequest', + credentials: existingCredentials, + }); + + nodeTypesStore.getNodeType = vi.fn().mockReturnValue({ + credentials: [{ name: 'httpBasicAuth' }], + }); + credentialsStore.getCredentialsByType = vi + .fn() + .mockReturnValue([{ id: 'other-cred', name: 'Other', type: 'httpBasicAuth' }]); + + mockCanvasOperations.addNodes.mockResolvedValue([nodeWithCreds as INodeUi]); + + const { updateWorkflow } = useWorkflowUpdate(); + + await updateWorkflow({ + nodes: [nodeWithCreds], + connections: {}, + }); + + // Existing credentials should not be overwritten + expect(mockCanvasOperations.addNodes).toHaveBeenCalledWith( + [expect.objectContaining({ credentials: existingCredentials })], + expect.any(Object), + ); + }); + }); + + describe('error handling', () => { + it('should return success: false with error when addNodes throws', async () => { + const testError = new Error('Failed to add nodes'); + mockCanvasOperations.addNodes.mockRejectedValueOnce(testError); + + const newNode = createTestNode({ + id: 'new-node-1', + name: 'New Node', + type: 'n8n-nodes-base.httpRequest', + }); + + const { updateWorkflow } = useWorkflowUpdate(); + + const result = await updateWorkflow({ + nodes: [newNode], + connections: {}, + }); + + expect(result.success).toBe(false); + if (!result.success) { + expect(result.error).toBe(testError); + } + }); + + it('should return success: false with error when addConnections throws', async () => { + const testError = new Error('Failed to add connections'); + mockCanvasOperations.addConnections.mockRejectedValueOnce(testError); + + // Mock mapLegacyConnectionsToCanvasConnections to return: + // - empty array for existing connections (first call) + // - non-empty array for new connections (second call) to trigger addConnections + mockMapLegacyConnectionsToCanvasConnections + .mockReturnValueOnce([]) // existing connections + .mockReturnValueOnce([ + { source: 'node-1', target: 'node-2', sourceHandle: 'main', targetHandle: 'main' }, + ]); // new connections + + const { updateWorkflow } = useWorkflowUpdate(); + + const result = await updateWorkflow({ + nodes: [], + connections: { 'Node 1': { main: [[{ node: 'Node 2', type: 'main', index: 0 }]] } }, + }); + + expect(result.success).toBe(false); + if (!result.success) { + expect(result.error).toBe(testError); + } + }); + + it('should return success: false with error when cloneWorkflowObject throws', async () => { + const existingNode = createTestNode({ + id: 'node-1', + name: 'Existing Node', + }) as INodeUi; + + workflowsStore.allNodes = [existingNode]; + + const testError = new Error('Failed to clone workflow'); + workflowsStore.cloneWorkflowObject = vi.fn().mockImplementation(() => { + throw testError; + }); + + const { updateWorkflow } = useWorkflowUpdate(); + + const result = await updateWorkflow({ + nodes: [ + { + id: 'node-1', + name: 'Existing Node', + type: 'n8n-nodes-base.httpRequest', + typeVersion: 1, + position: [0, 0], + parameters: { updated: true }, + }, + ], + connections: {}, + }); + + expect(result.success).toBe(false); + if (!result.success) { + expect(result.error).toBe(testError); + } + }); + + it('should not call setBuilderMadeEdits when error occurs', async () => { + const testError = new Error('Failed to add nodes'); + mockCanvasOperations.addNodes.mockRejectedValueOnce(testError); + + const newNode = createTestNode({ + id: 'new-node-1', + name: 'New Node', + type: 'n8n-nodes-base.httpRequest', + }); + + const { updateWorkflow } = useWorkflowUpdate(); + + await updateWorkflow({ + nodes: [newNode], + connections: {}, + }); + + expect(builderStore.setBuilderMadeEdits).not.toHaveBeenCalled(); + }); + + it('should not emit tidyUp event when error occurs', async () => { + const testError = new Error('Failed to add nodes'); + mockCanvasOperations.addNodes.mockRejectedValueOnce(testError); + + const newNode = createTestNode({ + id: 'new-node-1', + name: 'New Node', + type: 'n8n-nodes-base.httpRequest', + }); + + const { updateWorkflow } = useWorkflowUpdate(); + + await updateWorkflow({ + nodes: [newNode], + connections: {}, + }); + + expect(canvasEventBusEmitMock).not.toHaveBeenCalled(); + }); + }); + }); +}); diff --git a/packages/frontend/editor-ui/src/app/composables/useWorkflowUpdate.ts b/packages/frontend/editor-ui/src/app/composables/useWorkflowUpdate.ts new file mode 100644 index 00000000000..f1fb72280ab --- /dev/null +++ b/packages/frontend/editor-ui/src/app/composables/useWorkflowUpdate.ts @@ -0,0 +1,325 @@ +/** + * Composable for updating workflows with new data from the AI Builder. + * Handles update-in-place logic: updates existing nodes by ID, adds new nodes, + * removes stale nodes, and handles renames. + */ +import { DEFAULT_NEW_WORKFLOW_NAME } from '@/app/constants'; +import type { INodeUi } from '@/Interface'; +import { useWorkflowsStore } from '@/app/stores/workflows.store'; +import { useCredentialsStore } from '@/features/credentials/credentials.store'; +import { useNodeTypesStore } from '@/app/stores/nodeTypes.store'; +import { useBuilderStore } from '@/features/ai/assistant/builder.store'; +import { injectWorkflowState } from '@/app/composables/useWorkflowState'; +import { useCanvasOperations } from '@/app/composables/useCanvasOperations'; +import { canvasEventBus } from '@/features/workflows/canvas/canvas.eventBus'; +import { mapLegacyConnectionsToCanvasConnections } from '@/features/workflows/canvas/canvas.utils'; +import { getAuthTypeForNodeCredential, getMainAuthField } from '@/app/utils/nodeTypesUtils'; +import type { WorkflowDataUpdate } from '@n8n/rest-api-client/api/workflows'; +import type { IConnections, INode } from 'n8n-workflow'; +import isEqual from 'lodash/isEqual'; + +export interface UpdateWorkflowOptions { + isInitialGeneration?: boolean; + /** Node IDs to tidy up (in addition to any newly added nodes) */ + nodeIdsToTidyUp?: string[]; +} + +export type UpdateWorkflowResult = + | { + success: true; + /** IDs of nodes that were newly added */ + newNodeIds: string[]; + } + | { + success: false; + error: unknown; + }; + +export function useWorkflowUpdate() { + const workflowsStore = useWorkflowsStore(); + const workflowState = injectWorkflowState(); + const credentialsStore = useCredentialsStore(); + const nodeTypesStore = useNodeTypesStore(); + const builderStore = useBuilderStore(); + const canvasOperations = useCanvasOperations(); + + /** + * Categorize nodes into those to update, add, or remove + */ + function categorizeNodes(workflowData: WorkflowDataUpdate) { + const newNodesById = new Map(workflowData.nodes?.map((n) => [n.id, n]) ?? []); + const existingNodesById = new Map(workflowsStore.allNodes.map((n) => [n.id, n])); + + const nodesToUpdate: Array<{ existing: INodeUi; updated: INode }> = []; + const nodesToAdd: INode[] = []; + const nodesToRemove: INodeUi[] = []; + + workflowData.nodes?.forEach((newNode) => { + const existing = existingNodesById.get(newNode.id); + if (existing) { + nodesToUpdate.push({ existing, updated: newNode }); + } else { + nodesToAdd.push(newNode); + } + }); + + existingNodesById.forEach((node, id) => { + if (!newNodesById.has(id)) { + nodesToRemove.push(node); + } + }); + + return { nodesToUpdate, nodesToAdd, nodesToRemove }; + } + + /** + * Update existing nodes in place, handling renames via canvasOperations.renameNode() + * which properly updates pinData, nodeMetadata, runData, etc. + */ + async function updateExistingNodes( + nodesToUpdate: Array<{ existing: INodeUi; updated: INode }>, + ): Promise { + if (nodesToUpdate.length === 0) return; + + // Track successful renames (nodeId -> newName) + const renamedNodes = new Map(); + + // First handle renames via canvasOperations (handles pinData, metadata, etc.) + for (const { existing, updated } of nodesToUpdate) { + if (existing.name !== updated.name) { + const success = await canvasOperations.renameNode(existing.name, updated.name, { + trackHistory: true, + trackBulk: false, + showErrorToast: false, + }); + if (success) { + renamedNodes.set(existing.id, updated.name); + } + } + } + + // Then update other node properties on the (possibly renamed) nodes + const workflow = workflowsStore.cloneWorkflowObject(); + + for (const { existing, updated } of nodesToUpdate) { + // Use new name only if rename succeeded, otherwise use old name + const nodeName = renamedNodes.get(existing.id) ?? existing.name; + const node = workflow.nodes[nodeName]; + if (!node) continue; + + // Update node properties, preserving position and current name + Object.assign(node, { + ...updated, + position: existing.position, + name: nodeName, // Keep actual name (old if rename failed) + }); + + // Mark node as dirty if parameters changed + if (!isEqual(existing.parameters, updated.parameters)) { + workflowState.resetParametersLastUpdatedAt(nodeName); + } + } + + // Sync state back to store + workflowsStore.setNodes(Object.values(workflow.nodes)); + workflowsStore.setConnections(workflow.connectionsBySourceNode); + } + + /** + * Remove stale nodes that are no longer in the update + */ + function removeStaleNodes(nodesToRemove: INodeUi[]): void { + for (const node of nodesToRemove) { + canvasOperations.deleteNode(node.id, { trackHistory: true, trackBulk: false }); + } + } + + /** + * Add new nodes via canvas operations + */ + async function addNewNodes(nodesToAdd: INode[]): Promise { + if (nodesToAdd.length === 0) return []; + + const addedNodes = await canvasOperations.addNodes( + nodesToAdd.map((node) => ({ ...node })), + { + trackHistory: true, + trackBulk: false, + forcePosition: true, + telemetry: false, + }, + ); + + return addedNodes; + } + + /** + * Helper to compare canvas connections + */ + function isSameConnection( + a: { + source: string; + target: string; + sourceHandle?: string | null; + targetHandle?: string | null; + }, + b: { + source: string; + target: string; + sourceHandle?: string | null; + targetHandle?: string | null; + }, + ): boolean { + return ( + a.source === b.source && + a.target === b.target && + (a.sourceHandle ?? null) === (b.sourceHandle ?? null) && + (a.targetHandle ?? null) === (b.targetHandle ?? null) + ); + } + + /** + * Update connections - remove old, add new + */ + async function updateConnections(newConnections: IConnections): Promise { + const existingConnections = workflowsStore.workflow.connections; + + // Convert to canvas format for comparison + const existingCanvasConnections = mapLegacyConnectionsToCanvasConnections( + existingConnections, + workflowsStore.allNodes, + ); + const newCanvasConnections = mapLegacyConnectionsToCanvasConnections( + newConnections, + workflowsStore.allNodes, + ); + + // Find connections to remove (exist in current but not in new) + const connectionsToRemove = existingCanvasConnections.filter( + (existing) => !newCanvasConnections.some((newConn) => isSameConnection(existing, newConn)), + ); + + // Find connections to add (exist in new but not in current) + const connectionsToAdd = newCanvasConnections.filter( + (newConn) => + !existingCanvasConnections.some((existing) => isSameConnection(existing, newConn)), + ); + + // Remove old connections + for (const connection of connectionsToRemove) { + canvasOperations.deleteConnection(connection, { + trackHistory: true, + trackBulk: false, + }); + } + + // Add new connections + if (connectionsToAdd.length > 0) { + await canvasOperations.addConnections(connectionsToAdd, { + trackHistory: true, + trackBulk: false, + }); + } + } + + /** + * Set default credentials on nodes that don't have any (mutates input array). + * Must be called BEFORE nodes are added to store to avoid direct store mutation. + */ + function setDefaultCredentialsOnNodes(nodes: INode[]): void { + for (const node of nodes) { + const hasCredentials = node.credentials && Object.keys(node.credentials).length > 0; + if (hasCredentials) continue; + + const nodeType = nodeTypesStore.getNodeType(node.type); + if (!nodeType?.credentials) continue; + + for (const credentialConfig of nodeType.credentials) { + const credentials = credentialsStore.getCredentialsByType(credentialConfig.name); + if (!credentials || credentials.length === 0) continue; + + const credential = credentials[0]; + node.credentials = { + [credential.type]: { + id: credential.id, + name: credential.name, + }, + }; + + const authField = getMainAuthField(nodeType); + const authType = getAuthTypeForNodeCredential(nodeType, credentialConfig); + if (authField && authType) { + node.parameters[authField.name] = authType.value; + } + + break; + } + } + } + + /** + * Update workflow name if initial generation and name starts with default + */ + function updateWorkflowNameIfNeeded(name?: string, isInitialGeneration?: boolean): void { + if ( + name && + isInitialGeneration && + workflowsStore.workflow.name.startsWith(DEFAULT_NEW_WORKFLOW_NAME) + ) { + workflowState.setWorkflowName({ newName: name, setStateDirty: false }); + } + } + + /** + * Tidy up new node positions + */ + function tidyUpNewNodes(nodeIds: string[]): void { + if (nodeIds.length === 0) { + return; + } + + canvasEventBus.emit('tidyUp', { + source: 'builder-update', + nodeIdsFilter: nodeIds, + trackEvents: false, + trackHistory: true, + trackBulk: false, + }); + } + + /** + * Updates workflow with new data, preserving existing node positions + * and handling renames, additions, and removals efficiently. + * Note: Undo recording should be managed by the caller (e.g., start when streaming begins, stop when it ends). + */ + async function updateWorkflow( + workflowData: WorkflowDataUpdate, + options?: UpdateWorkflowOptions, + ): Promise { + try { + // Apply default credentials to incoming nodes BEFORE adding to store + setDefaultCredentialsOnNodes(workflowData.nodes ?? []); + + const { nodesToUpdate, nodesToAdd, nodesToRemove } = categorizeNodes(workflowData); + + await updateExistingNodes(nodesToUpdate); + removeStaleNodes(nodesToRemove); + const addedNodes = await addNewNodes(nodesToAdd); + const newNodeIds = addedNodes.map((n) => n.id); + await updateConnections(workflowData.connections ?? {}); + updateWorkflowNameIfNeeded(workflowData.name, options?.isInitialGeneration); + + builderStore.setBuilderMadeEdits(true); + + // Combine newly added node IDs with any additional IDs from previous messages + const allNodeIdsToTidyUp = [...newNodeIds, ...(options?.nodeIdsToTidyUp ?? [])]; + tidyUpNewNodes(allNodeIdsToTidyUp); + + return { success: true, newNodeIds }; + } catch (error) { + return { success: false, error }; + } + } + + return { updateWorkflow }; +} diff --git a/packages/frontend/editor-ui/src/app/views/NodeView.vue b/packages/frontend/editor-ui/src/app/views/NodeView.vue index 3409080a67c..6aaceb7592f 100644 --- a/packages/frontend/editor-ui/src/app/views/NodeView.vue +++ b/packages/frontend/editor-ui/src/app/views/NodeView.vue @@ -596,7 +596,14 @@ const allTriggerNodesDisabled = computed(() => { return disabledTriggerNodes.length === triggerNodes.value.length; }); -function onTidyUp(event: CanvasLayoutEvent, options?: { trackEvents?: boolean }) { +function onTidyUp( + event: CanvasLayoutEvent, + options?: { + trackEvents?: boolean; + trackHistory?: boolean; + trackBulk?: boolean; + }, +) { tidyUp(event, options); } diff --git a/packages/frontend/editor-ui/src/features/ai/assistant/builder.store.test.ts b/packages/frontend/editor-ui/src/features/ai/assistant/builder.store.test.ts index 84b95a2e5bf..a34064ddb22 100644 --- a/packages/frontend/editor-ui/src/features/ai/assistant/builder.store.test.ts +++ b/packages/frontend/editor-ui/src/features/ai/assistant/builder.store.test.ts @@ -31,7 +31,6 @@ import { import type { Telemetry } from '@/app/plugins/telemetry'; import type { ChatUI } from '@n8n/design-system/types/assistant'; import type { ChatRequest } from '@/features/ai/assistant/assistant.types'; -import { type INodeTypeDescription } from 'n8n-workflow'; import { mockedStore } from '@/__tests__/utils'; import { useWorkflowsStore } from '@/app/stores/workflows.store'; import { useNodeTypesStore } from '@/app/stores/nodeTypes.store'; @@ -1038,657 +1037,6 @@ describe('AI Builder store', () => { }); }); - describe('applyWorkflowUpdate with workflow naming', () => { - it('should apply generated workflow name during initial generation when workflow has default name', () => { - const builderStore = useBuilderStore(); - - // Set initial generation flag - builderStore.initialGeneration = true; - - // Ensure workflow has default name - workflowsStore.workflow.name = DEFAULT_NEW_WORKFLOW_NAME; - - // Create workflow JSON with a generated name - const workflowJson = JSON.stringify({ - name: 'Generated Workflow Name for Email Processing', - nodes: [ - { - id: 'node1', - name: 'Start', - type: 'n8n-nodes-base.manualTrigger', - position: [250, 300], - parameters: {}, - }, - ], - connections: {}, - }); - - // Apply the workflow update - const result = builderStore.applyWorkflowUpdate(workflowJson); - - // Verify the update was successful - expect(result.success).toBe(true); - - // Verify setWorkflowName was called with the generated name - expect(setWorkflowNameSpy).toHaveBeenCalledWith({ - newName: 'Generated Workflow Name for Email Processing', - setStateDirty: false, - }); - }); - - it('should NOT apply generated workflow name during initial generation when workflow has custom name', () => { - const builderStore = useBuilderStore(); - - // Set initial generation flag - builderStore.initialGeneration = true; - - // Set a custom workflow name (not the default) - workflowsStore.workflow.name = 'My Custom Workflow'; - - // Create workflow JSON with a generated name - const workflowJson = JSON.stringify({ - name: 'Generated Workflow Name for Email Processing', - nodes: [ - { - id: 'node1', - name: 'Start', - type: 'n8n-nodes-base.manualTrigger', - position: [250, 300], - parameters: {}, - }, - ], - connections: {}, - }); - - // Apply the workflow update - const result = builderStore.applyWorkflowUpdate(workflowJson); - - // Verify the update was successful - expect(result.success).toBe(true); - - // Verify setWorkflowName was NOT called - expect(setWorkflowNameSpy).not.toHaveBeenCalled(); - }); - - it('should NOT apply generated workflow name when not initial generation', () => { - const builderStore = useBuilderStore(); - - // Ensure initial generation flag is false - builderStore.initialGeneration = false; - - // Ensure workflow has default name - workflowsStore.workflow.name = DEFAULT_NEW_WORKFLOW_NAME; - - // Create workflow JSON with a generated name - const workflowJson = JSON.stringify({ - name: 'Generated Workflow Name for Email Processing', - nodes: [ - { - id: 'node1', - name: 'Start', - type: 'n8n-nodes-base.manualTrigger', - position: [250, 300], - parameters: {}, - }, - ], - connections: {}, - }); - - // Apply the workflow update - const result = builderStore.applyWorkflowUpdate(workflowJson); - - // Verify the update was successful - expect(result.success).toBe(true); - - // Verify setWorkflowName was NOT called - expect(setWorkflowNameSpy).not.toHaveBeenCalled(); - }); - - it('should handle workflow updates without name property', () => { - const builderStore = useBuilderStore(); - - // Set initial generation flag - builderStore.initialGeneration = true; - - // Ensure workflow has default name - workflowsStore.workflow.name = DEFAULT_NEW_WORKFLOW_NAME; - - // Create workflow JSON without a name property - const workflowJson = JSON.stringify({ - nodes: [ - { - id: 'node1', - name: 'Start', - type: 'n8n-nodes-base.manualTrigger', - position: [250, 300], - parameters: {}, - }, - ], - connections: {}, - }); - - // Apply the workflow update - const result = builderStore.applyWorkflowUpdate(workflowJson); - - // Verify the update was successful - expect(result.success).toBe(true); - - // Verify setWorkflowName was NOT called - expect(setWorkflowNameSpy).not.toHaveBeenCalled(); - }); - - it('should handle workflow names that start with but are not exactly the default name', () => { - const builderStore = useBuilderStore(); - - // Set initial generation flag - builderStore.initialGeneration = true; - - // Set workflow name that starts with default but has more text - workflowsStore.workflow.name = `${DEFAULT_NEW_WORKFLOW_NAME} - Copy`; - - // Create workflow JSON with a generated name - const workflowJson = JSON.stringify({ - name: 'Generated Workflow Name for Email Processing', - nodes: [ - { - id: 'node1', - name: 'Start', - type: 'n8n-nodes-base.manualTrigger', - position: [250, 300], - parameters: {}, - }, - ], - connections: {}, - }); - - // Apply the workflow update - const result = builderStore.applyWorkflowUpdate(workflowJson); - - // Verify the update was successful - expect(result.success).toBe(true); - - // Verify setWorkflowName WAS called because the name starts with default - expect(setWorkflowNameSpy).toHaveBeenCalledWith({ - newName: 'Generated Workflow Name for Email Processing', - setStateDirty: false, - }); - }); - - it('should handle malformed JSON gracefully', () => { - const builderStore = useBuilderStore(); - - // Set initial generation flag - builderStore.initialGeneration = true; - - // Create malformed JSON - const workflowJson = '{ invalid json }'; - - // Apply the workflow update - const result = builderStore.applyWorkflowUpdate(workflowJson); - - // Verify the update failed - expect(result.success).toBe(false); - expect(result.error).toBeDefined(); - }); - - it('should maintain initial generation flag state across multiple updates', () => { - const builderStore = useBuilderStore(); - - // Set initial generation flag - builderStore.initialGeneration = true; - - // Ensure workflow has default name - workflowsStore.workflow.name = DEFAULT_NEW_WORKFLOW_NAME; - - // First update with name - const workflowJson1 = JSON.stringify({ - name: 'First Generated Name', - nodes: [], - connections: {}, - }); - - builderStore.applyWorkflowUpdate(workflowJson1); - expect(setWorkflowNameSpy).toHaveBeenCalledTimes(1); - - // The flag should still be true for subsequent updates in the same generation - expect(builderStore.initialGeneration).toBe(true); - - // Second update without name (simulating further tool operations) - const workflowJson2 = JSON.stringify({ - nodes: [ - { - id: 'node2', - name: 'HTTP', - type: 'n8n-nodes-base.httpRequest', - position: [450, 300], - parameters: {}, - }, - ], - connections: {}, - }); - - builderStore.applyWorkflowUpdate(workflowJson2); - - // Should not call setWorkflowName again - expect(setWorkflowNameSpy).toHaveBeenCalledTimes(1); - }); - - describe('applyWorkflowUpdate credential defaults', () => { - const createTestNodeType = (): INodeTypeDescription => ({ - displayName: 'Test Node', - name: 'n8n-nodes-base.test', - description: 'Test node', - group: ['trigger'], - version: 1, - defaults: { name: 'Test Node' }, - inputs: ['main'], - outputs: ['main'], - properties: [ - { - displayName: 'Authentication', - name: 'authentication', - type: 'options', - options: [ - { - name: 'API Key', - value: 'apiKey', - }, - ], - default: 'apiKey', - required: true, - }, - ], - credentials: [ - { - name: 'testApi', - required: true, - displayOptions: { - show: { - authentication: ['apiKey'], - }, - }, - }, - ], - }); - - it('assigns default credentials when available', () => { - const builderStore = useBuilderStore(); - getNodeTypeSpy.mockReturnValue(createTestNodeType()); - getCredentialsByTypeSpy.mockReturnValue([ - { id: 'cred-id', name: 'API Credential', type: 'testApi' }, - ]); - - const workflowJson = JSON.stringify({ - nodes: [ - { - id: 'node1', - name: 'HTTP Request', - type: 'n8n-nodes-base.test', - position: [0, 0], - parameters: {}, - }, - ], - connections: {}, - }); - - const result = builderStore.applyWorkflowUpdate(workflowJson); - expect(result.success).toBe(true); - const [node] = result.workflowData?.nodes ?? []; - expect(node.credentials).toEqual({ - testApi: { id: 'cred-id', name: 'API Credential' }, - }); - expect(node.parameters.authentication).toBe('apiKey'); - }); - - it('keeps existing credentials untouched', () => { - const builderStore = useBuilderStore(); - getNodeTypeSpy.mockReturnValue(createTestNodeType()); - getCredentialsByTypeSpy.mockReturnValue([ - { id: 'cred-id', name: 'API Credential', type: 'testApi' }, - ]); - - const workflowJson = JSON.stringify({ - nodes: [ - { - id: 'node1', - name: 'HTTP Request', - type: 'n8n-nodes-base.test', - position: [0, 0], - parameters: { authentication: 'apiKey' }, - credentials: { - testApi: { id: 'existing', name: 'Existing Credential' }, - }, - }, - ], - connections: {}, - }); - - const result = builderStore.applyWorkflowUpdate(workflowJson); - expect(result.success).toBe(true); - const [node] = result.workflowData?.nodes ?? []; - expect(node.credentials).toEqual({ - testApi: { id: 'existing', name: 'Existing Credential' }, - }); - }); - }); - }); - - describe('applyWorkflowUpdate with pinned data preservation', () => { - it('should preserve pinned data for nodes with matching names', () => { - const builderStore = useBuilderStore(); - - // Set up initial workflow with nodes that have pinned data - const node1 = { - id: 'node1-id', - name: 'HTTP Request', - type: 'n8n-nodes-base.httpRequest', - typeVersion: 1, - position: [0, 0] as [number, number], - parameters: {}, - }; - const node2 = { - id: 'node2-id', - name: 'Set', - type: 'n8n-nodes-base.set', - position: [0, 0] as [number, number], - typeVersion: 1, - parameters: {}, - }; - - workflowsStore.allNodes = [node1, node2]; - workflowsStore.workflow.nodes = [node1, node2]; - - // Mock pinned data for these nodes - const pinnedData1 = [{ json: { data: 'test1' } }]; - const pinnedData2 = [{ json: { data: 'test2' } }]; - - vi.spyOn(workflowsStore, 'pinDataByNodeName', 'get').mockReturnValue( - vi.fn((nodeName: string) => { - if (nodeName === 'HTTP Request') return pinnedData1; - if (nodeName === 'Set') return pinnedData2; - return undefined; - }), - ); - - // Create workflow update with the same node names but different IDs - const workflowJson = JSON.stringify({ - nodes: [ - { - id: 'new-node1-id', - name: 'HTTP Request', - type: 'n8n-nodes-base.httpRequest', - position: [250, 300] as [number, number], - parameters: {}, - }, - { - id: 'new-node2-id', - name: 'Set', - type: 'n8n-nodes-base.set', - position: [450, 300] as [number, number], - parameters: {}, - }, - ], - connections: {}, - }); - - // Apply the workflow update - const result = builderStore.applyWorkflowUpdate(workflowJson); - - // Verify the update was successful - expect(result.success).toBe(true); - - // Verify pinned data was preserved for nodes with matching names - expect(result.workflowData?.pinData).toEqual({ - 'HTTP Request': pinnedData1, - Set: pinnedData2, - }); - }); - - it('should preserve pinned data only for nodes that still exist', () => { - const builderStore = useBuilderStore(); - - // Set up initial workflow with three nodes - const node1 = { - id: 'node1-id', - name: 'HTTP Request', - type: 'n8n-nodes-base.httpRequest', - typeVersion: 1, - position: [250, 300] as [number, number], - parameters: {}, - }; - const node2 = { - id: 'node2-id', - name: 'Set', - type: 'n8n-nodes-base.set', - typeVersion: 1, - position: [450, 300] as [number, number], - parameters: {}, - }; - const node3 = { - id: 'node3-id', - name: 'Code', - type: 'n8n-nodes-base.code', - typeVersion: 1, - position: [650, 300] as [number, number], - parameters: {}, - }; - - workflowsStore.allNodes = [node1, node2, node3]; - workflowsStore.workflow.nodes = [node1, node2, node3]; - - // Mock pinned data for all three nodes - const pinnedData1 = [{ json: { data: 'test1' } }]; - const pinnedData2 = [{ json: { data: 'test2' } }]; - const pinnedData3 = [{ json: { data: 'test3' } }]; - - vi.spyOn(workflowsStore, 'pinDataByNodeName', 'get').mockReturnValue( - vi.fn((nodeName: string) => { - if (nodeName === 'HTTP Request') return pinnedData1; - if (nodeName === 'Set') return pinnedData2; - if (nodeName === 'Code') return pinnedData3; - return undefined; - }), - ); - - // Create workflow update with only two of the three nodes - const workflowJson = JSON.stringify({ - nodes: [ - { - id: 'new-node1-id', - name: 'HTTP Request', - type: 'n8n-nodes-base.httpRequest', - position: [250, 300] as [number, number], - typeVersion: 1, - parameters: {}, - }, - { - id: 'new-node2-id', - name: 'Set', - type: 'n8n-nodes-base.set', - typeVersion: 1, - position: [450, 300] as [number, number], - parameters: {}, - }, - ], - connections: {}, - }); - - // Apply the workflow update - const result = builderStore.applyWorkflowUpdate(workflowJson); - - // Verify the update was successful - expect(result.success).toBe(true); - - // Verify only pinned data for existing nodes was preserved - expect(result.workflowData?.pinData).toEqual({ - 'HTTP Request': pinnedData1, - Set: pinnedData2, - }); - - // Code node's pinned data should not be included - expect(result.workflowData?.pinData).not.toHaveProperty('Code'); - }); - - it('should not add pinData property if no pinned data exists', () => { - const builderStore = useBuilderStore(); - - // Set up initial workflow without pinned data - const node1 = { - id: 'node1-id', - name: 'HTTP Request', - type: 'n8n-nodes-base.httpRequest', - typeVersion: 1, - position: [250, 300] as [number, number], - parameters: {}, - }; - - workflowsStore.allNodes = [node1]; - workflowsStore.workflow.nodes = [node1]; - - // Mock no pinned data - vi.spyOn(workflowsStore, 'pinDataByNodeName', 'get').mockReturnValue(vi.fn(() => undefined)); - - // Create workflow update - const workflowJson = JSON.stringify({ - nodes: [ - { - id: 'new-node1-id', - name: 'HTTP Request', - type: 'n8n-nodes-base.httpRequest', - typeVersion: 1, - position: [250, 300] as [number, number], - parameters: {}, - }, - ], - connections: {}, - }); - - // Apply the workflow update - const result = builderStore.applyWorkflowUpdate(workflowJson); - - // Verify the update was successful - expect(result.success).toBe(true); - - // Verify pinData property is not added - expect(result.workflowData?.pinData).toBeUndefined(); - }); - - it('should handle nodes with renamed names correctly', () => { - const builderStore = useBuilderStore(); - - // Set up initial workflow - const node1 = { - id: 'node1-id', - name: 'HTTP Request', - type: 'n8n-nodes-base.httpRequest', - typeVersion: 1, - position: [250, 300] as [number, number], - parameters: {}, - }; - - workflowsStore.allNodes = [node1]; - workflowsStore.workflow.nodes = [node1]; - - // Mock pinned data - const pinnedData1 = [{ json: { data: 'test1' } }]; - - vi.spyOn(workflowsStore, 'pinDataByNodeName', 'get').mockReturnValue( - vi.fn((nodeName: string) => { - if (nodeName === 'HTTP Request') return pinnedData1; - return undefined; - }), - ); - - // Create workflow update with renamed node - const workflowJson = JSON.stringify({ - nodes: [ - { - id: 'new-node1-id', - name: 'HTTP Request1', - type: 'n8n-nodes-base.httpRequest', - typeVersion: 1, - position: [250, 300] as [number, number], - parameters: {}, - }, - ], - connections: {}, - }); - - // Apply the workflow update - const result = builderStore.applyWorkflowUpdate(workflowJson); - - // Verify the update was successful - expect(result.success).toBe(true); - - // Verify pinned data was not preserved since the name changed - expect(result.workflowData?.pinData).toBeUndefined(); - }); - - it('should preserve pinned data when adding new nodes', () => { - const builderStore = useBuilderStore(); - - // Set up initial workflow with one node - const node1 = { - id: 'node1-id', - name: 'HTTP Request', - type: 'n8n-nodes-base.httpRequest', - typeVersion: 1, - position: [250, 300] as [number, number], - parameters: {}, - }; - - workflowsStore.allNodes = [node1]; - workflowsStore.workflow.nodes = [node1]; - - // Mock pinned data for the existing node - const pinnedData1 = [{ json: { data: 'test1' } }]; - - vi.spyOn(workflowsStore, 'pinDataByNodeName', 'get').mockReturnValue( - vi.fn((nodeName: string) => { - if (nodeName === 'HTTP Request') return pinnedData1; - return undefined; - }), - ); - - // Create workflow update with existing node plus a new node - const workflowJson = JSON.stringify({ - nodes: [ - { - id: 'new-node1-id', - name: 'HTTP Request', - type: 'n8n-nodes-base.httpRequest', - typeVersion: 1, - position: [250, 300] as [number, number], - parameters: {}, - }, - { - id: 'new-node2-id', - name: 'Set', - type: 'n8n-nodes-base.set', - typeVersion: 1, - position: [450, 300] as [number, number], - parameters: {}, - }, - ], - connections: {}, - }); - - // Apply the workflow update - const result = builderStore.applyWorkflowUpdate(workflowJson); - - // Verify the update was successful - expect(result.success).toBe(true); - - // Verify pinned data was preserved only for the existing node - expect(result.workflowData?.pinData).toEqual({ - 'HTTP Request': pinnedData1, - }); - - // New node should not have pinned data - expect(result.workflowData?.pinData).not.toHaveProperty('Set'); - }); - }); - describe('Credits management', () => { it('should update builder credits correctly', () => { const builderStore = useBuilderStore(); diff --git a/packages/frontend/editor-ui/src/features/ai/assistant/builder.store.ts b/packages/frontend/editor-ui/src/features/ai/assistant/builder.store.ts index ecdc128d084..64f622f1b36 100644 --- a/packages/frontend/editor-ui/src/features/ai/assistant/builder.store.ts +++ b/packages/frontend/editor-ui/src/features/ai/assistant/builder.store.ts @@ -1,5 +1,4 @@ import type { VIEWS } from '@/app/constants'; -import { DEFAULT_NEW_WORKFLOW_NAME } from '@/app/constants'; import { BUILDER_ENABLED_VIEWS } from './constants'; import { STORES } from '@n8n/stores'; import type { ChatUI } from '@n8n/design-system/types/assistant'; @@ -29,14 +28,9 @@ import { } from './builder.utils'; import { useBuilderTodos, type TodosTrackingPayload } from './composables/useBuilderTodos'; import { useRootStore } from '@n8n/stores/useRootStore'; -import type { WorkflowDataUpdate } from '@n8n/rest-api-client/api/workflows'; import pick from 'lodash/pick'; -import { type INodeExecutionData, type ITelemetryTrackProperties, jsonParse } from 'n8n-workflow'; -import { useToast } from '@/app/composables/useToast'; +import { type ITelemetryTrackProperties } from 'n8n-workflow'; import { injectWorkflowState } from '@/app/composables/useWorkflowState'; -import { useNodeTypesStore } from '@/app/stores/nodeTypes.store'; -import { useCredentialsStore } from '@/features/credentials/credentials.store'; -import { getAuthTypeForNodeCredential, getMainAuthField } from '@/app/utils/nodeTypesUtils'; import { stringSizeInBytes } from '@/app/utils/typesUtils'; import { useNDVStore } from '@/features/ndv/shared/ndv.store'; import { dedupe } from 'n8n-workflow'; @@ -127,8 +121,6 @@ export const useBuilderStore = defineStore(STORES.BUILDER, () => { const rootStore = useRootStore(); const workflowsStore = useWorkflowsStore(); const workflowState = injectWorkflowState(); - const credentialsStore = useCredentialsStore(); - const nodeTypesStore = useNodeTypesStore(); const ndvStore = useNDVStore(); const route = useRoute(); const locale = useI18n(); @@ -621,146 +613,11 @@ export const useBuilderStore = defineStore(STORES.BUILDER, () => { } } - function captureCurrentWorkflowState() { - const nodePositions = new Map(); - const existingNodeIds = new Set(); - const pinnedDataByNodeName = new Map(); - - workflowsStore.allNodes.forEach((node) => { - nodePositions.set(node.id, [...node.position]); - existingNodeIds.add(node.id); - - // Capture pinned data by node name - const pinData = workflowsStore.pinDataByNodeName(node.name); - if (pinData) { - pinnedDataByNodeName.set(node.name, pinData); - } - }); - - return { - nodePositions, - existingNodeIds, - pinnedDataByNodeName, - currentWorkflowJson: JSON.stringify(pick(workflowsStore.workflow, ['nodes', 'connections'])), - }; - } - - function setDefaultNodesCredentials(workflowData: WorkflowDataUpdate) { - // Set default credentials for new nodes if available - workflowData.nodes?.forEach((node) => { - const hasCredentials = node.credentials && Object.keys(node.credentials).length > 0; - if (hasCredentials) { - return; - } - - const nodeType = nodeTypesStore.getNodeType(node.type); - if (!nodeType?.credentials) { - return; - } - - // Try to find and set the first available credential - for (const credentialConfig of nodeType.credentials) { - const credentials = credentialsStore.getCredentialsByType(credentialConfig.name); - // No credentials of this type exist, try the next one - if (!credentials || credentials.length === 0) { - continue; - } - - // Found valid credentials - set them and exit the loop - const credential = credentials[0]; - - node.credentials = { - [credential.type]: { - id: credential.id, - name: credential.name, - }, - }; - - const authField = getMainAuthField(nodeType); - const authType = getAuthTypeForNodeCredential(nodeType, credentialConfig); - if (authField && authType) { - node.parameters[authField.name] = authType.value; - } - - break; // Exit loop after setting the first valid credential - } - }); - } - function clearExistingWorkflow() { workflowState.removeAllConnections({ setStateDirty: false }); workflowState.removeAllNodes({ setStateDirty: false, removePinData: true }); } - function applyWorkflowUpdate(workflowJson: string) { - let workflowData: WorkflowDataUpdate; - try { - workflowData = jsonParse(workflowJson); - } catch (error) { - useToast().showMessage({ - type: 'error', - title: locale.baseText('aiAssistant.builder.workflowParsingError.title'), - message: locale.baseText('aiAssistant.builder.workflowParsingError.content'), - }); - return { success: false, error }; - } - - // Capture current state before clearing - const { nodePositions, existingNodeIds, pinnedDataByNodeName } = captureCurrentWorkflowState(); - - clearExistingWorkflow(); - - // For the initial generation, we want to apply auto-generated workflow name - // but only if the workflow has default name - if ( - workflowData.name && - initialGeneration.value && - workflowsStore.workflow.name.startsWith(DEFAULT_NEW_WORKFLOW_NAME) - ) { - workflowState.setWorkflowName({ newName: workflowData.name, setStateDirty: false }); - } - - // Restore positions for nodes that still exist and identify new nodes - const nodesIdsToTidyUp: string[] = []; - if (workflowData.nodes) { - workflowData.nodes = workflowData.nodes.map((node) => { - const savedPosition = nodePositions.get(node.id); - if (savedPosition) { - return { ...node, position: savedPosition }; - } else { - // This is a new node, add it to the tidy up list - nodesIdsToTidyUp.push(node.id); - } - return node; - }); - } - - setDefaultNodesCredentials(workflowData); - - // Restore pinned data for nodes with matching names - const restoredPinData: Record = {}; - workflowData.nodes?.forEach((node) => { - const savedPinData = pinnedDataByNodeName.get(node.name); - if (savedPinData) { - restoredPinData[node.name] = savedPinData; - } - }); - - if (Object.keys(restoredPinData).length > 0) { - workflowData.pinData = restoredPinData; - } - - // Mark that AI Builder made edits (will be reset after save) - aiBuilderMadeEdits.value = true; - - return { - success: true, - workflowData, - newNodeIds: nodesIdsToTidyUp, - oldNodeIds: Array.from(existingNodeIds), - }; - } - function getWorkflowSnapshot() { return JSON.stringify(pick(workflowsStore.workflow, ['nodes', 'connections'])); } @@ -781,6 +638,14 @@ export const useBuilderStore = defineStore(STORES.BUILDER, () => { aiBuilderMadeEdits.value = false; } + /** + * Sets the AI Builder edits flag. + * Called by the useWorkflowUpdate composable when AI Builder makes changes. + */ + function setBuilderMadeEdits(value: boolean): void { + aiBuilderMadeEdits.value = value; + } + function updateBuilderCredits(quota?: number, claimed?: number) { creditsQuota.value = quota; creditsClaimed.value = claimed; @@ -954,7 +819,6 @@ export const useBuilderStore = defineStore(STORES.BUILDER, () => { resetBuilderChat, sendChatMessage, loadSessions, - applyWorkflowUpdate, getWorkflowSnapshot, fetchBuilderCredits, updateBuilderCredits, @@ -963,6 +827,7 @@ export const useBuilderStore = defineStore(STORES.BUILDER, () => { trackWorkflowBuilderJourney, getAiBuilderMadeEdits, resetAiBuilderMadeEdits, + setBuilderMadeEdits, incrementManualExecutionStats, resetManualExecutionStats, // Version management diff --git a/packages/frontend/editor-ui/src/features/ai/assistant/components/Agent/AskAssistantBuild.test.ts b/packages/frontend/editor-ui/src/features/ai/assistant/components/Agent/AskAssistantBuild.test.ts index 1b22b68a497..99eb1651e7c 100644 --- a/packages/frontend/editor-ui/src/features/ai/assistant/components/Agent/AskAssistantBuild.test.ts +++ b/packages/frontend/editor-ui/src/features/ai/assistant/components/Agent/AskAssistantBuild.test.ts @@ -24,6 +24,16 @@ vi.mock('@/app/composables/useWorkflowSaving', () => ({ }), })); +// Mock useWorkflowUpdate composable +const updateWorkflowMock = vi.hoisted(() => + vi.fn().mockResolvedValue({ success: true, newNodeIds: [] }), +); +vi.mock('@/app/composables/useWorkflowUpdate', () => ({ + useWorkflowUpdate: vi.fn().mockReturnValue({ + updateWorkflow: updateWorkflowMock, + }), +})); + // Mock ExecuteMessage component vi.mock('./ExecuteMessage.vue', () => ({ default: defineComponent({ @@ -109,6 +119,7 @@ import { useBuilderStore } from '../../builder.store'; import { mockedStore } from '@/__tests__/utils'; import { STORES } from '@n8n/stores'; import { useWorkflowsStore } from '@/app/stores/workflows.store'; +import { useHistoryStore } from '@/app/stores/history.store'; import type { INodeUi } from '@/Interface'; import { useUsersStore } from '@/features/settings/users/users.store'; import { useCollaborationStore } from '@/features/collaboration/collaboration/collaboration.store'; @@ -187,6 +198,7 @@ describe('AskAssistantBuild', () => { const renderComponent = createComponentRenderer(AskAssistantBuild); let builderStore: ReturnType>; let workflowsStore: ReturnType>; + let historyStore: ReturnType>; let collaborationStore: ReturnType>; beforeAll(() => { @@ -197,6 +209,10 @@ describe('AskAssistantBuild', () => { vi.clearAllMocks(); onDocumentVisibleCallback = null; + // Reset the updateWorkflow mock before each test + updateWorkflowMock.mockReset(); + updateWorkflowMock.mockResolvedValue({ success: true, newNodeIds: [] }); + const pinia = createTestingPinia({ initialState: { [STORES.BUILDER]: { @@ -230,6 +246,7 @@ describe('AskAssistantBuild', () => { setActivePinia(pinia); builderStore = mockedStore(useBuilderStore); workflowsStore = mockedStore(useWorkflowsStore); + historyStore = mockedStore(useHistoryStore); collaborationStore = mockedStore(useCollaborationStore); // Mock collaboration store methods @@ -239,14 +256,15 @@ describe('AskAssistantBuild', () => { builderStore.sendChatMessage = vi.fn(); builderStore.resetBuilderChat = vi.fn(); builderStore.addAssistantMessages = vi.fn(); - builderStore.applyWorkflowUpdate = vi - .fn() - .mockReturnValue({ success: true, workflowData: {}, newNodeIds: [] }); builderStore.getWorkflowSnapshot = vi.fn().mockReturnValue('{}'); builderStore.getRunningTools = vi.fn().mockReturnValue([]); builderStore.workflowMessages = []; builderStore.toolMessages = []; builderStore.workflowPrompt = workflowPrompt; + + // Mock history store actions + historyStore.startRecordingUndo = vi.fn(); + historyStore.stopRecordingUndo = vi.fn(); builderStore.trackingSessionId = 'app_session_id'; workflowsStore.workflowId = 'abc123'; }); @@ -895,9 +913,8 @@ describe('AskAssistantBuild', () => { }); }); - describe('shouldTidyUp logic', () => { - it('should set tidyUp to true when new nodes are added', async () => { - const originalWorkflow = { nodes: [], connections: {} }; + describe('workflow update via useWorkflowUpdate', () => { + it('should call updateWorkflow when workflow-updated message is received', async () => { const newWorkflow = { nodes: [ { @@ -912,15 +929,9 @@ describe('AskAssistantBuild', () => { connections: {}, }; - builderStore.getWorkflowSnapshot.mockReturnValue(JSON.stringify(originalWorkflow)); - builderStore.applyWorkflowUpdate.mockReturnValue({ - success: true, - workflowData: newWorkflow, - newNodeIds: ['new-node-1'], - oldNodeIds: [], - }); + updateWorkflowMock.mockResolvedValue({ success: true, newNodeIds: ['new-node-1'] }); - workflowsStore.$patch({ workflow: originalWorkflow }); + workflowsStore.$patch({ workflow: { nodes: [], connections: {} } }); renderComponent(); @@ -939,84 +950,15 @@ describe('AskAssistantBuild', () => { await flushPromises(); - // Verify importWorkflowData was called with tidyUp: true - expect(nodeViewEventBusEmitMock).toHaveBeenCalledWith('importWorkflowData', { - data: newWorkflow, - tidyUp: true, - nodesIdsToTidyUp: ['new-node-1'], - regenerateIds: false, - trackEvents: false, + // Verify updateWorkflow was called with parsed workflow data + expect(updateWorkflowMock).toHaveBeenCalledWith(newWorkflow, { + isInitialGeneration: false, + nodeIdsToTidyUp: [], }); }); - it('should set tidyUp to false when no new nodes are added (only parameter updates)', async () => { - const existingWorkflow = { - nodes: [ - { - id: 'existing-node', - name: 'HTTP', - type: 'n8n-nodes-base.httpRequest', - position: [0, 0] as [number, number], - typeVersion: 1, - parameters: { url: 'http://old.com' }, - }, - ], - connections: {}, - }; - const updatedWorkflow = { - nodes: [ - { - id: 'existing-node', - name: 'HTTP', - type: 'n8n-nodes-base.httpRequest', - position: [0, 0] as [number, number], - typeVersion: 1, - parameters: { url: 'http://new.com' }, - }, - ], - connections: {}, - }; - - builderStore.getWorkflowSnapshot.mockReturnValue(JSON.stringify(existingWorkflow)); - builderStore.applyWorkflowUpdate.mockReturnValue({ - success: true, - workflowData: updatedWorkflow, - newNodeIds: [], // No new nodes, just parameter update - oldNodeIds: ['existing-node'], - }); - - workflowsStore.$patch({ workflow: existingWorkflow }); - - renderComponent(); - - builderStore.$patch({ streaming: true }); - await flushPromises(); - - // Trigger workflow update without new nodes - builderStore.workflowMessages = [ - { - id: faker.string.uuid(), - role: 'assistant' as const, - type: 'workflow-updated' as const, - codeSnippet: JSON.stringify(updatedWorkflow), - }, - ]; - - await flushPromises(); - - // Verify importWorkflowData was called with tidyUp: false - expect(nodeViewEventBusEmitMock).toHaveBeenCalledWith('importWorkflowData', { - data: updatedWorkflow, - tidyUp: false, - nodesIdsToTidyUp: [], - regenerateIds: false, - trackEvents: false, - }); - }); - - it('should keep tidyUp true once set within the same user message exchange', async () => { - const workflow1 = { nodes: [], connections: {} }; - const workflow2 = { + it('should accumulate new node IDs across multiple workflow updates in same message exchange', async () => { + const workflow1 = { nodes: [ { id: 'node-1', @@ -1029,7 +971,7 @@ describe('AskAssistantBuild', () => { ], connections: {}, }; - const workflow3 = { + const workflow2 = { nodes: [ { id: 'node-1', @@ -1037,31 +979,27 @@ describe('AskAssistantBuild', () => { type: 'n8n-nodes-base.manualTrigger', position: [0, 0] as [number, number], typeVersion: 1, - parameters: { updated: true }, + parameters: {}, + }, + { + id: 'node-2', + name: 'HTTP', + type: 'n8n-nodes-base.httpRequest', + position: [100, 0] as [number, number], + typeVersion: 1, + parameters: {}, }, ], connections: {}, }; - builderStore.getWorkflowSnapshot.mockReturnValue(JSON.stringify(workflow1)); + // First update adds node-1 + updateWorkflowMock + .mockResolvedValueOnce({ success: true, newNodeIds: ['node-1'] }) + // Second update adds node-2 + .mockResolvedValueOnce({ success: true, newNodeIds: ['node-2'] }); - // First update adds new nodes - builderStore.applyWorkflowUpdate - .mockReturnValueOnce({ - success: true, - workflowData: workflow2, - newNodeIds: ['node-1'], - oldNodeIds: [], - }) - // Second update has no new nodes (just parameter change) - .mockReturnValueOnce({ - success: true, - workflowData: workflow3, - newNodeIds: [], - oldNodeIds: ['node-1'], - }); - - workflowsStore.$patch({ workflow: workflow1 }); + workflowsStore.$patch({ workflow: { nodes: [], connections: {} } }); renderComponent(); @@ -1071,58 +1009,50 @@ describe('AskAssistantBuild', () => { const msgId1 = faker.string.uuid(); const msgId2 = faker.string.uuid(); - // First workflow update (adds new nodes) + // First workflow update builderStore.workflowMessages = [ { id: msgId1, role: 'assistant' as const, type: 'workflow-updated' as const, - codeSnippet: JSON.stringify(workflow2), + codeSnippet: JSON.stringify(workflow1), }, ]; await flushPromises(); - // First call should have tidyUp: true - expect(nodeViewEventBusEmitMock).toHaveBeenCalledWith('importWorkflowData', { - data: workflow2, - tidyUp: true, - nodesIdsToTidyUp: ['node-1'], - regenerateIds: false, - trackEvents: false, + // First call should have empty nodeIdsToTidyUp + expect(updateWorkflowMock).toHaveBeenCalledWith(workflow1, { + isInitialGeneration: false, + nodeIdsToTidyUp: [], }); - nodeViewEventBusEmitMock.mockClear(); - - // Second workflow update (no new nodes, just parameter update) + // Second workflow update builderStore.workflowMessages = [ { id: msgId1, role: 'assistant' as const, type: 'workflow-updated' as const, - codeSnippet: JSON.stringify(workflow2), + codeSnippet: JSON.stringify(workflow1), }, { id: msgId2, role: 'assistant' as const, type: 'workflow-updated' as const, - codeSnippet: JSON.stringify(workflow3), + codeSnippet: JSON.stringify(workflow2), }, ]; await flushPromises(); - // Second call should still have tidyUp: true because it was set earlier - expect(nodeViewEventBusEmitMock).toHaveBeenCalledWith('importWorkflowData', { - data: workflow3, - tidyUp: true, - nodesIdsToTidyUp: [], - regenerateIds: false, - trackEvents: false, + // Second call should have accumulated node IDs from first call + expect(updateWorkflowMock).toHaveBeenLastCalledWith(workflow2, { + isInitialGeneration: false, + nodeIdsToTidyUp: ['node-1'], }); }); - it('should reset shouldTidyUp flag on new user message', async () => { + it('should reset accumulated node IDs on new user message', async () => { workflowsStore.$patch({ workflow: { nodes: [], connections: {} } }); workflowsStore.$patch({ workflowsById: { abc123: { id: 'abc123' } } }); @@ -1142,15 +1072,9 @@ describe('AskAssistantBuild', () => { connections: {}, }; - builderStore.getWorkflowSnapshot.mockReturnValue('{}'); - builderStore.applyWorkflowUpdate.mockReturnValue({ - success: true, - workflowData: workflow1, - newNodeIds: ['node-1'], - oldNodeIds: [], - }); + updateWorkflowMock.mockResolvedValue({ success: true, newNodeIds: ['node-1'] }); - // First message exchange - adds nodes + // First message exchange builderStore.$patch({ streaming: true }); await flushPromises(); @@ -1165,27 +1089,19 @@ describe('AskAssistantBuild', () => { await flushPromises(); - expect(nodeViewEventBusEmitMock).toHaveBeenCalledWith('importWorkflowData', { - data: workflow1, - tidyUp: true, - nodesIdsToTidyUp: ['node-1'], - regenerateIds: false, - trackEvents: false, - }); - builderStore.$patch({ streaming: false }); await flushPromises(); - nodeViewEventBusEmitMock.mockClear(); + updateWorkflowMock.mockClear(); - // Second message - user sends new message, which should reset shouldTidyUp + // User sends new message - should reset accumulated node IDs const vm = (container.firstElementChild as VueComponentInstance)?.__vueParentComponent; if (vm?.setupState?.onUserMessage) { await vm.setupState.onUserMessage('Update parameters'); } await flushPromises(); - // Now simulate a workflow update with no new nodes + // New workflow update with no new nodes const workflow2 = { nodes: [ { @@ -1200,12 +1116,7 @@ describe('AskAssistantBuild', () => { connections: {}, }; - builderStore.applyWorkflowUpdate.mockReturnValue({ - success: true, - workflowData: workflow2, - newNodeIds: [], // No new nodes this time - oldNodeIds: ['node-1'], - }); + updateWorkflowMock.mockResolvedValue({ success: true, newNodeIds: [] }); builderStore.$patch({ streaming: true }); await flushPromises(); @@ -1221,15 +1132,303 @@ describe('AskAssistantBuild', () => { await flushPromises(); - // shouldTidyUp should be reset, so tidyUp should be false - expect(nodeViewEventBusEmitMock).toHaveBeenCalledWith('importWorkflowData', { - data: workflow2, - tidyUp: false, - nodesIdsToTidyUp: [], - regenerateIds: false, - trackEvents: false, + // nodeIdsToTidyUp should be reset (empty), not containing 'node-1' from previous exchange + expect(updateWorkflowMock).toHaveBeenCalledWith(workflow2, { + isInitialGeneration: false, + nodeIdsToTidyUp: [], }); }); + + it('should pass isInitialGeneration flag from builder store', async () => { + const newWorkflow = { + nodes: [ + { + id: 'new-node-1', + name: 'Start', + type: 'n8n-nodes-base.manualTrigger', + position: [0, 0] as [number, number], + typeVersion: 1, + parameters: {}, + }, + ], + connections: {}, + }; + + updateWorkflowMock.mockResolvedValue({ success: true, newNodeIds: ['new-node-1'] }); + + workflowsStore.$patch({ workflow: { nodes: [], connections: {} } }); + builderStore.initialGeneration = true; + + renderComponent(); + + builderStore.$patch({ streaming: true }); + await flushPromises(); + + builderStore.workflowMessages = [ + { + id: faker.string.uuid(), + role: 'assistant' as const, + type: 'workflow-updated' as const, + codeSnippet: JSON.stringify(newWorkflow), + }, + ]; + + await flushPromises(); + + expect(updateWorkflowMock).toHaveBeenCalledWith(newWorkflow, { + isInitialGeneration: true, + nodeIdsToTidyUp: [], + }); + }); + + it('should not process same message twice', async () => { + const newWorkflow = { + nodes: [ + { + id: 'new-node-1', + name: 'Start', + type: 'n8n-nodes-base.manualTrigger', + position: [0, 0] as [number, number], + typeVersion: 1, + parameters: {}, + }, + ], + connections: {}, + }; + + updateWorkflowMock.mockResolvedValue({ success: true, newNodeIds: ['new-node-1'] }); + + renderComponent(); + + builderStore.$patch({ streaming: true }); + await flushPromises(); + + const msgId = faker.string.uuid(); + + // Same message added twice + builderStore.workflowMessages = [ + { + id: msgId, + role: 'assistant' as const, + type: 'workflow-updated' as const, + codeSnippet: JSON.stringify(newWorkflow), + }, + ]; + + await flushPromises(); + + // Trigger watcher again with same message + builderStore.workflowMessages = [ + { + id: msgId, + role: 'assistant' as const, + type: 'workflow-updated' as const, + codeSnippet: JSON.stringify(newWorkflow), + }, + ]; + + await flushPromises(); + + // Should only be called once + expect(updateWorkflowMock).toHaveBeenCalledTimes(1); + }); + + it('should abort streaming when updateWorkflow returns an error', async () => { + const testError = new Error('Failed to update workflow'); + updateWorkflowMock.mockResolvedValue({ success: false, error: testError }); + + workflowsStore.$patch({ workflow: { nodes: [], connections: {} } }); + + renderComponent(); + + builderStore.$patch({ streaming: true }); + await flushPromises(); + + builderStore.workflowMessages = [ + { + id: faker.string.uuid(), + role: 'assistant' as const, + type: 'workflow-updated' as const, + codeSnippet: JSON.stringify({ + nodes: [ + { + id: 'node-1', + name: 'Node', + type: 'n8n-nodes-base.httpRequest', + position: [0, 0], + typeVersion: 1, + parameters: {}, + }, + ], + connections: {}, + }), + }, + ]; + + await flushPromises(); + + expect(builderStore.abortStreaming).toHaveBeenCalled(); + }); + + it('should not accumulate node IDs after error occurs', async () => { + // First update succeeds and adds a node + updateWorkflowMock + .mockResolvedValueOnce({ success: true, newNodeIds: ['node-1'] }) + // Second update fails + .mockResolvedValueOnce({ success: false, error: new Error('Failed') }); + + workflowsStore.$patch({ workflow: { nodes: [], connections: {} } }); + + renderComponent(); + + builderStore.$patch({ streaming: true }); + await flushPromises(); + + const msgId1 = faker.string.uuid(); + const msgId2 = faker.string.uuid(); + + // First message - should succeed + builderStore.workflowMessages = [ + { + id: msgId1, + role: 'assistant' as const, + type: 'workflow-updated' as const, + codeSnippet: JSON.stringify({ nodes: [], connections: {} }), + }, + ]; + + await flushPromises(); + + // Second message - should fail and abort + builderStore.workflowMessages = [ + { + id: msgId1, + role: 'assistant' as const, + type: 'workflow-updated' as const, + codeSnippet: JSON.stringify({ nodes: [], connections: {} }), + }, + { + id: msgId2, + role: 'assistant' as const, + type: 'workflow-updated' as const, + codeSnippet: JSON.stringify({ nodes: [], connections: {} }), + }, + ]; + + await flushPromises(); + + // First call should have empty nodeIdsToTidyUp, second should have accumulated 'node-1' + expect(updateWorkflowMock).toHaveBeenNthCalledWith(1, expect.anything(), { + isInitialGeneration: false, + nodeIdsToTidyUp: [], + }); + expect(updateWorkflowMock).toHaveBeenNthCalledWith(2, expect.anything(), { + isInitialGeneration: false, + nodeIdsToTidyUp: ['node-1'], + }); + + // Should abort after error + expect(builderStore.abortStreaming).toHaveBeenCalled(); + }); + + it('should not process remaining messages after error occurs', async () => { + const testError = new Error('Failed to update workflow'); + updateWorkflowMock.mockResolvedValue({ success: false, error: testError }); + + workflowsStore.$patch({ workflow: { nodes: [], connections: {} } }); + + renderComponent(); + + builderStore.$patch({ streaming: true }); + await flushPromises(); + + // Add multiple messages at once + builderStore.workflowMessages = [ + { + id: faker.string.uuid(), + role: 'assistant' as const, + type: 'workflow-updated' as const, + codeSnippet: JSON.stringify({ nodes: [], connections: {} }), + }, + { + id: faker.string.uuid(), + role: 'assistant' as const, + type: 'workflow-updated' as const, + codeSnippet: JSON.stringify({ nodes: [], connections: {} }), + }, + { + id: faker.string.uuid(), + role: 'assistant' as const, + type: 'workflow-updated' as const, + codeSnippet: JSON.stringify({ nodes: [], connections: {} }), + }, + ]; + + await flushPromises(); + + // Should only be called once because error stops processing + expect(updateWorkflowMock).toHaveBeenCalledTimes(1); + expect(builderStore.abortStreaming).toHaveBeenCalled(); + }); + }); + + describe('undo recording management', () => { + it('should start undo recording when streaming begins', async () => { + renderComponent(); + + // Streaming starts + builderStore.$patch({ streaming: true }); + await flushPromises(); + + expect(historyStore.startRecordingUndo).toHaveBeenCalledTimes(1); + }); + + it('should stop undo recording when streaming ends', async () => { + renderComponent(); + + // Start streaming + builderStore.$patch({ streaming: true }); + await flushPromises(); + + // Stop streaming + builderStore.$patch({ streaming: false }); + await flushPromises(); + + expect(historyStore.stopRecordingUndo).toHaveBeenCalledTimes(1); + }); + + it('should not call start/stop if streaming state does not change', async () => { + builderStore.$patch({ streaming: false }); + + renderComponent(); + await flushPromises(); + + // Patch with same value + builderStore.$patch({ streaming: false }); + await flushPromises(); + + expect(historyStore.startRecordingUndo).not.toHaveBeenCalled(); + expect(historyStore.stopRecordingUndo).not.toHaveBeenCalled(); + }); + + it('should handle multiple streaming cycles correctly', async () => { + renderComponent(); + + // First cycle + builderStore.$patch({ streaming: true }); + await flushPromises(); + builderStore.$patch({ streaming: false }); + await flushPromises(); + + // Second cycle + builderStore.$patch({ streaming: true }); + await flushPromises(); + builderStore.$patch({ streaming: false }); + await flushPromises(); + + expect(historyStore.startRecordingUndo).toHaveBeenCalledTimes(2); + expect(historyStore.stopRecordingUndo).toHaveBeenCalledTimes(2); + }); }); describe('document visibility handling', () => { diff --git a/packages/frontend/editor-ui/src/features/ai/assistant/components/Agent/AskAssistantBuild.vue b/packages/frontend/editor-ui/src/features/ai/assistant/components/Agent/AskAssistantBuild.vue index c6979aa91ca..b4f8ff58b37 100644 --- a/packages/frontend/editor-ui/src/features/ai/assistant/components/Agent/AskAssistantBuild.vue +++ b/packages/frontend/editor-ui/src/features/ai/assistant/components/Agent/AskAssistantBuild.vue @@ -2,6 +2,7 @@ import { useBuilderStore } from '../../builder.store'; import { useUsersStore } from '@/features/settings/users/users.store'; import { useWorkflowHistoryStore } from '@/features/workflows/workflowHistory/workflowHistory.store'; +import { useHistoryStore } from '@/app/stores/history.store'; import { useCollaborationStore } from '@/features/collaboration/collaboration/collaboration.store'; import { useWorkflowAutosaveStore } from '@/app/stores/workflowAutosave.store'; import { AutoSaveState } from '@/app/constants'; @@ -19,6 +20,10 @@ import { useToast } from '@/app/composables/useToast'; import { useDocumentVisibility } from '@/app/composables/useDocumentVisibility'; import { WORKFLOW_SUGGESTIONS } from '@/app/constants/workflowSuggestions'; import { VIEWS } from '@/app/constants'; +import { useWorkflowUpdate } from '@/app/composables/useWorkflowUpdate'; +import { useErrorHandler } from '@/app/composables/useErrorHandler'; +import type { WorkflowDataUpdate } from '@n8n/rest-api-client/api/workflows'; +import { jsonParse } from 'n8n-workflow'; import shuffle from 'lodash/shuffle'; import { N8nAskAssistantChat, N8nText } from '@n8n/design-system'; @@ -30,6 +35,7 @@ const emit = defineEmits<{ const builderStore = useBuilderStore(); const usersStore = useUsersStore(); const workflowHistoryStore = useWorkflowHistoryStore(); +const historyStore = useHistoryStore(); const collaborationStore = useCollaborationStore(); const workflowAutosaveStore = useWorkflowAutosaveStore(); const telemetry = useTelemetry(); @@ -39,15 +45,20 @@ const i18n = useI18n(); const route = useRoute(); const { goToUpgrade } = usePageRedirectionHelper(); const toast = useToast(); +const { updateWorkflow } = useWorkflowUpdate(); +const { handleError } = useErrorHandler({ + source: 'ai-builder', + titleKey: 'aiAssistant.builder.error.title', +}); const { onDocumentVisible } = useDocumentVisibility(); onDocumentVisible(() => { builderStore.clearDoneIndicatorTitle(); }); -// Track processed workflow updates +// Track processed workflow updates and accumulated node IDs for tidyUp const processedWorkflowUpdates = ref(new Set()); -const shouldTidyUp = ref(false); +const accumulatedNodeIdsToTidyUp = ref([]); const n8nChatRef = ref>(); const user = computed(() => ({ @@ -126,8 +137,8 @@ async function onUserMessage(content: string) { // Record activity to maintain write lock while building collaborationStore.requestWriteAccess(); - // Reset tidy up flag for each new message exchange - shouldTidyUp.value = false; + // Reset accumulated node IDs for each new message exchange + accumulatedNodeIdsToTidyUp.value = []; // If the workflow is empty, set the initial generation flag const isInitialGeneration = workflowsStore.workflow.nodes.length === 0; @@ -141,7 +152,7 @@ async function onUserMessage(content: string) { function onNewWorkflow() { builderStore.resetBuilderChat(); processedWorkflowUpdates.value.clear(); - shouldTidyUp.value = false; + accumulatedNodeIdsToTidyUp.value = []; } function onFeedback(feedback: RatingFeedback) { @@ -211,40 +222,68 @@ async function onWorkflowExecuted() { }); } +function parseWorkflowJson(workflowJson: string): WorkflowDataUpdate | undefined { + try { + return jsonParse(workflowJson); + } catch (error) { + handleError(error, { + context: 'workflow-json-parse', + title: i18n.baseText('aiAssistant.builder.workflowParsingError.title'), + message: i18n.baseText('aiAssistant.builder.workflowParsingError.content'), + }); + return undefined; + } +} + // Watch for workflow updates and apply them watch( () => builderStore.workflowMessages, - (messages) => { - messages - .filter((msg) => { - return msg.id && !processedWorkflowUpdates.value.has(msg.id); - }) - .forEach((msg) => { - if (msg.id && isWorkflowUpdatedMessage(msg)) { - processedWorkflowUpdates.value.add(msg.id); + async (messages) => { + for (const msg of messages) { + if (!msg.id || processedWorkflowUpdates.value.has(msg.id)) continue; + if (!isWorkflowUpdatedMessage(msg)) continue; - const result = builderStore.applyWorkflowUpdate(msg.codeSnippet); + processedWorkflowUpdates.value.add(msg.id); - if (result.success) { - // Only tidy up if new nodes are added per user message - const hasNewNodes = Boolean(result.newNodeIds && result.newNodeIds.length > 0); - shouldTidyUp.value = shouldTidyUp.value || hasNewNodes; + const workflowData = parseWorkflowJson(msg.codeSnippet); + if (!workflowData) continue; - // Import the updated workflow - nodeViewEventBus.emit('importWorkflowData', { - data: result.workflowData, - tidyUp: shouldTidyUp.value, - nodesIdsToTidyUp: result.newNodeIds, - regenerateIds: false, - trackEvents: false, - }); - } - } + const result = await updateWorkflow(workflowData, { + isInitialGeneration: builderStore.initialGeneration, + nodeIdsToTidyUp: accumulatedNodeIdsToTidyUp.value, }); + + if (!result.success) { + handleError(result.error, { context: 'workflow-update' }); + builderStore.abortStreaming(); + return; + } + + // Accumulate new node IDs so subsequent messages tidy up all new nodes + if (result.newNodeIds.length > 0) { + accumulatedNodeIdsToTidyUp.value = [ + ...accumulatedNodeIdsToTidyUp.value, + ...result.newNodeIds, + ]; + } + } }, { deep: true }, ); +// Manage undo recording based on streaming state - start when streaming begins, stop when it ends +// This ensures all builder changes during a streaming session can be undone at once +watch( + () => builderStore.streaming, + (isStreaming, wasStreaming) => { + if (isStreaming && !wasStreaming) { + historyStore.startRecordingUndo(); + } else if (!isStreaming && wasStreaming) { + historyStore.stopRecordingUndo(); + } + }, +); + // Reset initial generation flag when streaming ends // Note: Saving is handled by auto-save in NodeView.vue watch( diff --git a/packages/frontend/editor-ui/src/features/workflows/canvas/canvas.types.ts b/packages/frontend/editor-ui/src/features/workflows/canvas/canvas.types.ts index 877bd2aa1f8..1460c257e8c 100644 --- a/packages/frontend/editor-ui/src/features/workflows/canvas/canvas.types.ts +++ b/packages/frontend/editor-ui/src/features/workflows/canvas/canvas.types.ts @@ -186,7 +186,13 @@ export type CanvasEventBusEvents = { action: keyof CanvasNodeEventBusEvents; payload?: CanvasNodeEventBusEvents[keyof CanvasNodeEventBusEvents]; }; - tidyUp: { source: CanvasLayoutSource; nodeIdsFilter?: string[]; trackEvents?: boolean }; + tidyUp: { + source: CanvasLayoutSource; + nodeIdsFilter?: string[]; + trackEvents?: boolean; + trackHistory?: boolean; + trackBulk?: boolean; + }; 'create:sticky': never; }; diff --git a/packages/frontend/editor-ui/src/features/workflows/canvas/components/Canvas.vue b/packages/frontend/editor-ui/src/features/workflows/canvas/components/Canvas.vue index c1c2caeba42..ad0ad1daeb4 100644 --- a/packages/frontend/editor-ui/src/features/workflows/canvas/components/Canvas.vue +++ b/packages/frontend/editor-ui/src/features/workflows/canvas/components/Canvas.vue @@ -111,7 +111,14 @@ const emit = defineEmits<{ 'run:workflow': []; 'create:workflow': []; 'drag-and-drop': [position: XYPosition, event: DragEvent]; - 'tidy-up': [CanvasLayoutEvent, { trackEvents?: boolean }]; + 'tidy-up': [ + CanvasLayoutEvent, + { + trackEvents?: boolean; + trackHistory?: boolean; + trackBulk?: boolean; + }, + ]; 'toggle:focus-panel': []; 'viewport:change': [viewport: ViewportTransform, dimensions: Dimensions]; 'selection:end': [position: XYPosition]; @@ -787,7 +794,15 @@ async function onTidyUp(payload: CanvasEventBusEvents['tidyUp']) { const target = applyOnSelection ? 'selection' : 'all'; const result = layout(target); - emit('tidy-up', { result, target, source: payload.source }, { trackEvents: payload.trackEvents }); + emit( + 'tidy-up', + { result, target, source: payload.source }, + { + trackEvents: payload.trackEvents, + trackHistory: payload.trackHistory, + trackBulk: payload.trackBulk, + }, + ); await nextTick(); if (applyOnSelection) { diff --git a/packages/frontend/editor-ui/src/features/workflows/canvas/composables/useCanvasLayout.ts b/packages/frontend/editor-ui/src/features/workflows/canvas/composables/useCanvasLayout.ts index 5a1573e976a..4001ff6a1d9 100644 --- a/packages/frontend/editor-ui/src/features/workflows/canvas/composables/useCanvasLayout.ts +++ b/packages/frontend/editor-ui/src/features/workflows/canvas/composables/useCanvasLayout.ts @@ -18,7 +18,8 @@ export type CanvasLayoutSource = | 'canvas-button' | 'context-menu' | 'command-bar' - | 'import-workflow-data'; + | 'import-workflow-data' + | 'builder-update'; export type CanvasLayoutTargetData = { nodes: Array>; edges: CanvasConnection[];