diff --git a/src/services/workflow-diff-engine.ts b/src/services/workflow-diff-engine.ts index f4b659c..738dc1f 100644 --- a/src/services/workflow-diff-engine.ts +++ b/src/services/workflow-diff-engine.ts @@ -523,8 +523,10 @@ export class WorkflowDiffEngine { return `"To" node not found: "${operation.to}". Available nodes: ${availableNodes}. Tip: Use node ID for names with special characters.`; } + // Resolve smart parameters (branch, case) before validating connections + const { sourceOutput } = this.resolveSmartParameters(workflow, operation); + // Validate that connection from source to "from" exists - const sourceOutput = operation.sourceOutput || 'main'; const connections = workflow.connections[sourceNode.name]?.[sourceOutput]; if (!connections) { return `No connections found from "${sourceNode.name}" on output "${sourceOutput}"`; @@ -631,16 +633,48 @@ export class WorkflowDiffEngine { node.disabled = true; } + /** + * Resolve smart parameters (branch, case) to technical parameters + * Phase 1 UX improvement: Semantic parameters for multi-output nodes + */ + private resolveSmartParameters( + workflow: Workflow, + operation: AddConnectionOperation | RewireConnectionOperation + ): { sourceOutput: string; sourceIndex: number } { + const sourceNode = this.findNode(workflow, operation.source, operation.source); + + // Start with explicit values or defaults + let sourceOutput = operation.sourceOutput ?? 'main'; + let sourceIndex = operation.sourceIndex ?? 0; + + // Smart parameter: branch (for IF nodes) + if (operation.branch && !operation.sourceOutput) { + // Only apply if sourceOutput not explicitly set + if (sourceNode?.type === 'n8n-nodes-base.if') { + sourceOutput = operation.branch; // 'true' or 'false' + } + } + + // Smart parameter: case (for Switch nodes) + if (operation.case !== undefined && operation.sourceIndex === undefined) { + // Only apply if sourceIndex not explicitly set + sourceIndex = operation.case; + } + + return { sourceOutput, sourceIndex }; + } + // Connection operation appliers private applyAddConnection(workflow: Workflow, operation: AddConnectionOperation): void { const sourceNode = this.findNode(workflow, operation.source, operation.source); const targetNode = this.findNode(workflow, operation.target, operation.target); if (!sourceNode || !targetNode) return; + // Resolve smart parameters (branch, case) to technical parameters + const { sourceOutput, sourceIndex } = this.resolveSmartParameters(workflow, operation); + // Use nullish coalescing to properly handle explicit 0 values - const sourceOutput = operation.sourceOutput ?? 'main'; const targetInput = operation.targetInput ?? 'main'; - const sourceIndex = operation.sourceIndex ?? 0; const targetIndex = operation.targetIndex ?? 0; // Initialize source node connections object @@ -763,12 +797,15 @@ export class WorkflowDiffEngine { * @param operation - Rewire operation specifying source, from, and to */ private applyRewireConnection(workflow: Workflow, operation: RewireConnectionOperation): void { + // Resolve smart parameters (branch, case) to technical parameters + const { sourceOutput, sourceIndex } = this.resolveSmartParameters(workflow, operation); + // First, remove the old connection (source → from) this.applyRemoveConnection(workflow, { type: 'removeConnection', source: operation.source, target: operation.from, - sourceOutput: operation.sourceOutput, + sourceOutput: sourceOutput, targetInput: operation.targetInput }); @@ -777,9 +814,9 @@ export class WorkflowDiffEngine { type: 'addConnection', source: operation.source, target: operation.to, - sourceOutput: operation.sourceOutput, + sourceOutput: sourceOutput, targetInput: operation.targetInput, - sourceIndex: operation.sourceIndex, + sourceIndex: sourceIndex, targetIndex: 0 // Default target index for new connection }); } diff --git a/src/types/workflow-diff.ts b/src/types/workflow-diff.ts index 5890e87..647f3ca 100644 --- a/src/types/workflow-diff.ts +++ b/src/types/workflow-diff.ts @@ -64,6 +64,9 @@ export interface AddConnectionOperation extends DiffOperation { targetInput?: string; // Default: 'main' sourceIndex?: number; // Default: 0 targetIndex?: number; // Default: 0 + // Smart parameters for multi-output nodes (Phase 1 UX improvement) + branch?: 'true' | 'false'; // For IF nodes: maps to sourceOutput + case?: number; // For Switch nodes: maps to sourceIndex } export interface RemoveConnectionOperation extends DiffOperation { @@ -95,6 +98,9 @@ export interface RewireConnectionOperation extends DiffOperation { sourceOutput?: string; // Optional: which output to rewire (default: 'main') targetInput?: string; // Optional: which input type (default: 'main') sourceIndex?: number; // Optional: which source index (default: 0) + // Smart parameters for multi-output nodes (Phase 1 UX improvement) + branch?: 'true' | 'false'; // For IF nodes: maps to sourceOutput + case?: number; // For Switch nodes: maps to sourceIndex } // Workflow Metadata Operations diff --git a/tests/unit/services/workflow-diff-engine.test.ts b/tests/unit/services/workflow-diff-engine.test.ts index 4baea0e..9fd3c0d 100644 --- a/tests/unit/services/workflow-diff-engine.test.ts +++ b/tests/unit/services/workflow-diff-engine.test.ts @@ -1144,6 +1144,369 @@ describe('WorkflowDiffEngine', () => { }); }); + describe('Smart Parameters (Phase 1)', () => { + it('should use branch="true" for IF node connections', async () => { + // Add IF node + const addIF: any = { + type: 'addNode', + node: { + name: 'IF', + type: 'n8n-nodes-base.if', + position: [400, 300] + } + }; + + // Add TrueHandler node (use unique name) + const addTrueHandler: any = { + type: 'addNode', + node: { + name: 'TrueHandler', + type: 'n8n-nodes-base.set', + position: [600, 300] + } + }; + + // Connect IF to TrueHandler using smart branch parameter + const connectWithBranch: any = { + type: 'addConnection', + source: 'IF', + target: 'TrueHandler', + branch: 'true' // Smart parameter instead of sourceOutput: 'true' + }; + + const request: WorkflowDiffRequest = { + id: 'test-workflow', + operations: [addIF, addTrueHandler, connectWithBranch] + }; + + const result = await diffEngine.applyDiff(baseWorkflow, request); + + expect(result.success).toBe(true); + expect(result.workflow).toBeDefined(); + + // Should create connection on 'true' output + expect(result.workflow!.connections['IF']['true']).toBeDefined(); + expect(result.workflow!.connections['IF']['true'][0][0].node).toBe('TrueHandler'); + }); + + it('should use branch="false" for IF node connections', async () => { + const addIF: any = { + type: 'addNode', + node: { + name: 'IF', + type: 'n8n-nodes-base.if', + position: [400, 300] + } + }; + + const addFalseHandler: any = { + type: 'addNode', + node: { + name: 'FalseHandler', + type: 'n8n-nodes-base.set', + position: [600, 300] + } + }; + + const connectWithBranch: any = { + type: 'addConnection', + source: 'IF', + target: 'FalseHandler', + branch: 'false' // Smart parameter for false branch + }; + + const request: WorkflowDiffRequest = { + id: 'test-workflow', + operations: [addIF, addFalseHandler, connectWithBranch] + }; + + const result = await diffEngine.applyDiff(baseWorkflow, request); + + expect(result.success).toBe(true); + + // Should create connection on 'false' output + expect(result.workflow!.connections['IF']['false']).toBeDefined(); + expect(result.workflow!.connections['IF']['false'][0][0].node).toBe('FalseHandler'); + }); + + it('should use case parameter for Switch node connections', async () => { + // Add Switch node + const addSwitch: any = { + type: 'addNode', + node: { + name: 'Switch', + type: 'n8n-nodes-base.switch', + position: [400, 300] + } + }; + + // Add handler nodes + const addCase0: any = { + type: 'addNode', + node: { + name: 'Case0Handler', + type: 'n8n-nodes-base.set', + position: [600, 200] + } + }; + + const addCase1: any = { + type: 'addNode', + node: { + name: 'Case1Handler', + type: 'n8n-nodes-base.set', + position: [600, 300] + } + }; + + const addCase2: any = { + type: 'addNode', + node: { + name: 'Case2Handler', + type: 'n8n-nodes-base.set', + position: [600, 400] + } + }; + + // Connect using case parameter + const connectCase0: any = { + type: 'addConnection', + source: 'Switch', + target: 'Case0Handler', + case: 0 // Smart parameter instead of sourceIndex: 0 + }; + + const connectCase1: any = { + type: 'addConnection', + source: 'Switch', + target: 'Case1Handler', + case: 1 // Smart parameter instead of sourceIndex: 1 + }; + + const connectCase2: any = { + type: 'addConnection', + source: 'Switch', + target: 'Case2Handler', + case: 2 // Smart parameter instead of sourceIndex: 2 + }; + + const request: WorkflowDiffRequest = { + id: 'test-workflow', + operations: [addSwitch, addCase0, addCase1, addCase2, connectCase0, connectCase1, connectCase2] + }; + + const result = await diffEngine.applyDiff(baseWorkflow, request); + + expect(result.success).toBe(true); + + // All cases should be routed correctly + expect(result.workflow!.connections['Switch']['main'][0][0].node).toBe('Case0Handler'); + expect(result.workflow!.connections['Switch']['main'][1][0].node).toBe('Case1Handler'); + expect(result.workflow!.connections['Switch']['main'][2][0].node).toBe('Case2Handler'); + }); + + it('should use branch parameter with rewireConnection', async () => { + // Setup: Create IF node with true/false branches + const addIF: any = { + type: 'addNode', + node: { + name: 'IFRewire', + type: 'n8n-nodes-base.if', + position: [400, 300] + } + }; + + const addSuccess: any = { + type: 'addNode', + node: { + name: 'SuccessHandler', + type: 'n8n-nodes-base.set', + position: [600, 200] + } + }; + + const addNewSuccess: any = { + type: 'addNode', + node: { + name: 'NewSuccessHandler', + type: 'n8n-nodes-base.set', + position: [600, 250] + } + }; + + // Initial connection + const initialConn: any = { + type: 'addConnection', + source: 'IFRewire', + target: 'SuccessHandler', + branch: 'true' + }; + + // Rewire using branch parameter + const rewire: any = { + type: 'rewireConnection', + source: 'IFRewire', + from: 'SuccessHandler', + to: 'NewSuccessHandler', + branch: 'true' // Smart parameter + }; + + const request: WorkflowDiffRequest = { + id: 'test-workflow', + operations: [addIF, addSuccess, addNewSuccess, initialConn, rewire] + }; + + const result = await diffEngine.applyDiff(baseWorkflow, request); + + expect(result.success).toBe(true); + + // Should rewire the true branch + expect(result.workflow!.connections['IFRewire']['true'][0][0].node).toBe('NewSuccessHandler'); + }); + + it('should use case parameter with rewireConnection', async () => { + const addSwitch: any = { + type: 'addNode', + node: { + name: 'Switch', + type: 'n8n-nodes-base.switch', + position: [400, 300] + } + }; + + const addCase1: any = { + type: 'addNode', + node: { + name: 'Case1Handler', + type: 'n8n-nodes-base.set', + position: [600, 300] + } + }; + + const addNewCase1: any = { + type: 'addNode', + node: { + name: 'NewCase1Handler', + type: 'n8n-nodes-base.slack', + position: [600, 350] + } + }; + + const initialConn: any = { + type: 'addConnection', + source: 'Switch', + target: 'Case1Handler', + case: 1 + }; + + const rewire: any = { + type: 'rewireConnection', + source: 'Switch', + from: 'Case1Handler', + to: 'NewCase1Handler', + case: 1 // Smart parameter + }; + + const request: WorkflowDiffRequest = { + id: 'test-workflow', + operations: [addSwitch, addCase1, addNewCase1, initialConn, rewire] + }; + + const result = await diffEngine.applyDiff(baseWorkflow, request); + + expect(result.success).toBe(true); + + // Should rewire case 1 + expect(result.workflow!.connections['Switch']['main'][1][0].node).toBe('NewCase1Handler'); + }); + + it('should not override explicit sourceOutput with branch parameter', async () => { + const addIF: any = { + type: 'addNode', + node: { + name: 'IFOverride', + type: 'n8n-nodes-base.if', + position: [400, 300] + } + }; + + const addHandler: any = { + type: 'addNode', + node: { + name: 'OverrideHandler', + type: 'n8n-nodes-base.set', + position: [600, 300] + } + }; + + // Both branch and sourceOutput provided - sourceOutput should win + const connectWithBoth: any = { + type: 'addConnection', + source: 'IFOverride', + target: 'OverrideHandler', + branch: 'true', // Smart parameter suggests 'true' + sourceOutput: 'false' // Explicit parameter should override + }; + + const request: WorkflowDiffRequest = { + id: 'test-workflow', + operations: [addIF, addHandler, connectWithBoth] + }; + + const result = await diffEngine.applyDiff(baseWorkflow, request); + + expect(result.success).toBe(true); + + // Should use explicit sourceOutput ('false'), not branch ('true') + expect(result.workflow!.connections['IFOverride']['false']).toBeDefined(); + expect(result.workflow!.connections['IFOverride']['false'][0][0].node).toBe('OverrideHandler'); + expect(result.workflow!.connections['IFOverride']['true']).toBeUndefined(); + }); + + it('should not override explicit sourceIndex with case parameter', async () => { + const addSwitch: any = { + type: 'addNode', + node: { + name: 'Switch', + type: 'n8n-nodes-base.switch', + position: [400, 300] + } + }; + + const addHandler: any = { + type: 'addNode', + node: { + name: 'Handler', + type: 'n8n-nodes-base.set', + position: [600, 300] + } + }; + + // Both case and sourceIndex provided - sourceIndex should win + const connectWithBoth: any = { + type: 'addConnection', + source: 'Switch', + target: 'Handler', + case: 1, // Smart parameter suggests index 1 + sourceIndex: 2 // Explicit parameter should override + }; + + const request: WorkflowDiffRequest = { + id: 'test-workflow', + operations: [addSwitch, addHandler, connectWithBoth] + }; + + const result = await diffEngine.applyDiff(baseWorkflow, request); + + expect(result.success).toBe(true); + + // Should use explicit sourceIndex (2), not case (1) + expect(result.workflow!.connections['Switch']['main'][2]).toBeDefined(); + expect(result.workflow!.connections['Switch']['main'][2][0].node).toBe('Handler'); + expect(result.workflow!.connections['Switch']['main'][1]).toEqual([]); + }); + }); + describe('AddConnection with sourceIndex (Phase 0 Fix)', () => { it('should add connection to correct sourceIndex', async () => { // Add IF node