diff --git a/src/mcp/tool-docs/workflow_management/n8n-update-partial-workflow.ts b/src/mcp/tool-docs/workflow_management/n8n-update-partial-workflow.ts index a5a441e..1557966 100644 --- a/src/mcp/tool-docs/workflow_management/n8n-update-partial-workflow.ts +++ b/src/mcp/tool-docs/workflow_management/n8n-update-partial-workflow.ts @@ -96,6 +96,8 @@ Add **ignoreErrors: true** to removeConnection operations to prevent failures wh 'continueOnError breaks atomic guarantees - use with caution', 'Order matters for dependent operations (e.g., must add node before connecting to it)', 'Node references accept ID or name, but name must be unique', + 'Node names with special characters (apostrophes, quotes) work correctly since v2.15.6 (Issue #270 fixed)', + 'For best compatibility, prefer node IDs over names when dealing with special characters', 'Use "updates" property for updateNode operations: {type: "updateNode", updates: {...}}', 'cleanStaleConnections removes ALL broken connections - cannot be selective', 'replaceConnections overwrites entire connections object - all previous connections lost' diff --git a/src/services/workflow-diff-engine.ts b/src/services/workflow-diff-engine.ts index 3d00210..f213f74 100644 --- a/src/services/workflow-diff-engine.ts +++ b/src/services/workflow-diff-engine.ts @@ -384,12 +384,16 @@ export class WorkflowDiffEngine { const targetNode = this.findNode(workflow, operation.target, operation.target); if (!sourceNode) { - const availableNodes = workflow.nodes.map(n => n.name).join(', '); - return `Source node not found: "${operation.source}". Available nodes: ${availableNodes}`; + const availableNodes = workflow.nodes + .map(n => `"${n.name}" (id: ${n.id.substring(0, 8)}...)`) + .join(', '); + return `Source node not found: "${operation.source}". Available nodes: ${availableNodes}. Tip: Use node ID for names with special characters (apostrophes, quotes).`; } if (!targetNode) { - const availableNodes = workflow.nodes.map(n => n.name).join(', '); - return `Target node not found: "${operation.target}". Available nodes: ${availableNodes}`; + const availableNodes = workflow.nodes + .map(n => `"${n.name}" (id: ${n.id.substring(0, 8)}...)`) + .join(', '); + return `Target node not found: "${operation.target}". Available nodes: ${availableNodes}. Tip: Use node ID for names with special characters (apostrophes, quotes).`; } // Check if connection already exists @@ -417,10 +421,16 @@ export class WorkflowDiffEngine { const targetNode = this.findNode(workflow, operation.target, operation.target); if (!sourceNode) { - return `Source node not found: ${operation.source}`; + const availableNodes = workflow.nodes + .map(n => `"${n.name}" (id: ${n.id.substring(0, 8)}...)`) + .join(', '); + return `Source node not found: "${operation.source}". Available nodes: ${availableNodes}. Tip: Use node ID for names with special characters.`; } if (!targetNode) { - return `Target node not found: ${operation.target}`; + const availableNodes = workflow.nodes + .map(n => `"${n.name}" (id: ${n.id.substring(0, 8)}...)`) + .join(', '); + return `Target node not found: "${operation.target}". Available nodes: ${availableNodes}. Tip: Use node ID for names with special characters.`; } const sourceOutput = operation.sourceOutput || 'main'; @@ -791,23 +801,58 @@ export class WorkflowDiffEngine { } // Helper methods + + /** + * Normalize node names to handle special characters and escaping differences. + * Fixes issue #270: apostrophes and other special characters in node names. + * + * @param name - The node name to normalize + * @returns Normalized node name for comparison + */ + private normalizeNodeName(name: string): string { + return name + .trim() // Remove leading/trailing whitespace + .replace(/\\'/g, "'") // Unescape single quotes: \' -> ' + .replace(/\\"/g, '"') // Unescape double quotes: \" -> " + .replace(/\\\\/g, '\\') // Unescape backslashes: \\ -> \ + .replace(/\s+/g, ' '); // Normalize multiple spaces to single space + } + + /** + * Find a node by ID or name in the workflow. + * Uses string normalization to handle special characters (Issue #270). + * + * @param workflow - The workflow to search in + * @param nodeId - Optional node ID to search for + * @param nodeName - Optional node name to search for + * @returns The found node or null + */ private findNode(workflow: Workflow, nodeId?: string, nodeName?: string): WorkflowNode | null { + // Try to find by ID first (exact match, no normalization needed for UUIDs) if (nodeId) { const nodeById = workflow.nodes.find(n => n.id === nodeId); if (nodeById) return nodeById; } - + + // Try to find by name with normalization (handles special characters) if (nodeName) { - const nodeByName = workflow.nodes.find(n => n.name === nodeName); + const normalizedSearch = this.normalizeNodeName(nodeName); + const nodeByName = workflow.nodes.find(n => + this.normalizeNodeName(n.name) === normalizedSearch + ); if (nodeByName) return nodeByName; } - - // If nodeId is provided but not found, try treating it as a name + + // Fallback: If nodeId provided but not found, try treating it as a name + // This allows operations to work with either IDs or names flexibly if (nodeId && !nodeName) { - const nodeByName = workflow.nodes.find(n => n.name === nodeId); + const normalizedSearch = this.normalizeNodeName(nodeId); + const nodeByName = workflow.nodes.find(n => + this.normalizeNodeName(n.name) === normalizedSearch + ); if (nodeByName) return nodeByName; } - + return null; } diff --git a/tests/unit/services/workflow-diff-engine.test.ts b/tests/unit/services/workflow-diff-engine.test.ts index ca2f1a9..0bf6557 100644 --- a/tests/unit/services/workflow-diff-engine.test.ts +++ b/tests/unit/services/workflow-diff-engine.test.ts @@ -3007,4 +3007,216 @@ describe('WorkflowDiffEngine', () => { }); }); }); + + // Issue #270: Special characters in node names + describe('Special Characters in Node Names', () => { + it('should handle apostrophes in node names for addConnection', async () => { + // Default n8n Manual Trigger node name contains apostrophes + const workflowWithApostrophes = { + ...baseWorkflow, + nodes: [ + ...baseWorkflow.nodes, + { + id: 'manual-trigger-1', + name: "When clicking 'Execute workflow'", // Contains apostrophes + type: 'n8n-nodes-base.manualTrigger', + typeVersion: 1, + position: [100, 100] as [number, number], + parameters: {} + } + ] + }; + + const operation: AddConnectionOperation = { + type: 'addConnection', + source: "When clicking 'Execute workflow'", // Using node name with apostrophes + target: 'HTTP Request' + }; + + const request: WorkflowDiffRequest = { + id: 'test-workflow', + operations: [operation] + }; + + const result = await diffEngine.applyDiff(workflowWithApostrophes as Workflow, request); + + expect(result.success).toBe(true); + expect(result.workflow.connections["When clicking 'Execute workflow'"]).toBeDefined(); + expect(result.workflow.connections["When clicking 'Execute workflow'"].main).toBeDefined(); + }); + + it('should handle double quotes in node names', async () => { + const workflowWithQuotes = { + ...baseWorkflow, + nodes: [ + ...baseWorkflow.nodes, + { + id: 'quoted-node-1', + name: 'Node with "quotes"', // Contains double quotes + type: 'n8n-nodes-base.set', + typeVersion: 1, + position: [100, 100] as [number, number], + parameters: {} + } + ] + }; + + const operation: AddConnectionOperation = { + type: 'addConnection', + source: 'Node with "quotes"', + target: 'HTTP Request' + }; + + const request: WorkflowDiffRequest = { + id: 'test-workflow', + operations: [operation] + }; + + const result = await diffEngine.applyDiff(workflowWithQuotes as Workflow, request); + + expect(result.success).toBe(true); + expect(result.workflow.connections['Node with "quotes"']).toBeDefined(); + }); + + it('should handle backslashes in node names', async () => { + const workflowWithBackslashes = { + ...baseWorkflow, + nodes: [ + ...baseWorkflow.nodes, + { + id: 'backslash-node-1', + name: 'Path\\with\\backslashes', // Contains backslashes + type: 'n8n-nodes-base.set', + typeVersion: 1, + position: [100, 100] as [number, number], + parameters: {} + } + ] + }; + + const operation: AddConnectionOperation = { + type: 'addConnection', + source: 'Path\\with\\backslashes', + target: 'HTTP Request' + }; + + const request: WorkflowDiffRequest = { + id: 'test-workflow', + operations: [operation] + }; + + const result = await diffEngine.applyDiff(workflowWithBackslashes as Workflow, request); + + expect(result.success).toBe(true); + expect(result.workflow.connections['Path\\with\\backslashes']).toBeDefined(); + }); + + it('should handle mixed special characters in node names', async () => { + const workflowWithMixed = { + ...baseWorkflow, + nodes: [ + ...baseWorkflow.nodes, + { + id: 'complex-node-1', + name: "Complex 'name' with \"quotes\" and \\backslash", + type: 'n8n-nodes-base.set', + typeVersion: 1, + position: [100, 100] as [number, number], + parameters: {} + } + ] + }; + + const operation: AddConnectionOperation = { + type: 'addConnection', + source: "Complex 'name' with \"quotes\" and \\backslash", + target: 'HTTP Request' + }; + + const request: WorkflowDiffRequest = { + id: 'test-workflow', + operations: [operation] + }; + + const result = await diffEngine.applyDiff(workflowWithMixed as Workflow, request); + + expect(result.success).toBe(true); + expect(result.workflow.connections["Complex 'name' with \"quotes\" and \\backslash"]).toBeDefined(); + }); + + it('should handle special characters in removeConnection', async () => { + const workflowWithConnections = { + ...baseWorkflow, + nodes: [ + ...baseWorkflow.nodes, + { + id: 'apostrophe-node-1', + name: "Node with 'apostrophes'", + type: 'n8n-nodes-base.set', + typeVersion: 1, + position: [100, 100] as [number, number], + parameters: {} + } + ], + connections: { + ...baseWorkflow.connections, + "Node with 'apostrophes'": { + main: [[{ node: 'HTTP Request', type: 'main', index: 0 }]] + } + } + }; + + const operation: RemoveConnectionOperation = { + type: 'removeConnection', + source: "Node with 'apostrophes'", + target: 'HTTP Request' + }; + + const request: WorkflowDiffRequest = { + id: 'test-workflow', + operations: [operation] + }; + + const result = await diffEngine.applyDiff(workflowWithConnections as any, request); + + expect(result.success).toBe(true); + expect(result.workflow.connections["Node with 'apostrophes'"]).toBeUndefined(); + }); + + it('should handle special characters in updateNode', async () => { + const workflowWithSpecialNode = { + ...baseWorkflow, + nodes: [ + ...baseWorkflow.nodes, + { + id: 'special-node-1', + name: "Update 'this' node", + type: 'n8n-nodes-base.set', + typeVersion: 1, + position: [100, 100] as [number, number], + parameters: { value: 'old' } + } + ] + }; + + const operation: UpdateNodeOperation = { + type: 'updateNode', + nodeName: "Update 'this' node", + updates: { + 'parameters.value': 'new' + } + }; + + const request: WorkflowDiffRequest = { + id: 'test-workflow', + operations: [operation] + }; + + const result = await diffEngine.applyDiff(workflowWithSpecialNode as Workflow, request); + + expect(result.success).toBe(true); + const updatedNode = result.workflow.nodes.find((n: any) => n.name === "Update 'this' node"); + expect(updatedNode?.parameters.value).toBe('new'); + }); + }); }); \ No newline at end of file diff --git a/tests/unit/services/workflow-validator-edge-cases.test.ts b/tests/unit/services/workflow-validator-edge-cases.test.ts index c1547d8..c897344 100644 --- a/tests/unit/services/workflow-validator-edge-cases.test.ts +++ b/tests/unit/services/workflow-validator-edge-cases.test.ts @@ -365,7 +365,28 @@ describe('WorkflowValidator - Edge Cases', () => { }); describe('Special Characters and Unicode', () => { - it.skip('should handle special characters in node names - FIXME: mock issues', async () => { + // Note: These tests are skipped because WorkflowValidator also needs special character + // normalization (similar to WorkflowDiffEngine fix in #270). Will be addressed in a future PR. + it.skip('should handle apostrophes in node names - TODO: needs WorkflowValidator normalization', async () => { + // Test default n8n Manual Trigger node name with apostrophes + const workflow = { + nodes: [ + { id: '1', name: "When clicking 'Execute workflow'", type: 'n8n-nodes-base.manualTrigger', position: [0, 0] as [number, number], parameters: {} }, + { id: '2', name: 'HTTP Request', type: 'n8n-nodes-base.httpRequest', position: [100, 0] as [number, number], parameters: {} } + ], + connections: { + "When clicking 'Execute workflow'": { + main: [[{ node: 'HTTP Request', type: 'main', index: 0 }]] + } + } + }; + + const result = await validator.validateWorkflow(workflow as any); + expect(result.valid).toBe(true); + expect(result.errors).toHaveLength(0); + }); + + it.skip('should handle special characters in node names - TODO: needs WorkflowValidator normalization', async () => { const workflow = { nodes: [ { id: '1', name: 'Node@#$%', type: 'n8n-nodes-base.set', position: [0, 0] as [number, number], parameters: {} }, @@ -381,9 +402,10 @@ describe('WorkflowValidator - Edge Cases', () => { } } }; - + const result = await validator.validateWorkflow(workflow as any); expect(result.valid).toBe(true); + expect(result.errors).toHaveLength(0); }); it('should handle very long node names', async () => {