fix: preserve array indices in multi-output nodes when removing connections

CRITICAL BUG FIX: Fixed array index corruption in multi-output nodes
(Switch, IF with multiple handlers, Merge) when rewiring connections.

Problem:
- applyRemoveConnection() filtered out empty arrays after removing connections
- This caused indices to shift in multi-output nodes
- Example: Switch.main = [[H0], [H1], [H2]] -> remove H1 -> [[H0], [H2]]
- H2 moved from index 2 to index 1, corrupting workflow structure

Root Cause:
```typescript
// Line 697 - BUGGY CODE:
workflow.connections[node][output] =
  connections.filter(conns => conns.length > 0);
```

Solution:
- Only remove trailing empty arrays
- Preserve intermediate empty arrays to maintain index integrity
- Example: [[H0], [], [H2]] stays [[H0], [], [H2]] not [[H0], [H2]]

Impact:
- Prevents production-breaking workflow corruption
- Fixes rewireConnection operation for multi-output nodes
- Critical for AI agents working with complex workflows

Testing:
- Added integration test for Switch node rewiring with array index verification
- Test creates 4-output Switch node, rewires middle connection
- Verifies indices 0, 2, 3 unchanged after rewiring index 1
- All 137 unit tests + 12 integration tests passing

Discovered by: @agent-n8n-mcp-tester during comprehensive testing
Issue: #272 (Connection Operations - Phase 1)

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
This commit is contained in:
czlonkowski
2025-10-06 09:18:27 +02:00
parent af949b09a5
commit aeb74102e5
5 changed files with 1862 additions and 12 deletions

Binary file not shown.

View File

