From 13c166348945640043cc579bb0ad3fc767e7c219 Mon Sep 17 00:00:00 2001 From: czlonkowski <56956555+czlonkowski@users.noreply.github.com> Date: Thu, 7 Aug 2025 20:05:57 +0200 Subject: [PATCH] fix: address critical code review issues for validation improvements - Fix type safety vulnerability in enhanced-config-validator.ts - Added proper type checking before string operations - Return early when nodeType is invalid instead of using empty string - Improve error handling robustness in MCP server - Wrapped validation in try-catch to handle unexpected errors - Properly re-throw ValidationError instances - Add user-friendly error messages for internal errors - Write comprehensive CHANGELOG entry for v2.10.3 - Document fixes for issues #58, #68, #70, #73 - Detail new validation system features - List all enhancements and test coverage Addressed HIGH priority issues from code review: - Type safety holes in config validator - Missing error handling for validation system failures - Consistent error types across validation tools --- CLAUDE.md | 3 + data/nodes.db | Bin 12296192 -> 12296192 bytes docs/CHANGELOG.md | 45 ++ package.json | 2 +- src/mcp/server.ts | 105 ++++- src/services/enhanced-config-validator.ts | 24 + src/services/workflow-validator.ts | 159 +++++-- src/utils/validation-schemas.ts | 312 +++++++++++++ .../workflow-validator-comprehensive.test.ts | 130 +++--- .../workflow-validator-edge-cases.test.ts | 28 +- .../workflow-validator-with-mocks.test.ts | 16 +- tests/unit/validation-fixes.test.ts | 410 ++++++++++++++++++ 12 files changed, 1106 insertions(+), 128 deletions(-) create mode 100644 src/utils/validation-schemas.ts create mode 100644 tests/unit/validation-fixes.test.ts diff --git a/CLAUDE.md b/CLAUDE.md index 6ad3701..bc6247e 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -180,6 +180,9 @@ The MCP server exposes tools in several categories: - Sub-agents are not allowed to spawn further sub-agents - When you use sub-agents, do not allow them to commit and push. That should be done by you +### Development Best Practices +- Run typecheck and lint after every code change + # important-instruction-reminders Do what has been asked; nothing more, nothing less. NEVER create files unless they're absolutely necessary for achieving your goal. diff --git a/data/nodes.db b/data/nodes.db index 77d1425800d1f8f55b20136dd53e77b0467b0806..bdd9a1de2bc1c82c8a30a89bfa00bb27f1d82638 100644 GIT binary patch delta 707 zcmWmA)l!xL7)9al=NAhD1G~G$7Q4G!F|b>g-HNqr1+hhJ>_#z8xB+j)@e+L9GtbOE zUWHYEG7Cdd%LZjhlJzr5(xpj~WF|?@@8VtqQe~eCa#AJZ(#^kIO;w~)r6pAv%}(d1 z&*i66AtU_%)k0>d9%_V|p;o9JvO=9uH`ELDLw0Bo8iq!pamWcxLetPJG!HF8%g`#c z4sAl)&@QwO9YV*@DRd59Lf6nObPqj3&(JIM4t+x3kQ@4i{$W5E7zTyGVMrKSnV-&k z{x&;G{9m=G@;y{mW)}?qTi>d@@~UBv%0K31%WxSXBW09~mN7C`#>se@AQNShOqMA! zRi?>wnISV}mdutpGFRrwd|4n1WsxkFC9+hONxm$X6|z!RNr9}EHL_OL$$Hr!8)cJh zmMyYXw#jzcAv*=xCA(#h?3I18Uk=DYIV6YWh#Zw;a$HVGp`4Uca$3&FSy9f(dAT4L z<&s>MD{@t?Ns$!Gbt#b>a#L=}ZMh?N<(}M^Qh6X{@=zYhV|gM^<(WK}a(N*y<(0ga hH}Y2A$$R-AALWyLmM>BvU*(%r%6Iu8Kg;=*`wzC;4, result: EnhancedValidationResult ): void { + // Type safety check - this should never happen with proper validation + if (typeof nodeType !== 'string') { + result.errors.push({ + type: 'invalid_type', + property: 'nodeType', + message: `Invalid nodeType: expected string, got ${typeof nodeType}`, + fix: 'Provide a valid node type string (e.g., "nodes-base.webhook")' + }); + return; + } + // First, validate fixedCollection properties for known problematic nodes this.validateFixedCollectionStructures(nodeType, config, result); diff --git a/src/services/workflow-validator.ts b/src/services/workflow-validator.ts index bc3127e..7003839 100644 --- a/src/services/workflow-validator.ts +++ b/src/services/workflow-validator.ts @@ -79,6 +79,18 @@ export class WorkflowValidator { private nodeValidator: typeof EnhancedConfigValidator ) {} + /** + * Check if a node is a Sticky Note or other non-executable node + */ + private isStickyNote(node: WorkflowNode): boolean { + const stickyNoteTypes = [ + 'n8n-nodes-base.stickyNote', + 'nodes-base.stickyNote', + '@n8n/n8n-nodes-base.stickyNote' + ]; + return stickyNoteTypes.includes(node.type); + } + /** * Validate a complete workflow */ @@ -127,9 +139,10 @@ export class WorkflowValidator { return result; } - // Update statistics after null check - result.statistics.totalNodes = Array.isArray(workflow.nodes) ? workflow.nodes.length : 0; - result.statistics.enabledNodes = Array.isArray(workflow.nodes) ? workflow.nodes.filter(n => !n.disabled).length : 0; + // Update statistics after null check (exclude sticky notes from counts) + const executableNodes = Array.isArray(workflow.nodes) ? workflow.nodes.filter(n => !this.isStickyNote(n)) : []; + result.statistics.totalNodes = executableNodes.length; + result.statistics.enabledNodes = executableNodes.filter(n => !n.disabled).length; // Basic workflow structure validation this.validateWorkflowStructure(workflow, result); @@ -143,21 +156,26 @@ export class WorkflowValidator { // Validate connections if requested if (validateConnections) { - this.validateConnections(workflow, result); + this.validateConnections(workflow, result, profile); } // Validate expressions if requested if (validateExpressions && workflow.nodes.length > 0) { - this.validateExpressions(workflow, result); + this.validateExpressions(workflow, result, profile); } // Check workflow patterns and best practices if (workflow.nodes.length > 0) { - this.checkWorkflowPatterns(workflow, result); + this.checkWorkflowPatterns(workflow, result, profile); } // Add suggestions based on findings this.generateSuggestions(workflow, result); + + // Add AI-specific recovery suggestions if there are errors + if (result.errors.length > 0) { + this.addErrorRecoverySuggestions(result); + } } } catch (error) { @@ -308,7 +326,7 @@ export class WorkflowValidator { profile: string ): Promise { for (const node of workflow.nodes) { - if (node.disabled) continue; + if (node.disabled || this.isStickyNote(node)) continue; try { // Validate node name length @@ -500,7 +518,8 @@ export class WorkflowValidator { */ private validateConnections( workflow: WorkflowJson, - result: WorkflowValidationResult + result: WorkflowValidationResult, + profile: string = 'runtime' ): void { const nodeMap = new Map(workflow.nodes.map(n => [n.name, n])); const nodeIdMap = new Map(workflow.nodes.map(n => [n.id, n])); @@ -591,9 +610,9 @@ export class WorkflowValidator { } }); - // Check for orphaned nodes + // Check for orphaned nodes (exclude sticky notes) for (const node of workflow.nodes) { - if (node.disabled) continue; + if (node.disabled || this.isStickyNote(node)) continue; const normalizedType = node.type.replace('n8n-nodes-base.', 'nodes-base.'); const isTrigger = normalizedType.toLowerCase().includes('trigger') || @@ -612,8 +631,8 @@ export class WorkflowValidator { } } - // Check for cycles - if (this.hasCycle(workflow)) { + // Check for cycles (skip in minimal profile to reduce false positives) + if (profile !== 'minimal' && this.hasCycle(workflow)) { result.errors.push({ type: 'error', message: 'Workflow contains a cycle (infinite loop)' @@ -757,10 +776,22 @@ export class WorkflowValidator { const recursionStack = new Set(); const nodeTypeMap = new Map(); - // Build node type map + // Build node type map (exclude sticky notes) workflow.nodes.forEach(node => { - nodeTypeMap.set(node.name, node.type); + if (!this.isStickyNote(node)) { + nodeTypeMap.set(node.name, node.type); + } }); + + // Known legitimate loop node types + const loopNodeTypes = [ + 'n8n-nodes-base.splitInBatches', + 'nodes-base.splitInBatches', + 'n8n-nodes-base.itemLists', + 'nodes-base.itemLists', + 'n8n-nodes-base.loop', + 'nodes-base.loop' + ]; const hasCycleDFS = (nodeName: string, pathFromLoopNode: boolean = false): boolean => { visited.add(nodeName); @@ -789,18 +820,18 @@ export class WorkflowValidator { } const currentNodeType = nodeTypeMap.get(nodeName); - const isLoopNode = currentNodeType === 'n8n-nodes-base.splitInBatches'; + const isLoopNode = loopNodeTypes.includes(currentNodeType || ''); for (const target of allTargets) { if (!visited.has(target)) { if (hasCycleDFS(target, pathFromLoopNode || isLoopNode)) return true; } else if (recursionStack.has(target)) { - // Allow cycles that involve loop nodes like SplitInBatches + // Allow cycles that involve legitimate loop nodes const targetNodeType = nodeTypeMap.get(target); - const isTargetLoopNode = targetNodeType === 'n8n-nodes-base.splitInBatches'; + const isTargetLoopNode = loopNodeTypes.includes(targetNodeType || ''); // If this cycle involves a loop node, it's legitimate - if (isTargetLoopNode || pathFromLoopNode) { + if (isTargetLoopNode || pathFromLoopNode || isLoopNode) { continue; // Allow this cycle } @@ -813,9 +844,9 @@ export class WorkflowValidator { return false; }; - // Check from all nodes + // Check from all executable nodes (exclude sticky notes) for (const node of workflow.nodes) { - if (!visited.has(node.name)) { + if (!this.isStickyNote(node) && !visited.has(node.name)) { if (hasCycleDFS(node.name)) return true; } } @@ -828,12 +859,13 @@ export class WorkflowValidator { */ private validateExpressions( workflow: WorkflowJson, - result: WorkflowValidationResult + result: WorkflowValidationResult, + profile: string = 'runtime' ): void { const nodeNames = workflow.nodes.map(n => n.name); for (const node of workflow.nodes) { - if (node.disabled) continue; + if (node.disabled || this.isStickyNote(node)) continue; // Create expression context const context = { @@ -922,23 +954,27 @@ export class WorkflowValidator { */ private checkWorkflowPatterns( workflow: WorkflowJson, - result: WorkflowValidationResult + result: WorkflowValidationResult, + profile: string = 'runtime' ): void { // Check for error handling const hasErrorHandling = Object.values(workflow.connections).some( outputs => outputs.error && outputs.error.length > 0 ); - if (!hasErrorHandling && workflow.nodes.length > 3) { + // Only suggest error handling in stricter profiles + if (!hasErrorHandling && workflow.nodes.length > 3 && profile !== 'minimal') { result.warnings.push({ type: 'warning', message: 'Consider adding error handling to your workflow' }); } - // Check node-level error handling properties for ALL nodes + // Check node-level error handling properties for ALL executable nodes for (const node of workflow.nodes) { - this.checkNodeErrorHandling(node, workflow, result); + if (!this.isStickyNote(node)) { + this.checkNodeErrorHandling(node, workflow, result); + } } // Check for very long linear workflows @@ -1641,4 +1677,75 @@ export class WorkflowValidator { return false; } + + /** + * Add AI-specific error recovery suggestions + */ + private addErrorRecoverySuggestions(result: WorkflowValidationResult): void { + // Categorize errors and provide specific recovery actions + const errorTypes = { + nodeType: result.errors.filter(e => e.message.includes('node type') || e.message.includes('Node type')), + connection: result.errors.filter(e => e.message.includes('connection') || e.message.includes('Connection')), + structure: result.errors.filter(e => e.message.includes('structure') || e.message.includes('nodes must be')), + configuration: result.errors.filter(e => e.message.includes('property') || e.message.includes('field')), + typeVersion: result.errors.filter(e => e.message.includes('typeVersion')) + }; + + // Add recovery suggestions based on error types + if (errorTypes.nodeType.length > 0) { + result.suggestions.unshift( + '🔧 RECOVERY: Invalid node types detected. Use these patterns:', + ' • For core nodes: "n8n-nodes-base.nodeName" (e.g., "n8n-nodes-base.webhook")', + ' • For AI nodes: "@n8n/n8n-nodes-langchain.nodeName"', + ' • Never use just the node name without package prefix' + ); + } + + if (errorTypes.connection.length > 0) { + result.suggestions.unshift( + '🔧 RECOVERY: Connection errors detected. Fix with:', + ' • Use node NAMES in connections, not IDs or types', + ' • Structure: { "Source Node Name": { "main": [[{ "node": "Target Node Name", "type": "main", "index": 0 }]] } }', + ' • Ensure all referenced nodes exist in the workflow' + ); + } + + if (errorTypes.structure.length > 0) { + result.suggestions.unshift( + '🔧 RECOVERY: Workflow structure errors. Fix with:', + ' • Ensure "nodes" is an array: "nodes": [...]', + ' • Ensure "connections" is an object: "connections": {...}', + ' • Add at least one node to create a valid workflow' + ); + } + + if (errorTypes.configuration.length > 0) { + result.suggestions.unshift( + '🔧 RECOVERY: Node configuration errors. Fix with:', + ' • Check required fields using validate_node_minimal first', + ' • Use get_node_essentials to see what fields are needed', + ' • Ensure operation-specific fields match the node\'s requirements' + ); + } + + if (errorTypes.typeVersion.length > 0) { + result.suggestions.unshift( + '🔧 RECOVERY: TypeVersion errors. Fix with:', + ' • Add "typeVersion": 1 (or latest version) to each node', + ' • Use get_node_info to check the correct version for each node type' + ); + } + + // Add general recovery workflow + if (result.errors.length > 3) { + result.suggestions.push( + '📋 SUGGESTED WORKFLOW: Too many errors detected. Try this approach:', + ' 1. Fix structural issues first (nodes array, connections object)', + ' 2. Validate node types and fix invalid ones', + ' 3. Add required typeVersion to all nodes', + ' 4. Test connections step by step', + ' 5. Use validate_node_minimal on individual nodes to verify configuration' + ); + } + } } \ No newline at end of file diff --git a/src/utils/validation-schemas.ts b/src/utils/validation-schemas.ts new file mode 100644 index 0000000..fe7d1a3 --- /dev/null +++ b/src/utils/validation-schemas.ts @@ -0,0 +1,312 @@ +/** + * Zod validation schemas for MCP tool parameters + * Provides robust input validation with detailed error messages + */ + +// Simple validation without zod for now, since it's not installed +// We can use TypeScript's built-in validation with better error messages + +export class ValidationError extends Error { + constructor(message: string, public field?: string, public value?: any) { + super(message); + this.name = 'ValidationError'; + } +} + +export interface ValidationResult { + valid: boolean; + errors: Array<{ + field: string; + message: string; + value?: any; + }>; +} + +/** + * Basic validation utilities + */ +export class Validator { + /** + * Validate that a value is a non-empty string + */ + static validateString(value: any, fieldName: string, required: boolean = true): ValidationResult { + const errors: Array<{field: string, message: string, value?: any}> = []; + + if (required && (value === undefined || value === null)) { + errors.push({ + field: fieldName, + message: `${fieldName} is required`, + value + }); + } else if (value !== undefined && value !== null && typeof value !== 'string') { + errors.push({ + field: fieldName, + message: `${fieldName} must be a string, got ${typeof value}`, + value + }); + } else if (required && typeof value === 'string' && value.trim().length === 0) { + errors.push({ + field: fieldName, + message: `${fieldName} cannot be empty`, + value + }); + } + + return { + valid: errors.length === 0, + errors + }; + } + + /** + * Validate that a value is a valid object (not null, not array) + */ + static validateObject(value: any, fieldName: string, required: boolean = true): ValidationResult { + const errors: Array<{field: string, message: string, value?: any}> = []; + + if (required && (value === undefined || value === null)) { + errors.push({ + field: fieldName, + message: `${fieldName} is required`, + value + }); + } else if (value !== undefined && value !== null) { + if (typeof value !== 'object') { + errors.push({ + field: fieldName, + message: `${fieldName} must be an object, got ${typeof value}`, + value + }); + } else if (Array.isArray(value)) { + errors.push({ + field: fieldName, + message: `${fieldName} must be an object, not an array`, + value + }); + } + } + + return { + valid: errors.length === 0, + errors + }; + } + + /** + * Validate that a value is an array + */ + static validateArray(value: any, fieldName: string, required: boolean = true): ValidationResult { + const errors: Array<{field: string, message: string, value?: any}> = []; + + if (required && (value === undefined || value === null)) { + errors.push({ + field: fieldName, + message: `${fieldName} is required`, + value + }); + } else if (value !== undefined && value !== null && !Array.isArray(value)) { + errors.push({ + field: fieldName, + message: `${fieldName} must be an array, got ${typeof value}`, + value + }); + } + + return { + valid: errors.length === 0, + errors + }; + } + + /** + * Validate that a value is a number + */ + static validateNumber(value: any, fieldName: string, required: boolean = true, min?: number, max?: number): ValidationResult { + const errors: Array<{field: string, message: string, value?: any}> = []; + + if (required && (value === undefined || value === null)) { + errors.push({ + field: fieldName, + message: `${fieldName} is required`, + value + }); + } else if (value !== undefined && value !== null) { + if (typeof value !== 'number' || isNaN(value)) { + errors.push({ + field: fieldName, + message: `${fieldName} must be a number, got ${typeof value}`, + value + }); + } else { + if (min !== undefined && value < min) { + errors.push({ + field: fieldName, + message: `${fieldName} must be at least ${min}, got ${value}`, + value + }); + } + if (max !== undefined && value > max) { + errors.push({ + field: fieldName, + message: `${fieldName} must be at most ${max}, got ${value}`, + value + }); + } + } + } + + return { + valid: errors.length === 0, + errors + }; + } + + /** + * Validate that a value is one of allowed values + */ + static validateEnum(value: any, fieldName: string, allowedValues: T[], required: boolean = true): ValidationResult { + const errors: Array<{field: string, message: string, value?: any}> = []; + + if (required && (value === undefined || value === null)) { + errors.push({ + field: fieldName, + message: `${fieldName} is required`, + value + }); + } else if (value !== undefined && value !== null && !allowedValues.includes(value)) { + errors.push({ + field: fieldName, + message: `${fieldName} must be one of: ${allowedValues.join(', ')}, got "${value}"`, + value + }); + } + + return { + valid: errors.length === 0, + errors + }; + } + + /** + * Combine multiple validation results + */ + static combineResults(...results: ValidationResult[]): ValidationResult { + const allErrors = results.flatMap(r => r.errors); + return { + valid: allErrors.length === 0, + errors: allErrors + }; + } + + /** + * Create a detailed error message from validation result + */ + static formatErrors(result: ValidationResult, toolName?: string): string { + if (result.valid) return ''; + + const prefix = toolName ? `${toolName}: ` : ''; + const errors = result.errors.map(e => ` • ${e.field}: ${e.message}`).join('\n'); + + return `${prefix}Validation failed:\n${errors}`; + } +} + +/** + * Tool-specific validation schemas + */ +export class ToolValidation { + /** + * Validate parameters for validate_node_operation tool + */ + static validateNodeOperation(args: any): ValidationResult { + const nodeTypeResult = Validator.validateString(args.nodeType, 'nodeType'); + const configResult = Validator.validateObject(args.config, 'config'); + const profileResult = Validator.validateEnum( + args.profile, + 'profile', + ['minimal', 'runtime', 'ai-friendly', 'strict'], + false // optional + ); + + return Validator.combineResults(nodeTypeResult, configResult, profileResult); + } + + /** + * Validate parameters for validate_node_minimal tool + */ + static validateNodeMinimal(args: any): ValidationResult { + const nodeTypeResult = Validator.validateString(args.nodeType, 'nodeType'); + const configResult = Validator.validateObject(args.config, 'config'); + + return Validator.combineResults(nodeTypeResult, configResult); + } + + /** + * Validate parameters for validate_workflow tool + */ + static validateWorkflow(args: any): ValidationResult { + const workflowResult = Validator.validateObject(args.workflow, 'workflow'); + + // Validate workflow structure if it's an object + let nodesResult: ValidationResult = { valid: true, errors: [] }; + let connectionsResult: ValidationResult = { valid: true, errors: [] }; + + if (workflowResult.valid && args.workflow) { + nodesResult = Validator.validateArray(args.workflow.nodes, 'workflow.nodes'); + connectionsResult = Validator.validateObject(args.workflow.connections, 'workflow.connections'); + } + + const optionsResult = args.options ? + Validator.validateObject(args.options, 'options', false) : + { valid: true, errors: [] }; + + return Validator.combineResults(workflowResult, nodesResult, connectionsResult, optionsResult); + } + + /** + * Validate parameters for search_nodes tool + */ + static validateSearchNodes(args: any): ValidationResult { + const queryResult = Validator.validateString(args.query, 'query'); + const limitResult = Validator.validateNumber(args.limit, 'limit', false, 1, 200); + const modeResult = Validator.validateEnum( + args.mode, + 'mode', + ['OR', 'AND', 'FUZZY'], + false + ); + + return Validator.combineResults(queryResult, limitResult, modeResult); + } + + /** + * Validate parameters for list_node_templates tool + */ + static validateListNodeTemplates(args: any): ValidationResult { + const nodeTypesResult = Validator.validateArray(args.nodeTypes, 'nodeTypes'); + const limitResult = Validator.validateNumber(args.limit, 'limit', false, 1, 50); + + return Validator.combineResults(nodeTypesResult, limitResult); + } + + /** + * Validate parameters for n8n workflow operations + */ + static validateWorkflowId(args: any): ValidationResult { + return Validator.validateString(args.id, 'id'); + } + + /** + * Validate parameters for n8n_create_workflow tool + */ + static validateCreateWorkflow(args: any): ValidationResult { + const nameResult = Validator.validateString(args.name, 'name'); + const nodesResult = Validator.validateArray(args.nodes, 'nodes'); + const connectionsResult = Validator.validateObject(args.connections, 'connections'); + const settingsResult = args.settings ? + Validator.validateObject(args.settings, 'settings', false) : + { valid: true, errors: [] }; + + return Validator.combineResults(nameResult, nodesResult, connectionsResult, settingsResult); + } +} \ 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 f61afd1..8b9e536 100644 --- a/tests/unit/services/workflow-validator-comprehensive.test.ts +++ b/tests/unit/services/workflow-validator-comprehensive.test.ts @@ -223,7 +223,7 @@ describe('WorkflowValidator - Comprehensive Tests', () => { it('should error when nodes array is missing', async () => { const workflow = { connections: {} } as any; - const result = await validator.validateWorkflow(workflow); + const result = await validator.validateWorkflow(workflow as any); expect(result.valid).toBe(false); expect(result.errors.some(e => e.message === 'Workflow must have a nodes array')).toBe(true); @@ -232,7 +232,7 @@ describe('WorkflowValidator - Comprehensive Tests', () => { it('should error when connections object is missing', async () => { const workflow = { nodes: [] } as any; - const result = await validator.validateWorkflow(workflow); + const result = await validator.validateWorkflow(workflow as any); expect(result.valid).toBe(false); expect(result.errors.some(e => e.message === 'Workflow must have a connections object')).toBe(true); @@ -241,7 +241,7 @@ describe('WorkflowValidator - Comprehensive Tests', () => { it('should warn when workflow has no nodes', async () => { const workflow = { nodes: [], connections: {} } as any; - const result = await validator.validateWorkflow(workflow); + const result = await validator.validateWorkflow(workflow as any); expect(result.valid).toBe(true); // Empty workflows are valid but get a warning expect(result.warnings).toHaveLength(1); @@ -260,7 +260,7 @@ describe('WorkflowValidator - Comprehensive Tests', () => { connections: {} } as any; - const result = await validator.validateWorkflow(workflow); + const result = await validator.validateWorkflow(workflow as any); expect(result.valid).toBe(false); expect(result.errors.some(e => e.message.includes('Single-node workflows are only valid for webhook endpoints'))).toBe(true); @@ -279,7 +279,7 @@ describe('WorkflowValidator - Comprehensive Tests', () => { connections: {} } as any; - const result = await validator.validateWorkflow(workflow); + const result = await validator.validateWorkflow(workflow as any); expect(result.valid).toBe(true); expect(result.warnings.some(w => w.message.includes('Webhook node has no connections'))).toBe(true); @@ -306,7 +306,7 @@ describe('WorkflowValidator - Comprehensive Tests', () => { connections: {} } as any; - const result = await validator.validateWorkflow(workflow); + const result = await validator.validateWorkflow(workflow as any); expect(result.valid).toBe(false); expect(result.errors.some(e => e.message.includes('Multi-node workflow has no connections'))).toBe(true); @@ -333,7 +333,7 @@ describe('WorkflowValidator - Comprehensive Tests', () => { connections: {} } as any; - const result = await validator.validateWorkflow(workflow); + const result = await validator.validateWorkflow(workflow as any); expect(result.errors.some(e => e.message.includes('Duplicate node name: "Webhook"'))).toBe(true); }); @@ -359,7 +359,7 @@ describe('WorkflowValidator - Comprehensive Tests', () => { connections: {} } as any; - const result = await validator.validateWorkflow(workflow); + const result = await validator.validateWorkflow(workflow as any); expect(result.errors.some(e => e.message.includes('Duplicate node ID: "1"'))).toBe(true); }); @@ -392,7 +392,7 @@ describe('WorkflowValidator - Comprehensive Tests', () => { connections: {} } as any; - const result = await validator.validateWorkflow(workflow); + const result = await validator.validateWorkflow(workflow as any); expect(result.statistics.triggerNodes).toBe(3); }); @@ -422,7 +422,7 @@ describe('WorkflowValidator - Comprehensive Tests', () => { } } as any; - const result = await validator.validateWorkflow(workflow); + const result = await validator.validateWorkflow(workflow as any); expect(result.warnings.some(w => w.message.includes('Workflow has no trigger nodes'))).toBe(true); }); @@ -449,7 +449,7 @@ describe('WorkflowValidator - Comprehensive Tests', () => { connections: {} } as any; - const result = await validator.validateWorkflow(workflow); + const result = await validator.validateWorkflow(workflow as any); expect(result.statistics.totalNodes).toBe(2); expect(result.statistics.enabledNodes).toBe(1); @@ -472,7 +472,7 @@ describe('WorkflowValidator - Comprehensive Tests', () => { connections: {} } as any; - const result = await validator.validateWorkflow(workflow); + const result = await validator.validateWorkflow(workflow as any); expect(mockNodeRepository.getNode).not.toHaveBeenCalled(); }); @@ -491,7 +491,7 @@ describe('WorkflowValidator - Comprehensive Tests', () => { connections: {} } as any; - const result = await validator.validateWorkflow(workflow); + const result = await validator.validateWorkflow(workflow as any); expect(result.valid).toBe(false); expect(result.errors.some(e => e.message.includes('Invalid node type: "nodes-base.webhook"'))).toBe(true); @@ -512,7 +512,7 @@ describe('WorkflowValidator - Comprehensive Tests', () => { connections: {} } as any; - const result = await validator.validateWorkflow(workflow); + const result = await validator.validateWorkflow(workflow as any); expect(result.valid).toBe(false); expect(result.errors.some(e => e.message.includes('Unknown node type: "httpRequest"'))).toBe(true); @@ -533,7 +533,7 @@ describe('WorkflowValidator - Comprehensive Tests', () => { connections: {} } as any; - const result = await validator.validateWorkflow(workflow); + const result = await validator.validateWorkflow(workflow as any); expect(mockNodeRepository.getNode).toHaveBeenCalledWith('n8n-nodes-base.webhook'); expect(mockNodeRepository.getNode).toHaveBeenCalledWith('nodes-base.webhook'); @@ -553,7 +553,7 @@ describe('WorkflowValidator - Comprehensive Tests', () => { connections: {} } as any; - const result = await validator.validateWorkflow(workflow); + const result = await validator.validateWorkflow(workflow as any); expect(mockNodeRepository.getNode).toHaveBeenCalledWith('@n8n/n8n-nodes-langchain.agent'); expect(mockNodeRepository.getNode).toHaveBeenCalledWith('nodes-langchain.agent'); @@ -574,7 +574,7 @@ describe('WorkflowValidator - Comprehensive Tests', () => { connections: {} } as any; - const result = await validator.validateWorkflow(workflow); + const result = await validator.validateWorkflow(workflow as any); expect(result.errors.some(e => e.message.includes('Missing required property \'typeVersion\''))).toBe(true); }); @@ -594,7 +594,7 @@ describe('WorkflowValidator - Comprehensive Tests', () => { connections: {} } as any; - const result = await validator.validateWorkflow(workflow); + const result = await validator.validateWorkflow(workflow as any); expect(result.errors.some(e => e.message.includes('Invalid typeVersion: invalid'))).toBe(true); }); @@ -614,7 +614,7 @@ describe('WorkflowValidator - Comprehensive Tests', () => { connections: {} } as any; - const result = await validator.validateWorkflow(workflow); + const result = await validator.validateWorkflow(workflow as any); expect(result.warnings.some(w => w.message.includes('Outdated typeVersion: 1. Latest is 2'))).toBe(true); }); @@ -634,7 +634,7 @@ describe('WorkflowValidator - Comprehensive Tests', () => { connections: {} } as any; - const result = await validator.validateWorkflow(workflow); + const result = await validator.validateWorkflow(workflow as any); expect(result.errors.some(e => e.message.includes('typeVersion 10 exceeds maximum supported version 2'))).toBe(true); }); @@ -664,7 +664,7 @@ describe('WorkflowValidator - Comprehensive Tests', () => { connections: {} } as any; - const result = await validator.validateWorkflow(workflow); + const result = await validator.validateWorkflow(workflow as any); expect(result.errors.some(e => e.message.includes('Missing required field: url'))).toBe(true); expect(result.warnings.some(w => w.message.includes('Consider using HTTPS'))).toBe(true); @@ -689,7 +689,7 @@ describe('WorkflowValidator - Comprehensive Tests', () => { connections: {} } as any; - const result = await validator.validateWorkflow(workflow); + const result = await validator.validateWorkflow(workflow as any); expect(result.errors.some(e => e.message.includes('Failed to validate node: Validation error'))).toBe(true); }); @@ -721,7 +721,7 @@ describe('WorkflowValidator - Comprehensive Tests', () => { } } as any; - const result = await validator.validateWorkflow(workflow); + const result = await validator.validateWorkflow(workflow as any); expect(result.statistics.validConnections).toBe(1); expect(result.statistics.invalidConnections).toBe(0); @@ -745,7 +745,7 @@ describe('WorkflowValidator - Comprehensive Tests', () => { } } as any; - const result = await validator.validateWorkflow(workflow); + const result = await validator.validateWorkflow(workflow as any); expect(result.errors.some(e => e.message.includes('Connection from non-existent node: "NonExistent"'))).toBe(true); expect(result.statistics.invalidConnections).toBe(1); @@ -776,7 +776,7 @@ describe('WorkflowValidator - Comprehensive Tests', () => { } } as any; - const result = await validator.validateWorkflow(workflow); + const result = await validator.validateWorkflow(workflow as any); expect(result.errors.some(e => e.message.includes('Connection uses node ID \'webhook-id\' instead of node name \'Webhook\''))).toBe(true); }); @@ -799,7 +799,7 @@ describe('WorkflowValidator - Comprehensive Tests', () => { } } as any; - const result = await validator.validateWorkflow(workflow); + const result = await validator.validateWorkflow(workflow as any); expect(result.errors.some(e => e.message.includes('Connection to non-existent node: "NonExistent"'))).toBe(true); expect(result.statistics.invalidConnections).toBe(1); @@ -830,7 +830,7 @@ describe('WorkflowValidator - Comprehensive Tests', () => { } } as any; - const result = await validator.validateWorkflow(workflow); + const result = await validator.validateWorkflow(workflow as any); expect(result.errors.some(e => e.message.includes('Connection target uses node ID \'set-id\' instead of node name \'Set\''))).toBe(true); }); @@ -861,7 +861,7 @@ describe('WorkflowValidator - Comprehensive Tests', () => { } } as any; - const result = await validator.validateWorkflow(workflow); + const result = await validator.validateWorkflow(workflow as any); expect(result.warnings.some(w => w.message.includes('Connection to disabled node: "Set"'))).toBe(true); }); @@ -891,7 +891,7 @@ describe('WorkflowValidator - Comprehensive Tests', () => { } } as any; - const result = await validator.validateWorkflow(workflow); + const result = await validator.validateWorkflow(workflow as any); expect(result.statistics.validConnections).toBe(1); }); @@ -921,7 +921,7 @@ describe('WorkflowValidator - Comprehensive Tests', () => { } } as any; - const result = await validator.validateWorkflow(workflow); + const result = await validator.validateWorkflow(workflow as any); expect(result.statistics.validConnections).toBe(1); }); @@ -953,7 +953,7 @@ describe('WorkflowValidator - Comprehensive Tests', () => { } } as any; - const result = await validator.validateWorkflow(workflow); + const result = await validator.validateWorkflow(workflow as any); expect(result.warnings.some(w => w.message.includes('Community node "CustomTool" is being used as an AI tool'))).toBe(true); }); @@ -990,7 +990,7 @@ describe('WorkflowValidator - Comprehensive Tests', () => { } } as any; - const result = await validator.validateWorkflow(workflow); + const result = await validator.validateWorkflow(workflow as any); expect(result.warnings.some(w => w.message.includes('Node is not connected to any other nodes') && w.nodeName === 'Orphaned')).toBe(true); }); @@ -1033,7 +1033,7 @@ describe('WorkflowValidator - Comprehensive Tests', () => { } } as any; - const result = await validator.validateWorkflow(workflow); + const result = await validator.validateWorkflow(workflow as any); expect(result.errors.some(e => e.message.includes('Workflow contains a cycle'))).toBe(true); }); @@ -1068,7 +1068,7 @@ describe('WorkflowValidator - Comprehensive Tests', () => { } } as any; - const result = await validator.validateWorkflow(workflow); + const result = await validator.validateWorkflow(workflow as any); expect(result.statistics.validConnections).toBe(1); expect(result.valid).toBe(true); @@ -1110,7 +1110,7 @@ describe('WorkflowValidator - Comprehensive Tests', () => { } } as any; - const result = await validator.validateWorkflow(workflow); + const result = await validator.validateWorkflow(workflow as any); expect(ExpressionValidator.validateNodeExpressions).toHaveBeenCalledWith( expect.objectContaining({ values: expect.any(Object) }), @@ -1146,7 +1146,7 @@ describe('WorkflowValidator - Comprehensive Tests', () => { connections: {} } as any; - const result = await validator.validateWorkflow(workflow); + const result = await validator.validateWorkflow(workflow as any); expect(result.errors.some(e => e.message.includes('Expression error: Invalid expression syntax'))).toBe(true); expect(result.warnings.some(w => w.message.includes('Expression warning: Deprecated variable usage'))).toBe(true); @@ -1170,7 +1170,7 @@ describe('WorkflowValidator - Comprehensive Tests', () => { connections: {} } as any; - const result = await validator.validateWorkflow(workflow); + const result = await validator.validateWorkflow(workflow as any); expect(ExpressionValidator.validateNodeExpressions).not.toHaveBeenCalled(); }); @@ -1187,7 +1187,7 @@ describe('WorkflowValidator - Comprehensive Tests', () => { const workflow = builder.build() as any; - const result = await validator.validateWorkflow(workflow); + const result = await validator.validateWorkflow(workflow as any); expect(result.warnings.some(w => w.message.includes('Consider adding error handling'))).toBe(true); }); @@ -1208,7 +1208,7 @@ describe('WorkflowValidator - Comprehensive Tests', () => { const workflow = builder.build() as any; - const result = await validator.validateWorkflow(workflow); + const result = await validator.validateWorkflow(workflow as any); expect(result.warnings.some(w => w.message.includes('Long linear chain detected'))).toBe(true); }); @@ -1230,7 +1230,7 @@ describe('WorkflowValidator - Comprehensive Tests', () => { connections: {} } as any; - const result = await validator.validateWorkflow(workflow); + const result = await validator.validateWorkflow(workflow as any); expect(result.warnings.some(w => w.message.includes('Missing credentials configuration for slackApi'))).toBe(true); }); @@ -1249,7 +1249,7 @@ describe('WorkflowValidator - Comprehensive Tests', () => { connections: {} } as any; - const result = await validator.validateWorkflow(workflow); + const result = await validator.validateWorkflow(workflow as any); expect(result.warnings.some(w => w.message.includes('AI Agent has no tools connected'))).toBe(true); }); @@ -1279,7 +1279,7 @@ describe('WorkflowValidator - Comprehensive Tests', () => { } } as any; - const result = await validator.validateWorkflow(workflow); + const result = await validator.validateWorkflow(workflow as any); expect(result.suggestions.some(s => s.includes('N8N_COMMUNITY_PACKAGES_ALLOW_TOOL_USAGE'))).toBe(true); }); @@ -1306,7 +1306,7 @@ describe('WorkflowValidator - Comprehensive Tests', () => { connections: {} } as any; - const result = await validator.validateWorkflow(workflow); + const result = await validator.validateWorkflow(workflow as any); expect(result.errors.some(e => e.message.includes('Node-level properties onError, retryOnFail, credentials are in the wrong location'))).toBe(true); expect(result.errors.some(e => e.details?.fix?.includes('Move these properties from node.parameters to the node level'))).toBe(true); @@ -1327,7 +1327,7 @@ describe('WorkflowValidator - Comprehensive Tests', () => { connections: {} } as any; - const result = await validator.validateWorkflow(workflow); + const result = await validator.validateWorkflow(workflow as any); expect(result.errors.some(e => e.message.includes('Invalid onError value: "invalidValue"'))).toBe(true); }); @@ -1347,7 +1347,7 @@ describe('WorkflowValidator - Comprehensive Tests', () => { connections: {} } as any; - const result = await validator.validateWorkflow(workflow); + const result = await validator.validateWorkflow(workflow as any); expect(result.warnings.some(w => w.message.includes('Using deprecated "continueOnFail: true"'))).toBe(true); }); @@ -1368,7 +1368,7 @@ describe('WorkflowValidator - Comprehensive Tests', () => { connections: {} } as any; - const result = await validator.validateWorkflow(workflow); + const result = await validator.validateWorkflow(workflow as any); expect(result.errors.some(e => e.message.includes('Cannot use both "continueOnFail" and "onError" properties'))).toBe(true); }); @@ -1390,7 +1390,7 @@ describe('WorkflowValidator - Comprehensive Tests', () => { connections: {} } as any; - const result = await validator.validateWorkflow(workflow); + const result = await validator.validateWorkflow(workflow as any); expect(result.errors.some(e => e.message.includes('maxTries must be a positive number'))).toBe(true); expect(result.errors.some(e => e.message.includes('waitBetweenTries must be a non-negative number'))).toBe(true); @@ -1413,7 +1413,7 @@ describe('WorkflowValidator - Comprehensive Tests', () => { connections: {} } as any; - const result = await validator.validateWorkflow(workflow); + const result = await validator.validateWorkflow(workflow as any); expect(result.warnings.some(w => w.message.includes('maxTries is set to 15'))).toBe(true); expect(result.warnings.some(w => w.message.includes('waitBetweenTries is set to 400000ms'))).toBe(true); @@ -1434,7 +1434,7 @@ describe('WorkflowValidator - Comprehensive Tests', () => { connections: {} } as any; - const result = await validator.validateWorkflow(workflow); + const result = await validator.validateWorkflow(workflow as any); expect(result.warnings.some(w => w.message.includes('retryOnFail is enabled but maxTries is not specified'))).toBe(true); }); @@ -1459,7 +1459,7 @@ describe('WorkflowValidator - Comprehensive Tests', () => { connections: {} } as any; - const result = await validator.validateWorkflow(workflow); + const result = await validator.validateWorkflow(workflow as any); expect(result.errors.some(e => e.message.includes('alwaysOutputData must be a boolean'))).toBe(true); @@ -1484,7 +1484,7 @@ describe('WorkflowValidator - Comprehensive Tests', () => { connections: {} } as any; - const result = await validator.validateWorkflow(workflow); + const result = await validator.validateWorkflow(workflow as any); expect(result.warnings.some(w => w.message.includes('executeOnce is enabled'))).toBe(true); }); @@ -1512,7 +1512,7 @@ describe('WorkflowValidator - Comprehensive Tests', () => { connections: {} } as any; - const result = await validator.validateWorkflow(workflow); + const result = await validator.validateWorkflow(workflow as any); expect(result.warnings.some(w => w.message.includes(nodeInfo.message) && w.message.includes('without error handling'))).toBe(true); } @@ -1534,7 +1534,7 @@ describe('WorkflowValidator - Comprehensive Tests', () => { connections: {} } as any; - const result = await validator.validateWorkflow(workflow); + const result = await validator.validateWorkflow(workflow as any); expect(result.warnings.some(w => w.message.includes('Both continueOnFail and retryOnFail are enabled'))).toBe(true); }); @@ -1554,7 +1554,7 @@ describe('WorkflowValidator - Comprehensive Tests', () => { connections: {} } as any; - const result = await validator.validateWorkflow(workflow); + const result = await validator.validateWorkflow(workflow as any); expect(result.suggestions.some(s => s.includes('Consider enabling alwaysOutputData'))).toBe(true); }); @@ -1569,7 +1569,7 @@ describe('WorkflowValidator - Comprehensive Tests', () => { const workflow = builder.build() as any; - const result = await validator.validateWorkflow(workflow); + const result = await validator.validateWorkflow(workflow as any); expect(result.suggestions.some(s => s.includes('Most nodes lack error handling'))).toBe(true); }); @@ -1589,7 +1589,7 @@ describe('WorkflowValidator - Comprehensive Tests', () => { connections: {} } as any; - const result = await validator.validateWorkflow(workflow); + const result = await validator.validateWorkflow(workflow as any); expect(result.suggestions.some(s => s.includes('Replace "continueOnFail: true" with "onError:'))).toBe(true); }); @@ -1610,7 +1610,7 @@ describe('WorkflowValidator - Comprehensive Tests', () => { connections: {} } as any; - const result = await validator.validateWorkflow(workflow); + const result = await validator.validateWorkflow(workflow as any); expect(result.suggestions.some(s => s.includes('Add a trigger node'))).toBe(true); }); @@ -1636,7 +1636,7 @@ describe('WorkflowValidator - Comprehensive Tests', () => { connections: {} // Missing connections } as any; - const result = await validator.validateWorkflow(workflow); + const result = await validator.validateWorkflow(workflow as any); expect(result.suggestions.some(s => s.includes('Example connection structure'))).toBe(true); expect(result.suggestions.some(s => s.includes('Use node NAMES (not IDs) in connections'))).toBe(true); @@ -1667,7 +1667,7 @@ describe('WorkflowValidator - Comprehensive Tests', () => { } } as any; - const result = await validator.validateWorkflow(workflow); + const result = await validator.validateWorkflow(workflow as any); expect(result.suggestions.some(s => s.includes('Add error handling'))).toBe(true); }); @@ -1682,7 +1682,7 @@ describe('WorkflowValidator - Comprehensive Tests', () => { const workflow = builder.build() as any; - const result = await validator.validateWorkflow(workflow); + const result = await validator.validateWorkflow(workflow as any); expect(result.suggestions.some(s => s.includes('Consider breaking this workflow into smaller sub-workflows'))).toBe(true); }); @@ -1708,7 +1708,7 @@ describe('WorkflowValidator - Comprehensive Tests', () => { connections: {} } as any; - const result = await validator.validateWorkflow(workflow); + const result = await validator.validateWorkflow(workflow as any); expect(result.suggestions.some(s => s.includes('Consider using a Code node for complex data transformations'))).toBe(true); }); @@ -1727,7 +1727,7 @@ describe('WorkflowValidator - Comprehensive Tests', () => { connections: {} } as any; - const result = await validator.validateWorkflow(workflow); + const result = await validator.validateWorkflow(workflow as any); expect(result.suggestions.some(s => s.includes('A minimal workflow needs'))).toBe(true); }); @@ -1756,7 +1756,7 @@ describe('WorkflowValidator - Comprehensive Tests', () => { connections: {} } as any; - const result = await validator.validateWorkflow(workflow); + const result = await validator.validateWorkflow(workflow as any); expect(result.errors.some(e => e.message.includes(`Did you mean`) && e.message.includes(testCase.suggestion))).toBe(true); } @@ -1848,7 +1848,7 @@ describe('WorkflowValidator - Comprehensive Tests', () => { } } as any; - const result = await validator.validateWorkflow(workflow); + const result = await validator.validateWorkflow(workflow as any); // Should have multiple errors expect(result.valid).toBe(false); @@ -1940,7 +1940,7 @@ describe('WorkflowValidator - Comprehensive Tests', () => { } } as any; - const result = await validator.validateWorkflow(workflow); + const result = await validator.validateWorkflow(workflow as any); expect(result.valid).toBe(true); expect(result.errors).toHaveLength(0); diff --git a/tests/unit/services/workflow-validator-edge-cases.test.ts b/tests/unit/services/workflow-validator-edge-cases.test.ts index 17befa7..c1547d8 100644 --- a/tests/unit/services/workflow-validator-edge-cases.test.ts +++ b/tests/unit/services/workflow-validator-edge-cases.test.ts @@ -157,7 +157,7 @@ describe('WorkflowValidator - Edge Cases', () => { nodes: [], connections: {} }; - const result = await validator.validateWorkflow(workflow); + const result = await validator.validateWorkflow(workflow as any); expect(result.valid).toBe(true); expect(result.warnings.some(w => w.message.includes('empty'))).toBe(true); }); @@ -181,7 +181,7 @@ describe('WorkflowValidator - Edge Cases', () => { const workflow = { nodes, connections }; const start = Date.now(); - const result = await validator.validateWorkflow(workflow); + const result = await validator.validateWorkflow(workflow as any); const duration = Date.now() - start; expect(result).toBeDefined(); @@ -207,7 +207,7 @@ describe('WorkflowValidator - Edge Cases', () => { } }; - const result = await validator.validateWorkflow(workflow); + const result = await validator.validateWorkflow(workflow as any); expect(result.statistics.invalidConnections).toBe(0); }); @@ -228,7 +228,7 @@ describe('WorkflowValidator - Edge Cases', () => { } }; - const result = await validator.validateWorkflow(workflow); + const result = await validator.validateWorkflow(workflow as any); expect(result.valid).toBe(true); }); }); @@ -264,7 +264,7 @@ describe('WorkflowValidator - Edge Cases', () => { connections: {} }; - const result = await validator.validateWorkflow(workflow); + const result = await validator.validateWorkflow(workflow as any); expect(result.errors.length).toBeGreaterThan(0); }); @@ -292,7 +292,7 @@ describe('WorkflowValidator - Edge Cases', () => { } }; - const result = await validator.validateWorkflow(workflow); + const result = await validator.validateWorkflow(workflow as any); expect(result.warnings.some(w => w.message.includes('self-referencing'))).toBe(true); }); @@ -308,7 +308,7 @@ describe('WorkflowValidator - Edge Cases', () => { } }; - const result = await validator.validateWorkflow(workflow); + const result = await validator.validateWorkflow(workflow as any); expect(result.errors.some(e => e.message.includes('non-existent'))).toBe(true); }); @@ -324,7 +324,7 @@ describe('WorkflowValidator - Edge Cases', () => { } }; - const result = await validator.validateWorkflow(workflow); + const result = await validator.validateWorkflow(workflow as any); expect(result.errors.length).toBeGreaterThan(0); }); @@ -341,7 +341,7 @@ describe('WorkflowValidator - Edge Cases', () => { } as any }; - const result = await validator.validateWorkflow(workflow); + const result = await validator.validateWorkflow(workflow as any); // Should still work as type and index can have defaults expect(result.statistics.validConnections).toBeGreaterThan(0); }); @@ -359,7 +359,7 @@ describe('WorkflowValidator - Edge Cases', () => { } }; - const result = await validator.validateWorkflow(workflow); + const result = await validator.validateWorkflow(workflow as any); expect(result.errors.some(e => e.message.includes('Invalid'))).toBe(true); }); }); @@ -382,7 +382,7 @@ describe('WorkflowValidator - Edge Cases', () => { } }; - const result = await validator.validateWorkflow(workflow); + const result = await validator.validateWorkflow(workflow as any); expect(result.valid).toBe(true); }); @@ -395,7 +395,7 @@ describe('WorkflowValidator - Edge Cases', () => { connections: {} }; - const result = await validator.validateWorkflow(workflow); + const result = await validator.validateWorkflow(workflow as any); expect(result.warnings.some(w => w.message.includes('very long'))).toBe(true); }); }); @@ -479,7 +479,7 @@ describe('WorkflowValidator - Edge Cases', () => { } }; - const result = await validator.validateWorkflow(workflow); + const result = await validator.validateWorkflow(workflow as any); expect(result.statistics.validConnections).toBeGreaterThan(0); }); }); @@ -499,7 +499,7 @@ describe('WorkflowValidator - Edge Cases', () => { } }; - const result = await validator.validateWorkflow(workflow); + const result = await validator.validateWorkflow(workflow as any); expect(result.errors.length).toBeGreaterThan(0); expect(result.statistics.validConnections).toBeGreaterThan(0); }); diff --git a/tests/unit/services/workflow-validator-with-mocks.test.ts b/tests/unit/services/workflow-validator-with-mocks.test.ts index e2d2a98..74f53c2 100644 --- a/tests/unit/services/workflow-validator-with-mocks.test.ts +++ b/tests/unit/services/workflow-validator-with-mocks.test.ts @@ -77,7 +77,7 @@ describe('WorkflowValidator - Simple Unit Tests', () => { }; // Act - const result = await validator.validateWorkflow(workflow); + const result = await validator.validateWorkflow(workflow as any); // Assert expect(result.valid).toBe(true); @@ -113,7 +113,7 @@ describe('WorkflowValidator - Simple Unit Tests', () => { }; // Act - const result = await validator.validateWorkflow(workflow); + const result = await validator.validateWorkflow(workflow as any); // Assert expect(result.valid).toBe(false); @@ -154,7 +154,7 @@ describe('WorkflowValidator - Simple Unit Tests', () => { }; // Act - const result = await validator.validateWorkflow(workflow); + const result = await validator.validateWorkflow(workflow as any); // Assert expect(result.valid).toBe(false); @@ -229,7 +229,7 @@ describe('WorkflowValidator - Simple Unit Tests', () => { }; // Act - const result = await validator.validateWorkflow(workflow); + const result = await validator.validateWorkflow(workflow as any); // Assert expect(result.valid).toBe(true); @@ -297,7 +297,7 @@ describe('WorkflowValidator - Simple Unit Tests', () => { }; // Act - const result = await validator.validateWorkflow(workflow); + const result = await validator.validateWorkflow(workflow as any); // Assert expect(result.valid).toBe(false); @@ -386,7 +386,7 @@ describe('WorkflowValidator - Simple Unit Tests', () => { }; // Act - const result = await validator.validateWorkflow(workflow); + const result = await validator.validateWorkflow(workflow as any); // Assert expect(result.valid).toBe(false); @@ -438,7 +438,7 @@ describe('WorkflowValidator - Simple Unit Tests', () => { }; // Act - const result = await validator.validateWorkflow(workflow); + const result = await validator.validateWorkflow(workflow as any); // Assert expect(result.warnings.some(w => w.message.includes('Outdated typeVersion'))).toBe(true); @@ -471,7 +471,7 @@ describe('WorkflowValidator - Simple Unit Tests', () => { }; // Act - const result = await validator.validateWorkflow(workflow); + const result = await validator.validateWorkflow(workflow as any); // Assert expect(result.valid).toBe(false); diff --git a/tests/unit/validation-fixes.test.ts b/tests/unit/validation-fixes.test.ts new file mode 100644 index 0000000..f1e95fb --- /dev/null +++ b/tests/unit/validation-fixes.test.ts @@ -0,0 +1,410 @@ +/** + * Test suite for validation system fixes + * Covers issues #58, #68, #70, #73 + */ + +import { describe, test, expect, beforeAll, afterAll } from 'vitest'; +import { WorkflowValidator } from '../../src/services/workflow-validator'; +import { EnhancedConfigValidator } from '../../src/services/enhanced-config-validator'; +import { ToolValidation, Validator, ValidationError } from '../../src/utils/validation-schemas'; + +describe('Validation System Fixes', () => { + let workflowValidator: WorkflowValidator; + let mockNodeRepository: any; + + beforeAll(async () => { + // Initialize test environment + process.env.NODE_ENV = 'test'; + + // Mock repository for testing + mockNodeRepository = { + getNode: (nodeType: string) => { + if (nodeType === 'nodes-base.webhook' || nodeType === 'n8n-nodes-base.webhook') { + return { + nodeType: 'nodes-base.webhook', + displayName: 'Webhook', + properties: [ + { name: 'path', required: true, displayName: 'Path' }, + { name: 'httpMethod', required: true, displayName: 'HTTP Method' } + ] + }; + } + if (nodeType === 'nodes-base.set' || nodeType === 'n8n-nodes-base.set') { + return { + nodeType: 'nodes-base.set', + displayName: 'Set', + properties: [ + { name: 'values', required: false, displayName: 'Values' } + ] + }; + } + return null; + } + } as any; + + workflowValidator = new WorkflowValidator(mockNodeRepository, EnhancedConfigValidator); + }); + + afterAll(() => { + delete process.env.NODE_ENV; + }); + + describe('Issue #73: validate_node_minimal crashes without input validation', () => { + test('should handle empty config in validation schemas', () => { + // Test the validation schema handles empty config + const result = ToolValidation.validateNodeMinimal({ + nodeType: 'nodes-base.webhook', + config: undefined + }); + + expect(result).toBeDefined(); + expect(result.valid).toBe(false); + expect(result.errors.length).toBeGreaterThan(0); + expect(result.errors[0].field).toBe('config'); + }); + + test('should handle null config in validation schemas', () => { + const result = ToolValidation.validateNodeMinimal({ + nodeType: 'nodes-base.webhook', + config: null + }); + + expect(result).toBeDefined(); + expect(result.valid).toBe(false); + expect(result.errors.length).toBeGreaterThan(0); + expect(result.errors[0].field).toBe('config'); + }); + + test('should accept valid config object', () => { + const result = ToolValidation.validateNodeMinimal({ + nodeType: 'nodes-base.webhook', + config: { path: '/webhook', httpMethod: 'POST' } + }); + + expect(result).toBeDefined(); + expect(result.valid).toBe(true); + expect(result.errors).toHaveLength(0); + }); + }); + + describe('Issue #58: validate_node_operation crashes on nested input', () => { + test('should handle invalid nodeType gracefully', () => { + expect(() => { + EnhancedConfigValidator.validateWithMode( + undefined as any, + { resource: 'channel', operation: 'create' }, + [], + 'operation', + 'ai-friendly' + ); + }).toThrow(Error); + }); + + test('should handle null nodeType gracefully', () => { + expect(() => { + EnhancedConfigValidator.validateWithMode( + null as any, + { resource: 'channel', operation: 'create' }, + [], + 'operation', + 'ai-friendly' + ); + }).toThrow(Error); + }); + + test('should handle non-string nodeType gracefully', () => { + expect(() => { + EnhancedConfigValidator.validateWithMode( + { type: 'nodes-base.slack' } as any, + { resource: 'channel', operation: 'create' }, + [], + 'operation', + 'ai-friendly' + ); + }).toThrow(Error); + }); + + test('should handle valid nodeType properly', () => { + const result = EnhancedConfigValidator.validateWithMode( + 'nodes-base.set', + { values: {} }, + [], + 'operation', + 'ai-friendly' + ); + + expect(result).toBeDefined(); + expect(typeof result.valid).toBe('boolean'); + }); + }); + + describe('Issue #70: Profile settings not respected', () => { + test('should pass profile parameter to all validation phases', async () => { + const workflow = { + nodes: [ + { + id: '1', + name: 'Webhook', + type: 'n8n-nodes-base.webhook', + position: [100, 200], + parameters: { path: '/test', httpMethod: 'POST' }, + typeVersion: 1 + }, + { + id: '2', + name: 'Set', + type: 'n8n-nodes-base.set', + position: [300, 200], + parameters: { values: {} }, + typeVersion: 1 + } + ], + connections: { + 'Webhook': { + main: [[{ node: 'Set', type: 'main', index: 0 }]] + } + } + }; + + const result = await workflowValidator.validateWorkflow(workflow, { + validateNodes: true, + validateConnections: true, + validateExpressions: true, + profile: 'minimal' + }); + + expect(result).toBeDefined(); + expect(result.valid).toBe(true); + // In minimal profile, should have fewer warnings/errors - just check it's reasonable + expect(result.warnings.length).toBeLessThanOrEqual(5); + }); + + test('should filter out sticky notes from validation', async () => { + const workflow = { + nodes: [ + { + id: '1', + name: 'Webhook', + type: 'n8n-nodes-base.webhook', + position: [100, 200], + parameters: { path: '/test', httpMethod: 'POST' }, + typeVersion: 1 + }, + { + id: '2', + name: 'Sticky Note', + type: 'n8n-nodes-base.stickyNote', + position: [300, 100], + parameters: { content: 'This is a note' }, + typeVersion: 1 + } + ], + connections: {} + }; + + const result = await workflowValidator.validateWorkflow(workflow); + + expect(result).toBeDefined(); + expect(result.statistics.totalNodes).toBe(1); // Only webhook, sticky note excluded + expect(result.statistics.enabledNodes).toBe(1); + }); + + test('should allow legitimate loops in cycle detection', async () => { + const workflow = { + nodes: [ + { + id: '1', + name: 'Manual Trigger', + type: 'n8n-nodes-base.manualTrigger', + position: [100, 200], + parameters: {}, + typeVersion: 1 + }, + { + id: '2', + name: 'SplitInBatches', + type: 'n8n-nodes-base.splitInBatches', + position: [300, 200], + parameters: { batchSize: 1 }, + typeVersion: 1 + }, + { + id: '3', + name: 'Set', + type: 'n8n-nodes-base.set', + position: [500, 200], + parameters: { values: {} }, + typeVersion: 1 + } + ], + connections: { + 'Manual Trigger': { + main: [[{ node: 'SplitInBatches', type: 'main', index: 0 }]] + }, + 'SplitInBatches': { + main: [ + [{ node: 'Set', type: 'main', index: 0 }], // Done output + [{ node: 'Set', type: 'main', index: 0 }] // Loop output + ] + }, + 'Set': { + main: [[{ node: 'SplitInBatches', type: 'main', index: 0 }]] // Loop back + } + } + }; + + const result = await workflowValidator.validateWorkflow(workflow); + + expect(result).toBeDefined(); + // Should not report cycle error for legitimate SplitInBatches loop + const cycleErrors = result.errors.filter(e => e.message.includes('cycle')); + expect(cycleErrors).toHaveLength(0); + }); + }); + + describe('Issue #68: Better error recovery suggestions', () => { + test('should provide recovery suggestions for invalid node types', async () => { + const workflow = { + nodes: [ + { + id: '1', + name: 'Invalid Node', + type: 'invalid-node-type', + position: [100, 200], + parameters: {}, + typeVersion: 1 + } + ], + connections: {} + }; + + const result = await workflowValidator.validateWorkflow(workflow); + + expect(result).toBeDefined(); + expect(result.valid).toBe(false); + expect(result.suggestions.length).toBeGreaterThan(0); + + // Should contain recovery suggestions + const recoveryStarted = result.suggestions.some(s => s.includes('🔧 RECOVERY')); + expect(recoveryStarted).toBe(true); + }); + + test('should provide recovery suggestions for connection errors', async () => { + const workflow = { + nodes: [ + { + id: '1', + name: 'Webhook', + type: 'n8n-nodes-base.webhook', + position: [100, 200], + parameters: { path: '/test', httpMethod: 'POST' }, + typeVersion: 1 + } + ], + connections: { + 'Webhook': { + main: [[{ node: 'NonExistentNode', type: 'main', index: 0 }]] + } + } + }; + + const result = await workflowValidator.validateWorkflow(workflow); + + expect(result).toBeDefined(); + expect(result.valid).toBe(false); + expect(result.suggestions.length).toBeGreaterThan(0); + + // Should contain connection recovery suggestions + const connectionRecovery = result.suggestions.some(s => + s.includes('Connection errors detected') || s.includes('connection') + ); + expect(connectionRecovery).toBe(true); + }); + + test('should provide workflow for multiple errors', async () => { + const workflow = { + nodes: [ + { + id: '1', + name: 'Invalid Node 1', + type: 'invalid-type-1', + position: [100, 200], + parameters: {} + // Missing typeVersion + }, + { + id: '2', + name: 'Invalid Node 2', + type: 'invalid-type-2', + position: [300, 200], + parameters: {} + // Missing typeVersion + }, + { + id: '3', + name: 'Invalid Node 3', + type: 'invalid-type-3', + position: [500, 200], + parameters: {} + // Missing typeVersion + } + ], + connections: { + 'Invalid Node 1': { + main: [[{ node: 'NonExistent', type: 'main', index: 0 }]] + } + } + }; + + const result = await workflowValidator.validateWorkflow(workflow); + + expect(result).toBeDefined(); + expect(result.valid).toBe(false); + expect(result.errors.length).toBeGreaterThan(3); + + // Should provide step-by-step recovery workflow + const workflowSuggestion = result.suggestions.some(s => + s.includes('SUGGESTED WORKFLOW') && s.includes('Too many errors detected') + ); + expect(workflowSuggestion).toBe(true); + }); + }); + + describe('Enhanced Input Validation', () => { + test('should validate tool parameters with schemas', () => { + // Test validate_node_operation parameters + const validationResult = ToolValidation.validateNodeOperation({ + nodeType: 'nodes-base.webhook', + config: { path: '/test' }, + profile: 'ai-friendly' + }); + + expect(validationResult.valid).toBe(true); + expect(validationResult.errors).toHaveLength(0); + }); + + test('should reject invalid parameters', () => { + const validationResult = ToolValidation.validateNodeOperation({ + nodeType: 123, // Invalid type + config: 'not an object', // Invalid type + profile: 'invalid-profile' // Invalid enum value + }); + + expect(validationResult.valid).toBe(false); + expect(validationResult.errors.length).toBeGreaterThan(0); + }); + + test('should format validation errors properly', () => { + const validationResult = ToolValidation.validateNodeOperation({ + nodeType: null, + config: null + }); + + const errorMessage = Validator.formatErrors(validationResult, 'validate_node_operation'); + + expect(errorMessage).toContain('validate_node_operation: Validation failed:'); + expect(errorMessage).toContain('nodeType'); + expect(errorMessage).toContain('config'); + }); + }); +}); \ No newline at end of file