From 3188d209b78ad8267fb0afb70bf3050174116230 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Romuald=20Cz=C5=82onkowski?= <56956555+czlonkowski@users.noreply.github.com> Date: Mon, 1 Dec 2025 18:54:15 +0100 Subject: [PATCH] fix: AI connection type propagation and get_node improvements (v2.28.1) (#461) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix: AI connection type propagation and get_node improvements (v2.28.1) Bug fixes: - Issue #458: addConnection now preserves AI connection types (ai_tool, ai_memory, ai_languageModel) instead of defaulting to 'main' - Fixed false positive "AI Agent has no tools connected" validation warning Enhancements: - Added expectedFormat field to resourceLocator properties in get_node output - Added versionNotice field to make typeVersion more prominent Conceived by Romuald Członkowski - https://www.aiadvisors.pl/en 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude * test: add missing test coverage for PR #461 improvements - Added test for AI Agent validation positive case (tools properly connected) - Added 3 tests for expectedFormat on resourceLocator properties Conceived by Romuald Członkowski - https://www.aiadvisors.pl/en 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude --------- Co-authored-by: Claude --- CHANGELOG.md | 51 ++++ package.json | 2 +- src/mcp/server.ts | 7 +- src/services/property-filter.ts | 18 +- src/services/workflow-diff-engine.ts | 3 +- src/services/workflow-validator.ts | 17 +- tests/unit/services/property-filter.test.ts | 69 ++++++ .../services/workflow-diff-engine.test.ts | 219 ++++++++++++++++++ .../workflow-validator-comprehensive.test.ts | 31 +++ 9 files changed, 408 insertions(+), 9 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index d5ea4c7..439d849 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,57 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +## [2.28.1] - 2025-12-01 + +### 🐛 Bug Fixes + +**Issue #458: AI Connection Type Propagation** + +Fixed `addConnection` operation in workflow diff engine defaulting `targetInput` to "main" instead of preserving the source output type. This caused AI tool connections to be created with incorrect type. + +- **Root Cause**: `targetInput` defaulted to `'main'` regardless of `sourceOutput` type +- **Fix**: Changed default to `sourceOutput` to preserve connection type (ai_tool, ai_memory, ai_languageModel) +- **Files**: `src/services/workflow-diff-engine.ts:760` + +**AI Agent Validation False Positive** + +Fixed false positive "AI Agent has no tools connected" warning when tools were properly connected. + +- **Root Cause**: Validation checked connections FROM agent instead of TO agent +- **Fix**: Search all connections where target node is the agent +- **Files**: `src/services/workflow-validator.ts:1148-1163` + +### ✨ Enhancements + +**get_node: expectedFormat for resourceLocator Properties** + +Added `expectedFormat` field to resourceLocator properties in `get_node` output. This helps AI models understand the correct format for these complex property types. + +```json +{ + "name": "model", + "type": "resourceLocator", + "expectedFormat": { + "structure": { "mode": "string", "value": "string" }, + "modes": ["list", "id"], + "example": { "mode": "id", "value": "gpt-4o-mini" } + } +} +``` + +**get_node: versionNotice Field** + +Added `versionNotice` field to make typeVersion more prominent in get_node output, reducing the chance of AI models using outdated versions. + +```json +{ + "version": "1.3", + "versionNotice": "⚠️ Use typeVersion: 1.3 when creating this node" +} +``` + +**Conceived by Romuald Członkowski - [AiAdvisors](https://www.aiadvisors.pl/en)** + ## [2.28.0] - 2025-12-01 ### ✨ Features diff --git a/package.json b/package.json index 00630ee..e427b16 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "n8n-mcp", - "version": "2.28.0", + "version": "2.28.1", "description": "Integration between n8n workflow automation and Model Context Protocol (MCP)", "main": "dist/index.js", "types": "dist/index.d.ts", diff --git a/src/mcp/server.ts b/src/mcp/server.ts index 2ee783a..a024c0d 100644 --- a/src/mcp/server.ts +++ b/src/mcp/server.ts @@ -2203,14 +2203,19 @@ Full documentation is being prepared. For now, use get_node_essentials for confi // Get operations (already parsed by repository) const operations = node.operations || []; + // Get the latest version - this is important for AI to use correct typeVersion + const latestVersion = node.version ?? '1'; + const result = { nodeType: node.nodeType, workflowNodeType: getWorkflowNodeType(node.package ?? 'n8n-nodes-base', node.nodeType), displayName: node.displayName, description: node.description, category: node.category, - version: node.version ?? '1', + version: latestVersion, isVersioned: node.isVersioned ?? false, + // Prominent warning to use the correct typeVersion + versionNotice: `⚠️ Use typeVersion: ${latestVersion} when creating this node`, requiredProperties: essentials.required, commonProperties: essentials.common, operations: operations.map((op: any) => ({ diff --git a/src/services/property-filter.ts b/src/services/property-filter.ts index a82c657..ec7c5a3 100644 --- a/src/services/property-filter.ts +++ b/src/services/property-filter.ts @@ -16,6 +16,11 @@ export interface SimplifiedProperty { placeholder?: string; showWhen?: Record; usageHint?: string; + expectedFormat?: { + structure: Record; + modes?: string[]; + example: Record; + }; } export interface EssentialConfig { @@ -322,7 +327,18 @@ export class PropertyFilter { }; }); } - + + // Add expectedFormat for resourceLocator types - critical for correct configuration + if (prop.type === 'resourceLocator') { + const modes = prop.modes?.map((m: any) => m.name || m) || ['list', 'id']; + const defaultValue = prop.default?.value || 'your-resource-id'; + simplified.expectedFormat = { + structure: { mode: 'string', value: 'string' }, + modes, + example: { mode: 'id', value: defaultValue } + }; + } + // Include simple display conditions (max 2 conditions) if (prop.displayOptions?.show) { const conditions = Object.keys(prop.displayOptions.show); diff --git a/src/services/workflow-diff-engine.ts b/src/services/workflow-diff-engine.ts index 3538dd8..5f1b62b 100644 --- a/src/services/workflow-diff-engine.ts +++ b/src/services/workflow-diff-engine.ts @@ -757,7 +757,8 @@ export class WorkflowDiffEngine { const { sourceOutput, sourceIndex } = this.resolveSmartParameters(workflow, operation); // Use nullish coalescing to properly handle explicit 0 values - const targetInput = operation.targetInput ?? 'main'; + // Default targetInput to sourceOutput to preserve connection type for AI connections (ai_tool, ai_memory, etc.) + const targetInput = operation.targetInput ?? sourceOutput; const targetIndex = operation.targetIndex ?? 0; // Initialize source node connections object diff --git a/src/services/workflow-validator.ts b/src/services/workflow-validator.ts index 935505c..aeadbb9 100644 --- a/src/services/workflow-validator.ts +++ b/src/services/workflow-validator.ts @@ -1137,16 +1137,23 @@ export class WorkflowValidator { } // Check for AI Agent workflows - const aiAgentNodes = workflow.nodes.filter(n => - n.type.toLowerCase().includes('agent') || + const aiAgentNodes = workflow.nodes.filter(n => + n.type.toLowerCase().includes('agent') || n.type.includes('langchain.agent') ); - + if (aiAgentNodes.length > 0) { // Check if AI agents have tools connected + // Tools connect TO the agent, so we need to find connections where the target is the agent for (const agentNode of aiAgentNodes) { - const connections = workflow.connections[agentNode.name]; - if (!connections?.ai_tool || connections.ai_tool.flat().filter(c => c).length === 0) { + // Search all connections to find ones targeting this agent via ai_tool + const hasToolConnected = Object.values(workflow.connections).some(sourceOutputs => { + const aiToolConnections = sourceOutputs.ai_tool; + if (!aiToolConnections) return false; + return aiToolConnections.flat().some(conn => conn && conn.node === agentNode.name); + }); + + if (!hasToolConnected) { result.warnings.push({ type: 'warning', nodeId: agentNode.id, diff --git a/tests/unit/services/property-filter.test.ts b/tests/unit/services/property-filter.test.ts index fa8d008..2f34e44 100644 --- a/tests/unit/services/property-filter.test.ts +++ b/tests/unit/services/property-filter.test.ts @@ -406,5 +406,74 @@ describe('PropertyFilter', () => { const complex = result.common.find(p => p.name === 'complex'); expect(complex?.default).toBeUndefined(); }); + + it('should add expectedFormat for resourceLocator type properties', () => { + const properties = [ + { + name: 'channel', + type: 'resourceLocator', + displayName: 'Channel', + description: 'The channel to send message to', + modes: [ + { name: 'list', displayName: 'From List' }, + { name: 'id', displayName: 'By ID' }, + { name: 'url', displayName: 'By URL' } + ], + default: { mode: 'list', value: '' } + } + ]; + + const result = PropertyFilter.getEssentials(properties, 'nodes-base.slack'); + + const channelProp = result.common.find(p => p.name === 'channel'); + expect(channelProp).toBeDefined(); + expect(channelProp?.expectedFormat).toBeDefined(); + expect(channelProp?.expectedFormat?.structure).toEqual({ + mode: 'string', + value: 'string' + }); + expect(channelProp?.expectedFormat?.modes).toEqual(['list', 'id', 'url']); + expect(channelProp?.expectedFormat?.example).toBeDefined(); + expect(channelProp?.expectedFormat?.example.mode).toBe('id'); + expect(channelProp?.expectedFormat?.example.value).toBeDefined(); + }); + + it('should handle resourceLocator without modes array', () => { + const properties = [ + { + name: 'resource', + type: 'resourceLocator', + displayName: 'Resource', + default: { mode: 'id', value: 'test-123' } + } + ]; + + const result = PropertyFilter.getEssentials(properties, 'nodes-base.unknownNode'); + + const resourceProp = result.common.find(p => p.name === 'resource'); + expect(resourceProp?.expectedFormat).toBeDefined(); + // Should default to common modes + expect(resourceProp?.expectedFormat?.modes).toEqual(['list', 'id']); + expect(resourceProp?.expectedFormat?.example.value).toBe('test-123'); + }); + + it('should handle resourceLocator with no default value', () => { + const properties = [ + { + name: 'item', + type: 'resourceLocator', + displayName: 'Item', + modes: [{ name: 'search' }, { name: 'id' }] + } + ]; + + const result = PropertyFilter.getEssentials(properties, 'nodes-base.unknownNode'); + + const itemProp = result.common.find(p => p.name === 'item'); + expect(itemProp?.expectedFormat).toBeDefined(); + expect(itemProp?.expectedFormat?.modes).toEqual(['search', 'id']); + // Should use fallback value + expect(itemProp?.expectedFormat?.example.value).toBe('your-resource-id'); + }); }); }); \ No newline at end of file diff --git a/tests/unit/services/workflow-diff-engine.test.ts b/tests/unit/services/workflow-diff-engine.test.ts index c24fb37..751e6e9 100644 --- a/tests/unit/services/workflow-diff-engine.test.ts +++ b/tests/unit/services/workflow-diff-engine.test.ts @@ -4665,4 +4665,223 @@ describe('WorkflowDiffEngine', () => { expect(result.errors![0].message).toContain('executeWorkflowTrigger cannot activate workflows'); }); }); + + // Issue #458: AI connection type propagation + describe('AI Connection Type Propagation (Issue #458)', () => { + it('should propagate ai_tool connection type when targetInput is not specified', async () => { + const workflowWithAI = { + ...baseWorkflow, + nodes: [ + { + id: 'agent1', + name: 'AI Agent', + type: '@n8n/n8n-nodes-langchain.agent', + typeVersion: 2.1, + position: [500, 300] as [number, number], + parameters: {} + }, + { + id: 'tool1', + name: 'Calculator', + type: '@n8n/n8n-nodes-langchain.toolCalculator', + typeVersion: 1, + position: [300, 400] as [number, number], + parameters: {} + } + ], + connections: {} + }; + + const operation: AddConnectionOperation = { + type: 'addConnection', + source: 'Calculator', + target: 'AI Agent', + sourceOutput: 'ai_tool' + // targetInput not specified - should default to sourceOutput ('ai_tool') + }; + + const request: WorkflowDiffRequest = { + id: 'test-workflow', + operations: [operation] + }; + + const result = await diffEngine.applyDiff(workflowWithAI as Workflow, request); + + expect(result.success).toBe(true); + expect(result.workflow.connections['Calculator']).toBeDefined(); + expect(result.workflow.connections['Calculator']['ai_tool']).toBeDefined(); + // The inner type should be 'ai_tool', NOT 'main' + expect(result.workflow.connections['Calculator']['ai_tool'][0][0].type).toBe('ai_tool'); + expect(result.workflow.connections['Calculator']['ai_tool'][0][0].node).toBe('AI Agent'); + }); + + it('should propagate ai_languageModel connection type', async () => { + const workflowWithAI = { + ...baseWorkflow, + nodes: [ + { + id: 'agent1', + name: 'AI Agent', + type: '@n8n/n8n-nodes-langchain.agent', + typeVersion: 2.1, + position: [500, 300] as [number, number], + parameters: {} + }, + { + id: 'llm1', + name: 'OpenAI Chat Model', + type: '@n8n/n8n-nodes-langchain.lmChatOpenAi', + typeVersion: 1.2, + position: [300, 200] as [number, number], + parameters: {} + } + ], + connections: {} + }; + + const operation: AddConnectionOperation = { + type: 'addConnection', + source: 'OpenAI Chat Model', + target: 'AI Agent', + sourceOutput: 'ai_languageModel' + }; + + const request: WorkflowDiffRequest = { + id: 'test-workflow', + operations: [operation] + }; + + const result = await diffEngine.applyDiff(workflowWithAI as Workflow, request); + + expect(result.success).toBe(true); + expect(result.workflow.connections['OpenAI Chat Model']['ai_languageModel'][0][0].type).toBe('ai_languageModel'); + }); + + it('should propagate ai_memory connection type', async () => { + const workflowWithAI = { + ...baseWorkflow, + nodes: [ + { + id: 'agent1', + name: 'AI Agent', + type: '@n8n/n8n-nodes-langchain.agent', + typeVersion: 2.1, + position: [500, 300] as [number, number], + parameters: {} + }, + { + id: 'memory1', + name: 'Window Buffer Memory', + type: '@n8n/n8n-nodes-langchain.memoryBufferWindow', + typeVersion: 1.3, + position: [300, 500] as [number, number], + parameters: {} + } + ], + connections: {} + }; + + const operation: AddConnectionOperation = { + type: 'addConnection', + source: 'Window Buffer Memory', + target: 'AI Agent', + sourceOutput: 'ai_memory' + }; + + const request: WorkflowDiffRequest = { + id: 'test-workflow', + operations: [operation] + }; + + const result = await diffEngine.applyDiff(workflowWithAI as Workflow, request); + + expect(result.success).toBe(true); + expect(result.workflow.connections['Window Buffer Memory']['ai_memory'][0][0].type).toBe('ai_memory'); + }); + + it('should allow explicit targetInput override for mixed connection types', async () => { + const workflowWithNodes = { + ...baseWorkflow, + nodes: [ + { + id: 'node1', + name: 'Source Node', + type: 'n8n-nodes-base.set', + typeVersion: 3.4, + position: [300, 300] as [number, number], + parameters: {} + }, + { + id: 'node2', + name: 'Target Node', + type: 'n8n-nodes-base.set', + typeVersion: 3.4, + position: [500, 300] as [number, number], + parameters: {} + } + ], + connections: {} + }; + + const operation: AddConnectionOperation = { + type: 'addConnection', + source: 'Source Node', + target: 'Target Node', + sourceOutput: 'main', + targetInput: 'main' // Explicit override + }; + + const request: WorkflowDiffRequest = { + id: 'test-workflow', + operations: [operation] + }; + + const result = await diffEngine.applyDiff(workflowWithNodes as Workflow, request); + + expect(result.success).toBe(true); + expect(result.workflow.connections['Source Node']['main'][0][0].type).toBe('main'); + }); + + it('should default to main for regular connections when sourceOutput is not specified', async () => { + const workflowWithNodes = { + ...baseWorkflow, + nodes: [ + { + id: 'node1', + name: 'Source Node', + type: 'n8n-nodes-base.set', + typeVersion: 3.4, + position: [300, 300] as [number, number], + parameters: {} + }, + { + id: 'node2', + name: 'Target Node', + type: 'n8n-nodes-base.set', + typeVersion: 3.4, + position: [500, 300] as [number, number], + parameters: {} + } + ], + connections: {} + }; + + const operation: AddConnectionOperation = { + type: 'addConnection', + source: 'Source Node', + target: 'Target Node' + // Neither sourceOutput nor targetInput specified - should default to 'main' + }; + + const request: WorkflowDiffRequest = { + id: 'test-workflow', + operations: [operation] + }; + + const result = await diffEngine.applyDiff(workflowWithNodes as Workflow, request); + + expect(result.success).toBe(true); + expect(result.workflow.connections['Source Node']['main'][0][0].type).toBe('main'); + }); + }); }); \ No newline at end of file diff --git a/tests/unit/services/workflow-validator-comprehensive.test.ts b/tests/unit/services/workflow-validator-comprehensive.test.ts index 55440d4..4cce02b 100644 --- a/tests/unit/services/workflow-validator-comprehensive.test.ts +++ b/tests/unit/services/workflow-validator-comprehensive.test.ts @@ -1329,6 +1329,37 @@ describe('WorkflowValidator - Comprehensive Tests', () => { expect(result.warnings.some(w => w.message.includes('AI Agent has no tools connected'))).toBe(true); }); + it('should NOT warn about AI agents WITH tools properly connected', async () => { + const workflow = { + nodes: [ + { + id: '1', + name: 'Calculator Tool', + type: 'n8n-nodes-base.httpRequest', + position: [100, 100], + parameters: {} + }, + { + id: '2', + name: 'Agent', + type: '@n8n/n8n-nodes-langchain.agent', + position: [300, 100], + parameters: {} + } + ], + connections: { + 'Calculator Tool': { + ai_tool: [[{ node: 'Agent', type: 'ai_tool', index: 0 }]] + } + } + } as any; + + const result = await validator.validateWorkflow(workflow as any); + + // Should NOT have warning about missing tools + expect(result.warnings.some(w => w.message.includes('AI Agent has no tools connected'))).toBe(false); + }); + it('should suggest community package setting for AI tools', async () => { const workflow = { nodes: [