@@ -70,7 +70,72 @@
- Test file: validate-workflow.test.ts (431 lines)
- Test results: 83/83 integration tests passing (Phase 1-5, 6A complete)
**Next Phase**: Phase 6B - Workflow Autofix Tests
**Phase 6B: Workflow Autofix Tests****COMPLETE** (October 5, 2025)
- 16 test scenarios implemented and passing
- All autofix operations tested: preview mode, apply mode, fix types, confidence filtering
- Test coverage:
- Preview mode (2 scenarios - expression-format, multiple fix types)
- Apply mode (2 scenarios - expression-format, webhook-missing-path)
- Fix type filtering (2 scenarios - single type, multiple types)
- Confidence thresholds (3 scenarios - high, medium, low)
- Max fixes parameter (1 scenario)
- No fixes available (1 scenario)
- Error handling (3 scenarios - non-existent workflow, invalid parameters)
- Response format verification (2 scenarios - preview and apply modes)
- Fix types tested:
- expression-format (missing = prefix for resource locators)
- typeversion-correction (outdated typeVersion values)
- error-output-config (error output configuration issues)
- node-type-correction (incorrect node types)
- webhook-missing-path (missing webhook path parameters)
- Code quality improvements:
- Fixed database resource leak in NodeRepository utility
- Added TypeScript interfaces (ValidationResponse, AutofixResponse)
- Replaced unsafe `as any` casts with proper type definitions
- All lint and typecheck errors resolved
- Test file: autofix-workflow.test.ts (855 lines)
- Test results: 99/99 integration tests passing (Phase 1-6 complete)
**Phase 7: Execution Management Tests****COMPLETE** (October 5, 2025)
- 54 test scenarios implemented and passing
- All 4 execution management handlers tested against real n8n instance
- Test coverage:
- handleTriggerWebhookWorkflow (20 tests): All HTTP methods (GET/POST/PUT/DELETE), query params, JSON body, custom headers, error handling
- handleGetExecution (16 tests): All 4 retrieval modes (preview/summary/filtered/full), node filtering, item limits, input data inclusion, legacy compatibility
- handleListExecutions (13 tests): Status filtering (success/error/waiting), pagination with cursor, various limits (1/10/50/100), data inclusion control
- handleDeleteExecution (5 tests): Successful deletion, verification via fetch attempt, error handling
- Critical fix: Corrected response structure expectations (executions/returned vs data/count)
- Test files:
- trigger-webhook.test.ts (375 lines, 20 tests)
- get-execution.test.ts (429 lines, 16 tests)
- list-executions.test.ts (264 lines, 13 tests)
- delete-execution.test.ts (149 lines, 5 tests)
- Code review: APPROVED (9.5/10 quality score)
- Test results: 153/153 integration tests passing (Phase 1-7 complete)
**Phase 8: System Tools Tests****COMPLETE** (October 5, 2025)
- 19 test scenarios implemented and passing
- All 3 system tool handlers tested against real n8n instance
- Test coverage:
- handleHealthCheck (3 tests): API connectivity verification, version information, feature availability
- handleListAvailableTools (7 tests): Complete tool inventory by category, configuration status, API limitations
- handleDiagnostic (9 tests): Environment checks, API connectivity, tools availability, verbose mode with debug info
- TypeScript type safety improvements:
- Created response-types.ts with comprehensive interfaces for all response types
- Replaced all 'as any' casts with proper TypeScript interfaces
- Added null-safety checks and non-null assertions
- Full type safety and IDE autocomplete support
- Test files:
- health-check.test.ts (117 lines, 3 tests)
- list-tools.test.ts (181 lines, 7 tests)
- diagnostic.test.ts (243 lines, 9 tests)
- response-types.ts (241 lines, comprehensive type definitions)
- Code review: APPROVED
- Test results: 172/172 integration tests passing (Phase 1-8 complete)
**🎉 INTEGRATION TEST SUITE COMPLETE**: All 18 MCP handlers fully tested
**Next Phase**: Update documentation and finalize integration testing plan
---
@@ -1166,15 +1231,16 @@ jobs:
- ✅ All tests passing against real n8n instance
### Overall Project (In Progress)
- ⏳ All 17 handlers have integration tests (10 of 17 complete)
- ⏳ All operations/parameters covered (83 of 150+ scenarios complete)
- ✅ Tests run successfully locally (Phases 1-6A verified)
- ⏳ All 17 handlers have integration tests (11 of 17 complete)
- ⏳ All operations/parameters covered (99 of 150+ scenarios complete)
- ✅ Tests run successfully locally (Phases 1-6 verified)
- ⏳ Tests run successfully in CI (pending Phase 9)
- ✅ No manual cleanup required (automatic)
- ✅ Test coverage catches P0-level bugs (verified in Phase 2)
- ⏳ CI runs on every PR and daily (pending Phase 9)
- ✅ Clear error messages when tests fail
- ✅ Documentation for webhook workflow setup
- ✅ Code quality maintained (lint, typecheck, type safety)
---
@@ -1186,12 +1252,12 @@ jobs:
- **Phase 4 (Updates)**: ✅ COMPLETE (October 4, 2025)
- **Phase 5 (Management)**: ✅ COMPLETE (October 4, 2025)
- **Phase 6A (Validation)**: ✅ COMPLETE (October 5, 2025)
- **Phase 6B (Autofix)**: 1 day
- **Phase 6B (Autofix)**: ✅ COMPLETE (October 5, 2025)
- **Phase 7 (Executions)**: 2 days
- **Phase 8 (System)**: 1 day
- **Phase 9 (CI/CD)**: 1 day
**Total**: 5.5 days complete, ~5 days remaining
**Total**: 6 days complete, ~4 days remaining
---

View File

