mirror of
https://github.com/czlonkowski/n8n-mcp.git
synced 2026-01-30 06:22:04 +00:00
fix: CRITICAL - branch parameter now correctly maps to sourceIndex, not sourceOutput
Found by n8n-mcp-tester agent: IF nodes in n8n store connections as: IF.main[0] (true branch) IF.main[1] (false branch) NOT as IF.true and IF.false Previous implementation (WRONG): - branch='true' → sourceOutput='true' Correct implementation (FIXED): - branch='true' → sourceIndex=0, sourceOutput='main' - branch='false' → sourceIndex=1, sourceOutput='main' Changes: - resolveSmartParameters(): branch now sets sourceIndex, not sourceOutput - Type definition comments updated to reflect correct mapping - All unit tests fixed to expect connections under 'main' with correct indices - All 141 tests passing with correct behavior This was caught by integration testing against real n8n API, not by unit tests. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
This commit is contained in:
@@ -648,10 +648,12 @@ export class WorkflowDiffEngine {
|
||||
let sourceIndex = operation.sourceIndex ?? 0;
|
||||
|
||||
// Smart parameter: branch (for IF nodes)
|
||||
if (operation.branch && !operation.sourceOutput) {
|
||||
// Only apply if sourceOutput not explicitly set
|
||||
// IF nodes use 'main' output with index 0 (true) or 1 (false)
|
||||
if (operation.branch !== undefined && operation.sourceIndex === undefined) {
|
||||
// Only apply if sourceIndex not explicitly set
|
||||
if (sourceNode?.type === 'n8n-nodes-base.if') {
|
||||
sourceOutput = operation.branch; // 'true' or 'false'
|
||||
sourceIndex = operation.branch === 'true' ? 0 : 1;
|
||||
// sourceOutput remains 'main' (do not change it)
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -65,8 +65,8 @@ export interface AddConnectionOperation extends DiffOperation {
|
||||
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
|
||||
branch?: 'true' | 'false'; // For IF nodes: maps to sourceIndex (0=true, 1=false)
|
||||
case?: number; // For Switch/multi-output nodes: maps to sourceIndex
|
||||
}
|
||||
|
||||
export interface RemoveConnectionOperation extends DiffOperation {
|
||||
@@ -99,8 +99,8 @@ export interface RewireConnectionOperation extends DiffOperation {
|
||||
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
|
||||
branch?: 'true' | 'false'; // For IF nodes: maps to sourceIndex (0=true, 1=false)
|
||||
case?: number; // For Switch/multi-output nodes: maps to sourceIndex
|
||||
}
|
||||
|
||||
// Workflow Metadata Operations
|
||||
|
||||
@@ -1184,9 +1184,10 @@ describe('WorkflowDiffEngine', () => {
|
||||
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');
|
||||
// Should create connection on 'main' output, index 0 (true branch)
|
||||
expect(result.workflow!.connections['IF']['main']).toBeDefined();
|
||||
expect(result.workflow!.connections['IF']['main'][0]).toBeDefined();
|
||||
expect(result.workflow!.connections['IF']['main'][0][0].node).toBe('TrueHandler');
|
||||
});
|
||||
|
||||
it('should use branch="false" for IF node connections', async () => {
|
||||
@@ -1224,9 +1225,10 @@ describe('WorkflowDiffEngine', () => {
|
||||
|
||||
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');
|
||||
// Should create connection on 'main' output, index 1 (false branch)
|
||||
expect(result.workflow!.connections['IF']['main']).toBeDefined();
|
||||
expect(result.workflow!.connections['IF']['main'][1]).toBeDefined();
|
||||
expect(result.workflow!.connections['IF']['main'][1][0].node).toBe('FalseHandler');
|
||||
});
|
||||
|
||||
it('should use case parameter for Switch node connections', async () => {
|
||||
@@ -1360,8 +1362,10 @@ describe('WorkflowDiffEngine', () => {
|
||||
|
||||
expect(result.success).toBe(true);
|
||||
|
||||
// Should rewire the true branch
|
||||
expect(result.workflow!.connections['IFRewire']['true'][0][0].node).toBe('NewSuccessHandler');
|
||||
// Should rewire the true branch (main output, index 0)
|
||||
expect(result.workflow!.connections['IFRewire']['main']).toBeDefined();
|
||||
expect(result.workflow!.connections['IFRewire']['main'][0]).toBeDefined();
|
||||
expect(result.workflow!.connections['IFRewire']['main'][0][0].node).toBe('NewSuccessHandler');
|
||||
});
|
||||
|
||||
it('should use case parameter with rewireConnection', async () => {
|
||||
@@ -1457,10 +1461,12 @@ describe('WorkflowDiffEngine', () => {
|
||||
|
||||
expect(result.success).toBe(true);
|
||||
|
||||
// Should use explicit sourceOutput ('false'), not branch ('true')
|
||||
// Should use explicit sourceOutput ('false'), not smart branch parameter
|
||||
// Note: explicit sourceOutput='false' creates connection on output named 'false'
|
||||
// This is different from branch parameter which maps to sourceIndex
|
||||
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();
|
||||
expect(result.workflow!.connections['IFOverride']['main']).toBeUndefined();
|
||||
});
|
||||
|
||||
it('should not override explicit sourceIndex with case parameter', async () => {
|
||||
|
||||
Reference in New Issue
Block a user