mirror of
https://github.com/czlonkowski/n8n-mcp.git
synced 2026-03-16 23:43:07 +00:00
- Normalize name→nodeName and id→nodeId for node-targeting operations in the Zod schema transform, so LLMs using natural field names no longer get "Node not found" errors - Replace hardcoded ALL_CONNECTION_TYPES with dynamic iteration so AI sub-nodes (ai_outputParser, ai_document, ai_textSplitter, etc.) are not flagged as disconnected during save - Add .catchall() to workflowConnectionSchema and extend connection reference validation to cover all connection types, not just main - Fix filterOperationsByFixes ID-vs-name mismatch: typeversion-upgrade operations now include nodeName alongside nodeId, and the filter checks both fields Conceived by Romuald Członkowski - https://www.aiadvisors.pl/en Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This commit is contained in:
committed by
GitHub
parent
65ab94deb2
commit
f7a1cfe8bf
@@ -1001,5 +1001,122 @@ describe('handlers-workflow-diff', () => {
|
||||
expect(mockApiClient.updateWorkflowTags).not.toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
|
||||
describe('field name normalization', () => {
|
||||
it('should normalize "name" to "nodeName" for updateNode operations', async () => {
|
||||
const testWorkflow = createTestWorkflow();
|
||||
const updatedWorkflow = { ...testWorkflow };
|
||||
|
||||
mockApiClient.getWorkflow.mockResolvedValue(testWorkflow);
|
||||
mockDiffEngine.applyDiff.mockResolvedValue({
|
||||
success: true,
|
||||
workflow: updatedWorkflow,
|
||||
operationsApplied: 1,
|
||||
message: 'Success',
|
||||
errors: [],
|
||||
});
|
||||
mockApiClient.updateWorkflow.mockResolvedValue(updatedWorkflow);
|
||||
|
||||
await handleUpdatePartialWorkflow({
|
||||
id: 'test-workflow-id',
|
||||
operations: [{
|
||||
type: 'updateNode',
|
||||
name: 'HTTP Request', // LLMs often use "name" instead of "nodeName"
|
||||
updates: { 'parameters.url': 'https://new-url.com' },
|
||||
}],
|
||||
}, mockRepository);
|
||||
|
||||
// Verify the diff engine received nodeName (normalized from name)
|
||||
expect(mockDiffEngine.applyDiff).toHaveBeenCalled();
|
||||
const diffArgs = mockDiffEngine.applyDiff.mock.calls[0][1];
|
||||
expect(diffArgs.operations[0].nodeName).toBe('HTTP Request');
|
||||
});
|
||||
|
||||
it('should normalize "id" to "nodeId" for removeNode operations', async () => {
|
||||
const testWorkflow = createTestWorkflow();
|
||||
const updatedWorkflow = { ...testWorkflow };
|
||||
|
||||
mockApiClient.getWorkflow.mockResolvedValue(testWorkflow);
|
||||
mockDiffEngine.applyDiff.mockResolvedValue({
|
||||
success: true,
|
||||
workflow: updatedWorkflow,
|
||||
operationsApplied: 1,
|
||||
message: 'Success',
|
||||
errors: [],
|
||||
});
|
||||
mockApiClient.updateWorkflow.mockResolvedValue(updatedWorkflow);
|
||||
|
||||
await handleUpdatePartialWorkflow({
|
||||
id: 'test-workflow-id',
|
||||
operations: [{
|
||||
type: 'removeNode',
|
||||
id: 'node2', // LLMs may use "id" instead of "nodeId"
|
||||
}],
|
||||
}, mockRepository);
|
||||
|
||||
// Verify the diff engine received nodeId (normalized from id)
|
||||
expect(mockDiffEngine.applyDiff).toHaveBeenCalled();
|
||||
const diffArgs = mockDiffEngine.applyDiff.mock.calls[0][1];
|
||||
expect(diffArgs.operations[0].nodeId).toBe('node2');
|
||||
});
|
||||
|
||||
it('should NOT normalize "name" for updateName operations', async () => {
|
||||
const testWorkflow = createTestWorkflow();
|
||||
const updatedWorkflow = { ...testWorkflow };
|
||||
|
||||
mockApiClient.getWorkflow.mockResolvedValue(testWorkflow);
|
||||
mockDiffEngine.applyDiff.mockResolvedValue({
|
||||
success: true,
|
||||
workflow: updatedWorkflow,
|
||||
operationsApplied: 1,
|
||||
message: 'Success',
|
||||
errors: [],
|
||||
});
|
||||
mockApiClient.updateWorkflow.mockResolvedValue(updatedWorkflow);
|
||||
|
||||
await handleUpdatePartialWorkflow({
|
||||
id: 'test-workflow-id',
|
||||
operations: [{
|
||||
type: 'updateName',
|
||||
name: 'New Workflow Name', // This is the correct field for updateName
|
||||
}],
|
||||
}, mockRepository);
|
||||
|
||||
// Verify "name" stays as "name" (not moved to nodeName) for updateName
|
||||
expect(mockDiffEngine.applyDiff).toHaveBeenCalled();
|
||||
const diffArgs = mockDiffEngine.applyDiff.mock.calls[0][1];
|
||||
expect(diffArgs.operations[0].name).toBe('New Workflow Name');
|
||||
expect(diffArgs.operations[0].nodeName).toBeUndefined();
|
||||
});
|
||||
|
||||
it('should prefer explicit "nodeName" over "name" alias', async () => {
|
||||
const testWorkflow = createTestWorkflow();
|
||||
const updatedWorkflow = { ...testWorkflow };
|
||||
|
||||
mockApiClient.getWorkflow.mockResolvedValue(testWorkflow);
|
||||
mockDiffEngine.applyDiff.mockResolvedValue({
|
||||
success: true,
|
||||
workflow: updatedWorkflow,
|
||||
operationsApplied: 1,
|
||||
message: 'Success',
|
||||
errors: [],
|
||||
});
|
||||
mockApiClient.updateWorkflow.mockResolvedValue(updatedWorkflow);
|
||||
|
||||
await handleUpdatePartialWorkflow({
|
||||
id: 'test-workflow-id',
|
||||
operations: [{
|
||||
type: 'updateNode',
|
||||
nodeName: 'HTTP Request', // Explicit nodeName provided
|
||||
name: 'Should Be Ignored', // Should NOT override nodeName
|
||||
updates: { 'parameters.url': 'https://new-url.com' },
|
||||
}],
|
||||
}, mockRepository);
|
||||
|
||||
expect(mockDiffEngine.applyDiff).toHaveBeenCalled();
|
||||
const diffArgs = mockDiffEngine.applyDiff.mock.calls[0][1];
|
||||
expect(diffArgs.operations[0].nodeName).toBe('HTTP Request');
|
||||
});
|
||||
});
|
||||
});
|
||||
});
|
||||
@@ -1096,6 +1096,83 @@ describe('n8n-validation', () => {
|
||||
expect(disconnectedErrors).toHaveLength(0);
|
||||
});
|
||||
|
||||
it('should NOT flag nodes as disconnected when connected via ai_outputParser', () => {
|
||||
const workflow = {
|
||||
name: 'AI Output Parser Workflow',
|
||||
nodes: [
|
||||
{
|
||||
id: 'agent-1',
|
||||
name: 'AI Agent',
|
||||
type: '@n8n/n8n-nodes-langchain.agent',
|
||||
typeVersion: 1.6,
|
||||
position: [500, 300] as [number, number],
|
||||
parameters: {},
|
||||
},
|
||||
{
|
||||
id: 'parser-1',
|
||||
name: 'Structured Output Parser',
|
||||
type: '@n8n/n8n-nodes-langchain.outputParserStructured',
|
||||
typeVersion: 1,
|
||||
position: [300, 400] as [number, number],
|
||||
parameters: {},
|
||||
},
|
||||
],
|
||||
connections: {
|
||||
'Structured Output Parser': {
|
||||
ai_outputParser: [[{ node: 'AI Agent', type: 'ai_outputParser', index: 0 }]],
|
||||
},
|
||||
},
|
||||
};
|
||||
|
||||
const errors = validateWorkflowStructure(workflow);
|
||||
const disconnectedErrors = errors.filter(e => e.includes('Disconnected'));
|
||||
expect(disconnectedErrors).toHaveLength(0);
|
||||
});
|
||||
|
||||
it('should NOT flag nodes as disconnected when connected via ai_document or ai_textSplitter', () => {
|
||||
const workflow = {
|
||||
name: 'Document Processing Workflow',
|
||||
nodes: [
|
||||
{
|
||||
id: 'vs-1',
|
||||
name: 'Pinecone Vector Store',
|
||||
type: '@n8n/n8n-nodes-langchain.vectorStorePinecone',
|
||||
typeVersion: 1,
|
||||
position: [500, 300] as [number, number],
|
||||
parameters: {},
|
||||
},
|
||||
{
|
||||
id: 'doc-1',
|
||||
name: 'Default Data Loader',
|
||||
type: '@n8n/n8n-nodes-langchain.documentDefaultDataLoader',
|
||||
typeVersion: 1,
|
||||
position: [300, 400] as [number, number],
|
||||
parameters: {},
|
||||
},
|
||||
{
|
||||
id: 'splitter-1',
|
||||
name: 'Text Splitter',
|
||||
type: '@n8n/n8n-nodes-langchain.textSplitterRecursiveCharacterTextSplitter',
|
||||
typeVersion: 1,
|
||||
position: [100, 400] as [number, number],
|
||||
parameters: {},
|
||||
},
|
||||
],
|
||||
connections: {
|
||||
'Default Data Loader': {
|
||||
ai_document: [[{ node: 'Pinecone Vector Store', type: 'ai_document', index: 0 }]],
|
||||
},
|
||||
'Text Splitter': {
|
||||
ai_textSplitter: [[{ node: 'Default Data Loader', type: 'ai_textSplitter', index: 0 }]],
|
||||
},
|
||||
},
|
||||
};
|
||||
|
||||
const errors = validateWorkflowStructure(workflow);
|
||||
const disconnectedErrors = errors.filter(e => e.includes('Disconnected'));
|
||||
expect(disconnectedErrors).toHaveLength(0);
|
||||
});
|
||||
|
||||
it('should still flag truly disconnected nodes in AI workflows', () => {
|
||||
const workflow = {
|
||||
name: 'AI Workflow with Disconnected Node',
|
||||
|
||||
Reference in New Issue
Block a user