@@ -692,11 +692,13 @@ export class WorkflowDiffEngine {
conns.filter(conn => conn.node !== targetNode.name)
);
// Clean up empty arrays
workflow.connections[sourceNode.name][sourceOutput] =
workflow.connections[sourceNode.name][sourceOutput].filter(conns => conns.length > 0);
// Remove trailing empty arrays only (preserve intermediate empty arrays to maintain indices)
const outputConnections = workflow.connections[sourceNode.name][sourceOutput];
while (outputConnections.length > 0 && outputConnections[outputConnections.length - 1].length === 0) {
outputConnections.pop();
}
if (workflow.connections[sourceNode.name][sourceOutput].length === 0) {
if (outputConnections.length === 0) {
delete workflow.connections[sourceNode.name][sourceOutput];
}

1637
test-output.txt Normal file

File diff suppressed because it is too large Load Diff

View File

@@ -1357,4 +1357,149 @@ describe('Integration: Smart Parameters with Real n8n API', () => {
expect(fetchedWorkflow.connections.Set.main[0][0].node).toBe('Handler');
});
});
// ======================================================================
// TEST 11: Array Index Preservation (Issue #272 - Critical Bug Fix)
// ======================================================================
describe('Array Index Preservation for Multi-Output Nodes', () => {
it('should preserve array indices when rewiring Switch node connections', async () => {
// This test verifies the fix for the critical bug where filtering empty arrays
// caused index shifting in multi-output nodes (Switch, IF with multiple handlers)
//
// Bug: workflow.connections[node][output].filter(conns => conns.length > 0)
// Fix: Only remove trailing empty arrays, preserve intermediate ones
const workflowName = createTestWorkflowName('Array Index Preservation - Switch');
// Create workflow with Switch node connected to 4 handlers
const workflow: Workflow = await client.createWorkflow({
name: workflowName,
nodes: [
{
id: '1',
name: 'Start',
type: 'n8n-nodes-base.manualTrigger',
typeVersion: 1,
position: [0, 0],
parameters: {}
},
{
id: '2',
name: 'Switch',
type: 'n8n-nodes-base.switch',
typeVersion: 3,
position: [200, 0],
parameters: {
options: {},
rules: {
rules: [
{ conditions: { conditions: [{ leftValue: '={{$json.value}}', rightValue: '1', operator: { type: 'string', operation: 'equals' } }] } },
{ conditions: { conditions: [{ leftValue: '={{$json.value}}', rightValue: '2', operator: { type: 'string', operation: 'equals' } }] } },
{ conditions: { conditions: [{ leftValue: '={{$json.value}}', rightValue: '3', operator: { type: 'string', operation: 'equals' } }] } }
]
}
}
},
{
id: '3',
name: 'Handler0',
type: 'n8n-nodes-base.noOp',
typeVersion: 1,
position: [400, -100],
parameters: {}
},
{
id: '4',
name: 'Handler1',
type: 'n8n-nodes-base.noOp',
typeVersion: 1,
position: [400, 0],
parameters: {}
},
{
id: '5',
name: 'Handler2',
type: 'n8n-nodes-base.noOp',
typeVersion: 1,
position: [400, 100],
parameters: {}
},
{
id: '6',
name: 'Handler3',
type: 'n8n-nodes-base.noOp',
typeVersion: 1,
position: [400, 200],
parameters: {}
},
{
id: '7',
name: 'NewHandler1',
type: 'n8n-nodes-base.noOp',
typeVersion: 1,
position: [400, 50],
parameters: {}
}
],
connections: {
Start: {
main: [[{ node: 'Switch', type: 'main', index: 0 }]]
},
Switch: {
main: [
[{ node: 'Handler0', type: 'main', index: 0 }], // case 0
[{ node: 'Handler1', type: 'main', index: 0 }], // case 1
[{ node: 'Handler2', type: 'main', index: 0 }], // case 2
[{ node: 'Handler3', type: 'main', index: 0 }] // case 3 (fallback)
]
}
}
});
expect(workflow.id).toBeTruthy();
if (!workflow.id) throw new Error('Workflow ID is missing');
context.trackWorkflow(workflow.id);
// Rewire case 1 from Handler1 to NewHandler1
// CRITICAL: This should NOT shift indices of case 2 and case 3
await handleUpdatePartialWorkflow({
id: workflow.id,
operations: [
{
type: 'rewireConnection',
source: 'Switch',
from: 'Handler1',
to: 'NewHandler1',
case: 1
}
]
});
const fetchedWorkflow = await client.getWorkflow(workflow.id);
// Verify all indices are preserved correctly
expect(fetchedWorkflow.connections.Switch).toBeDefined();
expect(fetchedWorkflow.connections.Switch.main).toBeDefined();
// case 0: Should still be Handler0
expect(fetchedWorkflow.connections.Switch.main[0]).toBeDefined();
expect(fetchedWorkflow.connections.Switch.main[0].length).toBe(1);
expect(fetchedWorkflow.connections.Switch.main[0][0].node).toBe('Handler0');
// case 1: Should now be NewHandler1 (rewired)
expect(fetchedWorkflow.connections.Switch.main[1]).toBeDefined();
expect(fetchedWorkflow.connections.Switch.main[1].length).toBe(1);
expect(fetchedWorkflow.connections.Switch.main[1][0].node).toBe('NewHandler1');
// case 2: Should STILL be Handler2 (index NOT shifted!)
expect(fetchedWorkflow.connections.Switch.main[2]).toBeDefined();
expect(fetchedWorkflow.connections.Switch.main[2].length).toBe(1);
expect(fetchedWorkflow.connections.Switch.main[2][0].node).toBe('Handler2');
// case 3: Should STILL be Handler3 (index NOT shifted!)
expect(fetchedWorkflow.connections.Switch.main[3]).toBeDefined();
expect(fetchedWorkflow.connections.Switch.main[3].length).toBe(1);
expect(fetchedWorkflow.connections.Switch.main[3][0].node).toBe('Handler3');
});
});
});