fix: resolve issue #90 - prevent 'propertyValues[itemName] is not iterable' error
- Add validation for invalid fixedCollection structures in Switch, If, and Filter nodes - Detect and prevent nested 'conditions.values' patterns that cause n8n UI crashes - Support both 'n8n-nodes-base.x' and 'nodes-base.x' node type formats - Provide auto-fix suggestions for invalid structures - Add comprehensive test coverage for all edge cases This prevents AI agents from creating invalid node configurations that break n8n's UI.
This commit is contained in:
czlonkowski
@@ -86,6 +86,9 @@ export class EnhancedConfigValidator extends ConfigValidator {
|
||||
// Generate next steps based on errors
|
||||
enhancedResult.nextSteps = this.generateNextSteps(enhancedResult);
|
||||
|
||||
// Recalculate validity after all enhancements (crucial for fixedCollection validation)
|
||||
enhancedResult.valid = enhancedResult.errors.length === 0;
|
||||
|
||||
return enhancedResult;
|
||||
}
|
||||
|
||||
@@ -186,6 +189,9 @@ export class EnhancedConfigValidator extends ConfigValidator {
|
||||
config: Record<string, any>,
|
||||
result: EnhancedValidationResult
|
||||
): void {
|
||||
// First, validate fixedCollection properties for known problematic nodes
|
||||
this.validateFixedCollectionStructures(nodeType, config, result);
|
||||
|
||||
// Create context for node-specific validators
|
||||
const context: NodeValidationContext = {
|
||||
config,
|
||||
@@ -195,8 +201,11 @@ export class EnhancedConfigValidator extends ConfigValidator {
|
||||
autofix: result.autofix || {}
|
||||
};
|
||||
|
||||
// Normalize node type (handle both 'n8n-nodes-base.x' and 'nodes-base.x' formats)
|
||||
const normalizedNodeType = nodeType.replace('n8n-nodes-base.', 'nodes-base.');
|
||||
|
||||
// Use node-specific validators
|
||||
switch (nodeType) {
|
||||
switch (normalizedNodeType) {
|
||||
case 'nodes-base.slack':
|
||||
NodeSpecificValidators.validateSlack(context);
|
||||
this.enhanceSlackValidation(config, result);
|
||||
@@ -235,6 +244,18 @@ export class EnhancedConfigValidator extends ConfigValidator {
|
||||
case 'nodes-base.mysql':
|
||||
NodeSpecificValidators.validateMySQL(context);
|
||||
break;
|
||||
|
||||
case 'nodes-base.switch':
|
||||
this.validateSwitchNodeStructure(config, result);
|
||||
break;
|
||||
|
||||
case 'nodes-base.if':
|
||||
this.validateIfNodeStructure(config, result);
|
||||
break;
|
||||
|
||||
case 'nodes-base.filter':
|
||||
this.validateFilterNodeStructure(config, result);
|
||||
break;
|
||||
}
|
||||
|
||||
// Update autofix if changes were made
|
||||
@@ -468,4 +489,249 @@ export class EnhancedConfigValidator extends ConfigValidator {
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Validate fixedCollection structures for known problematic nodes
|
||||
* This prevents the "propertyValues[itemName] is not iterable" error
|
||||
*/
|
||||
private static validateFixedCollectionStructures(
|
||||
nodeType: string,
|
||||
config: Record<string, any>,
|
||||
result: EnhancedValidationResult
|
||||
): void {
|
||||
// Normalize node type (handle both 'n8n-nodes-base.x' and 'nodes-base.x' formats)
|
||||
const normalizedNodeType = nodeType.replace('n8n-nodes-base.', 'nodes-base.');
|
||||
|
||||
// Define nodes and their problematic patterns
|
||||
const problematicNodes = {
|
||||
'nodes-base.switch': {
|
||||
property: 'rules',
|
||||
expectedStructure: 'rules.values array',
|
||||
invalidPatterns: ['rules.conditions', 'rules.conditions.values']
|
||||
},
|
||||
'nodes-base.if': {
|
||||
property: 'conditions',
|
||||
expectedStructure: 'conditions array/object',
|
||||
invalidPatterns: ['conditions.values']
|
||||
},
|
||||
'nodes-base.filter': {
|
||||
property: 'conditions',
|
||||
expectedStructure: 'conditions array/object',
|
||||
invalidPatterns: ['conditions.values']
|
||||
}
|
||||
};
|
||||
|
||||
const nodeConfig = problematicNodes[normalizedNodeType as keyof typeof problematicNodes];
|
||||
if (!nodeConfig) return;
|
||||
|
||||
const propertyValue = config[nodeConfig.property];
|
||||
if (!propertyValue || typeof propertyValue !== 'object') return;
|
||||
|
||||
// Check for incorrect nesting patterns
|
||||
for (const pattern of nodeConfig.invalidPatterns) {
|
||||
const parts = pattern.split('.');
|
||||
let current = config;
|
||||
let isInvalid = true;
|
||||
|
||||
for (const part of parts) {
|
||||
if (!current || typeof current !== 'object' || !current[part]) {
|
||||
isInvalid = false;
|
||||
break;
|
||||
}
|
||||
current = current[part];
|
||||
}
|
||||
|
||||
if (isInvalid) {
|
||||
result.errors.push({
|
||||
type: 'invalid_value',
|
||||
property: nodeConfig.property,
|
||||
message: `Invalid structure for ${normalizedNodeType} node: found nested "${pattern}" but expected "${nodeConfig.expectedStructure}". This causes "propertyValues[itemName] is not iterable" error in n8n.`,
|
||||
fix: this.generateFixedCollectionFix(normalizedNodeType, pattern, nodeConfig.expectedStructure)
|
||||
});
|
||||
|
||||
// Provide auto-fix suggestion
|
||||
if (!result.autofix) result.autofix = {};
|
||||
result.autofix[nodeConfig.property] = this.generateFixedCollectionAutofix(normalizedNodeType, config[nodeConfig.property]);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Generate fix message for fixedCollection errors
|
||||
*/
|
||||
private static generateFixedCollectionFix(nodeType: string, invalidPattern: string, expectedStructure: string): string {
|
||||
switch (nodeType) {
|
||||
case 'nodes-base.switch':
|
||||
return 'Use: { "rules": { "values": [{ "conditions": {...}, "outputKey": "output1" }] } }';
|
||||
case 'nodes-base.if':
|
||||
case 'nodes-base.filter':
|
||||
return 'Use: { "conditions": {...} } or { "conditions": [...] } directly, not nested under "values"';
|
||||
default:
|
||||
return `Use ${expectedStructure} instead of ${invalidPattern}`;
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Generate auto-fix for fixedCollection structures
|
||||
*/
|
||||
private static generateFixedCollectionAutofix(nodeType: string, invalidValue: any): any {
|
||||
switch (nodeType) {
|
||||
case 'nodes-base.switch':
|
||||
// If it has rules.conditions.values, convert to rules.values
|
||||
if (invalidValue.conditions?.values) {
|
||||
return {
|
||||
values: Array.isArray(invalidValue.conditions.values)
|
||||
? invalidValue.conditions.values.map((condition: any) => ({
|
||||
conditions: condition,
|
||||
outputKey: `output${Math.random().toString(36).substring(2, 7)}`
|
||||
}))
|
||||
: [{
|
||||
conditions: invalidValue.conditions.values,
|
||||
outputKey: `output${Math.random().toString(36).substring(2, 7)}`
|
||||
}]
|
||||
};
|
||||
}
|
||||
break;
|
||||
case 'nodes-base.if':
|
||||
case 'nodes-base.filter':
|
||||
// If it has conditions.values, extract the values
|
||||
if (invalidValue.values) {
|
||||
return invalidValue.values;
|
||||
}
|
||||
break;
|
||||
}
|
||||
return invalidValue;
|
||||
}
|
||||
|
||||
/**
|
||||
* Validate Switch node structure specifically
|
||||
*/
|
||||
private static validateSwitchNodeStructure(
|
||||
config: Record<string, any>,
|
||||
result: EnhancedValidationResult
|
||||
): void {
|
||||
if (!config.rules) return;
|
||||
|
||||
// Check for common AI mistakes in Switch node
|
||||
if (config.rules.conditions) {
|
||||
// Check if it's the nested invalid pattern rules.conditions.values
|
||||
if (config.rules.conditions.values) {
|
||||
result.errors.push({
|
||||
type: 'invalid_value',
|
||||
property: 'rules',
|
||||
message: 'Invalid structure for nodes-base.switch node: found nested "rules.conditions.values" but expected "rules.values array". This causes "propertyValues[itemName] is not iterable" error in n8n.',
|
||||
fix: '{ "rules": { "values": [{ "conditions": {...}, "outputKey": "output1" }] } }'
|
||||
});
|
||||
|
||||
// Auto-fix: transform the nested structure
|
||||
if (Array.isArray(config.rules.conditions.values)) {
|
||||
result.autofix = {
|
||||
...result.autofix,
|
||||
rules: {
|
||||
values: config.rules.conditions.values.map((condition: any, index: number) => ({
|
||||
conditions: condition,
|
||||
outputKey: `output${index + 1}`
|
||||
}))
|
||||
}
|
||||
};
|
||||
}
|
||||
} else {
|
||||
// Direct conditions under rules
|
||||
result.errors.push({
|
||||
type: 'invalid_value',
|
||||
property: 'rules',
|
||||
message: 'Switch node "rules" should contain "values" array, not "conditions". This structure causes n8n UI loading errors.',
|
||||
fix: 'Change { "rules": { "conditions": {...} } } to { "rules": { "values": [{ "conditions": {...}, "outputKey": "output1" }] } }'
|
||||
});
|
||||
}
|
||||
}
|
||||
|
||||
// Validate rules.values structure if present
|
||||
if (config.rules.values && Array.isArray(config.rules.values)) {
|
||||
config.rules.values.forEach((rule: any, index: number) => {
|
||||
if (!rule.conditions) {
|
||||
result.warnings.push({
|
||||
type: 'missing_common',
|
||||
property: 'rules',
|
||||
message: `Switch rule ${index + 1} is missing "conditions" property`,
|
||||
suggestion: 'Each rule in the values array should have a "conditions" property'
|
||||
});
|
||||
}
|
||||
if (!rule.outputKey && rule.renameOutput !== false) {
|
||||
result.warnings.push({
|
||||
type: 'missing_common',
|
||||
property: 'rules',
|
||||
message: `Switch rule ${index + 1} is missing "outputKey" property`,
|
||||
suggestion: 'Add "outputKey" to specify which output to use when this rule matches'
|
||||
});
|
||||
}
|
||||
});
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Validate If node structure specifically
|
||||
*/
|
||||
private static validateIfNodeStructure(
|
||||
config: Record<string, any>,
|
||||
result: EnhancedValidationResult
|
||||
): void {
|
||||
if (!config.conditions) return;
|
||||
|
||||
// Check for incorrect nesting
|
||||
if (config.conditions.values) {
|
||||
result.errors.push({
|
||||
type: 'invalid_value',
|
||||
property: 'conditions',
|
||||
message: 'Invalid structure for nodes-base.if node: found nested "conditions.values" but expected "conditions array/object". If node "conditions" should be a filter object/array directly.',
|
||||
fix: 'Use: { "conditions": {...} } or { "conditions": [...] } directly, not nested under "values"'
|
||||
});
|
||||
|
||||
// Auto-fix: unwrap the values
|
||||
if (Array.isArray(config.conditions.values)) {
|
||||
result.autofix = {
|
||||
...result.autofix,
|
||||
conditions: config.conditions.values
|
||||
};
|
||||
} else if (typeof config.conditions.values === 'object') {
|
||||
result.autofix = {
|
||||
...result.autofix,
|
||||
conditions: config.conditions.values
|
||||
};
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Validate Filter node structure specifically
|
||||
*/
|
||||
private static validateFilterNodeStructure(
|
||||
config: Record<string, any>,
|
||||
result: EnhancedValidationResult
|
||||
): void {
|
||||
if (!config.conditions) return;
|
||||
|
||||
// Check for incorrect nesting
|
||||
if (config.conditions.values) {
|
||||
result.errors.push({
|
||||
type: 'invalid_value',
|
||||
property: 'conditions',
|
||||
message: 'Invalid structure for nodes-base.filter node: found nested "conditions.values" but expected "conditions array/object". Filter node "conditions" should be a filter object/array directly.',
|
||||
fix: 'Use: { "conditions": {...} } or { "conditions": [...] } directly, not nested under "values"'
|
||||
});
|
||||
|
||||
// Auto-fix: unwrap the values
|
||||
if (Array.isArray(config.conditions.values)) {
|
||||
result.autofix = {
|
||||
...result.autofix,
|
||||
conditions: config.conditions.values
|
||||
};
|
||||
} else if (typeof config.conditions.values === 'object') {
|
||||
result.autofix = {
|
||||
...result.autofix,
|
||||
conditions: config.conditions.values
|
||||
};
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
450
tests/unit/services/fixed-collection-validation.test.ts
Normal file
450
tests/unit/services/fixed-collection-validation.test.ts
Normal file
@@ -0,0 +1,450 @@
|
||||
/**
|
||||
* Fixed Collection Validation Tests
|
||||
* Tests for the fix of issue #90: "propertyValues[itemName] is not iterable" error
|
||||
*
|
||||
* This ensures AI agents cannot create invalid fixedCollection structures that break n8n UI
|
||||
*/
|
||||
|
||||
import { describe, test, expect } from 'vitest';
|
||||
import { EnhancedConfigValidator } from '../../../src/services/enhanced-config-validator';
|
||||
|
||||
describe('FixedCollection Validation', () => {
|
||||
describe('Switch Node v2/v3 Validation', () => {
|
||||
test('should detect invalid nested conditions structure', () => {
|
||||
const invalidConfig = {
|
||||
rules: {
|
||||
conditions: {
|
||||
values: [
|
||||
{
|
||||
value1: '={{$json.status}}',
|
||||
operation: 'equals',
|
||||
value2: 'active'
|
||||
}
|
||||
]
|
||||
}
|
||||
}
|
||||
};
|
||||
|
||||
const result = EnhancedConfigValidator.validateWithMode(
|
||||
'nodes-base.switch',
|
||||
invalidConfig,
|
||||
[],
|
||||
'operation',
|
||||
'ai-friendly'
|
||||
);
|
||||
|
||||
expect(result.valid).toBe(false);
|
||||
expect(result.errors).toHaveLength(1);
|
||||
expect(result.errors[0].type).toBe('invalid_value');
|
||||
expect(result.errors[0].property).toBe('rules');
|
||||
expect(result.errors[0].message).toContain('propertyValues[itemName] is not iterable');
|
||||
expect(result.errors[0].fix).toContain('{ "rules": { "values": [{ "conditions": {...}, "outputKey": "output1" }] } }');
|
||||
});
|
||||
|
||||
test('should detect direct conditions in rules (another invalid pattern)', () => {
|
||||
const invalidConfig = {
|
||||
rules: {
|
||||
conditions: {
|
||||
value1: '={{$json.status}}',
|
||||
operation: 'equals',
|
||||
value2: 'active'
|
||||
}
|
||||
}
|
||||
};
|
||||
|
||||
const result = EnhancedConfigValidator.validateWithMode(
|
||||
'nodes-base.switch',
|
||||
invalidConfig,
|
||||
[],
|
||||
'operation',
|
||||
'ai-friendly'
|
||||
);
|
||||
|
||||
expect(result.valid).toBe(false);
|
||||
expect(result.errors).toHaveLength(1);
|
||||
expect(result.errors[0].message).toContain('Invalid structure for nodes-base.switch node');
|
||||
});
|
||||
|
||||
test('should provide auto-fix for invalid switch structure', () => {
|
||||
const invalidConfig = {
|
||||
rules: {
|
||||
conditions: {
|
||||
values: [
|
||||
{
|
||||
value1: '={{$json.status}}',
|
||||
operation: 'equals',
|
||||
value2: 'active'
|
||||
}
|
||||
]
|
||||
}
|
||||
}
|
||||
};
|
||||
|
||||
const result = EnhancedConfigValidator.validateWithMode(
|
||||
'nodes-base.switch',
|
||||
invalidConfig,
|
||||
[],
|
||||
'operation',
|
||||
'ai-friendly'
|
||||
);
|
||||
|
||||
expect(result.autofix).toBeDefined();
|
||||
expect(result.autofix!.rules).toBeDefined();
|
||||
expect(result.autofix!.rules.values).toBeInstanceOf(Array);
|
||||
expect(result.autofix!.rules.values).toHaveLength(1);
|
||||
expect(result.autofix!.rules.values[0]).toHaveProperty('conditions');
|
||||
expect(result.autofix!.rules.values[0]).toHaveProperty('outputKey');
|
||||
});
|
||||
|
||||
test('should accept valid switch structure', () => {
|
||||
const validConfig = {
|
||||
rules: {
|
||||
values: [
|
||||
{
|
||||
conditions: {
|
||||
value1: '={{$json.status}}',
|
||||
operation: 'equals',
|
||||
value2: 'active'
|
||||
},
|
||||
outputKey: 'active'
|
||||
}
|
||||
]
|
||||
}
|
||||
};
|
||||
|
||||
const result = EnhancedConfigValidator.validateWithMode(
|
||||
'nodes-base.switch',
|
||||
validConfig,
|
||||
[],
|
||||
'operation',
|
||||
'ai-friendly'
|
||||
);
|
||||
|
||||
// Should not have the specific fixedCollection error
|
||||
const hasFixedCollectionError = result.errors.some(e =>
|
||||
e.message.includes('propertyValues[itemName] is not iterable')
|
||||
);
|
||||
expect(hasFixedCollectionError).toBe(false);
|
||||
});
|
||||
|
||||
test('should warn about missing outputKey in valid structure', () => {
|
||||
const configMissingOutputKey = {
|
||||
rules: {
|
||||
values: [
|
||||
{
|
||||
conditions: {
|
||||
value1: '={{$json.status}}',
|
||||
operation: 'equals',
|
||||
value2: 'active'
|
||||
}
|
||||
// Missing outputKey
|
||||
}
|
||||
]
|
||||
}
|
||||
};
|
||||
|
||||
const result = EnhancedConfigValidator.validateWithMode(
|
||||
'nodes-base.switch',
|
||||
configMissingOutputKey,
|
||||
[],
|
||||
'operation',
|
||||
'ai-friendly'
|
||||
);
|
||||
|
||||
const hasOutputKeyWarning = result.warnings.some(w =>
|
||||
w.message.includes('missing "outputKey" property')
|
||||
);
|
||||
expect(hasOutputKeyWarning).toBe(true);
|
||||
});
|
||||
});
|
||||
|
||||
describe('If Node Validation', () => {
|
||||
test('should detect invalid nested values structure', () => {
|
||||
const invalidConfig = {
|
||||
conditions: {
|
||||
values: [
|
||||
{
|
||||
value1: '={{$json.age}}',
|
||||
operation: 'largerEqual',
|
||||
value2: 18
|
||||
}
|
||||
]
|
||||
}
|
||||
};
|
||||
|
||||
const result = EnhancedConfigValidator.validateWithMode(
|
||||
'nodes-base.if',
|
||||
invalidConfig,
|
||||
[],
|
||||
'operation',
|
||||
'ai-friendly'
|
||||
);
|
||||
|
||||
expect(result.valid).toBe(false);
|
||||
expect(result.errors).toHaveLength(1);
|
||||
expect(result.errors[0].type).toBe('invalid_value');
|
||||
expect(result.errors[0].property).toBe('conditions');
|
||||
expect(result.errors[0].message).toContain('Invalid structure for nodes-base.if node');
|
||||
expect(result.errors[0].fix).toBe('Use: { "conditions": {...} } or { "conditions": [...] } directly, not nested under "values"');
|
||||
});
|
||||
|
||||
test('should provide auto-fix for invalid if structure', () => {
|
||||
const invalidConfig = {
|
||||
conditions: {
|
||||
values: [
|
||||
{
|
||||
value1: '={{$json.age}}',
|
||||
operation: 'largerEqual',
|
||||
value2: 18
|
||||
}
|
||||
]
|
||||
}
|
||||
};
|
||||
|
||||
const result = EnhancedConfigValidator.validateWithMode(
|
||||
'nodes-base.if',
|
||||
invalidConfig,
|
||||
[],
|
||||
'operation',
|
||||
'ai-friendly'
|
||||
);
|
||||
|
||||
expect(result.autofix).toBeDefined();
|
||||
expect(result.autofix!.conditions).toEqual(invalidConfig.conditions.values);
|
||||
});
|
||||
|
||||
test('should accept valid if structure', () => {
|
||||
const validConfig = {
|
||||
conditions: {
|
||||
value1: '={{$json.age}}',
|
||||
operation: 'largerEqual',
|
||||
value2: 18
|
||||
}
|
||||
};
|
||||
|
||||
const result = EnhancedConfigValidator.validateWithMode(
|
||||
'nodes-base.if',
|
||||
validConfig,
|
||||
[],
|
||||
'operation',
|
||||
'ai-friendly'
|
||||
);
|
||||
|
||||
// Should not have the specific structure error
|
||||
const hasStructureError = result.errors.some(e =>
|
||||
e.message.includes('should be a filter object/array directly')
|
||||
);
|
||||
expect(hasStructureError).toBe(false);
|
||||
});
|
||||
});
|
||||
|
||||
describe('Filter Node Validation', () => {
|
||||
test('should detect invalid nested values structure', () => {
|
||||
const invalidConfig = {
|
||||
conditions: {
|
||||
values: [
|
||||
{
|
||||
value1: '={{$json.score}}',
|
||||
operation: 'larger',
|
||||
value2: 80
|
||||
}
|
||||
]
|
||||
}
|
||||
};
|
||||
|
||||
const result = EnhancedConfigValidator.validateWithMode(
|
||||
'nodes-base.filter',
|
||||
invalidConfig,
|
||||
[],
|
||||
'operation',
|
||||
'ai-friendly'
|
||||
);
|
||||
|
||||
expect(result.valid).toBe(false);
|
||||
expect(result.errors).toHaveLength(1);
|
||||
expect(result.errors[0].type).toBe('invalid_value');
|
||||
expect(result.errors[0].property).toBe('conditions');
|
||||
expect(result.errors[0].message).toContain('Invalid structure for nodes-base.filter node');
|
||||
});
|
||||
|
||||
test('should accept valid filter structure', () => {
|
||||
const validConfig = {
|
||||
conditions: {
|
||||
value1: '={{$json.score}}',
|
||||
operation: 'larger',
|
||||
value2: 80
|
||||
}
|
||||
};
|
||||
|
||||
const result = EnhancedConfigValidator.validateWithMode(
|
||||
'nodes-base.filter',
|
||||
validConfig,
|
||||
[],
|
||||
'operation',
|
||||
'ai-friendly'
|
||||
);
|
||||
|
||||
// Should not have the specific structure error
|
||||
const hasStructureError = result.errors.some(e =>
|
||||
e.message.includes('should be a filter object/array directly')
|
||||
);
|
||||
expect(hasStructureError).toBe(false);
|
||||
});
|
||||
});
|
||||
|
||||
describe('Edge Cases', () => {
|
||||
test('should not validate non-problematic nodes', () => {
|
||||
const config = {
|
||||
someProperty: {
|
||||
conditions: {
|
||||
values: ['should', 'not', 'trigger', 'validation']
|
||||
}
|
||||
}
|
||||
};
|
||||
|
||||
const result = EnhancedConfigValidator.validateWithMode(
|
||||
'nodes-base.httpRequest',
|
||||
config,
|
||||
[],
|
||||
'operation',
|
||||
'ai-friendly'
|
||||
);
|
||||
|
||||
// Should not have fixedCollection errors for non-problematic nodes
|
||||
const hasFixedCollectionError = result.errors.some(e =>
|
||||
e.message.includes('propertyValues[itemName] is not iterable')
|
||||
);
|
||||
expect(hasFixedCollectionError).toBe(false);
|
||||
});
|
||||
|
||||
test('should handle empty config gracefully', () => {
|
||||
const result = EnhancedConfigValidator.validateWithMode(
|
||||
'nodes-base.switch',
|
||||
{},
|
||||
[],
|
||||
'operation',
|
||||
'ai-friendly'
|
||||
);
|
||||
|
||||
// Should not crash or produce false positives
|
||||
expect(result).toBeDefined();
|
||||
expect(result.errors).toBeInstanceOf(Array);
|
||||
});
|
||||
|
||||
test('should handle non-object property values', () => {
|
||||
const config = {
|
||||
rules: 'not an object'
|
||||
};
|
||||
|
||||
const result = EnhancedConfigValidator.validateWithMode(
|
||||
'nodes-base.switch',
|
||||
config,
|
||||
[],
|
||||
'operation',
|
||||
'ai-friendly'
|
||||
);
|
||||
|
||||
// Should not crash on non-object values
|
||||
expect(result).toBeDefined();
|
||||
expect(result.errors).toBeInstanceOf(Array);
|
||||
});
|
||||
});
|
||||
|
||||
describe('Real-world AI Agent Patterns', () => {
|
||||
test('should catch common ChatGPT/Claude switch patterns', () => {
|
||||
// This is a pattern commonly generated by AI agents
|
||||
const aiGeneratedConfig = {
|
||||
rules: {
|
||||
conditions: {
|
||||
values: [
|
||||
{
|
||||
"value1": "={{$json.status}}",
|
||||
"operation": "equals",
|
||||
"value2": "active"
|
||||
},
|
||||
{
|
||||
"value1": "={{$json.priority}}",
|
||||
"operation": "equals",
|
||||
"value2": "high"
|
||||
}
|
||||
]
|
||||
}
|
||||
}
|
||||
};
|
||||
|
||||
const result = EnhancedConfigValidator.validateWithMode(
|
||||
'nodes-base.switch',
|
||||
aiGeneratedConfig,
|
||||
[],
|
||||
'operation',
|
||||
'ai-friendly'
|
||||
);
|
||||
|
||||
expect(result.valid).toBe(false);
|
||||
expect(result.errors).toHaveLength(1);
|
||||
expect(result.errors[0].message).toContain('propertyValues[itemName] is not iterable');
|
||||
|
||||
// Check auto-fix generates correct structure
|
||||
expect(result.autofix!.rules.values).toHaveLength(2);
|
||||
result.autofix!.rules.values.forEach((rule: any) => {
|
||||
expect(rule).toHaveProperty('conditions');
|
||||
expect(rule).toHaveProperty('outputKey');
|
||||
});
|
||||
});
|
||||
|
||||
test('should catch common AI if/filter patterns', () => {
|
||||
const aiGeneratedIfConfig = {
|
||||
conditions: {
|
||||
values: {
|
||||
"value1": "={{$json.age}}",
|
||||
"operation": "largerEqual",
|
||||
"value2": 21
|
||||
}
|
||||
}
|
||||
};
|
||||
|
||||
const result = EnhancedConfigValidator.validateWithMode(
|
||||
'nodes-base.if',
|
||||
aiGeneratedIfConfig,
|
||||
[],
|
||||
'operation',
|
||||
'ai-friendly'
|
||||
);
|
||||
|
||||
expect(result.valid).toBe(false);
|
||||
expect(result.errors[0].message).toContain('Invalid structure for nodes-base.if node');
|
||||
});
|
||||
});
|
||||
|
||||
describe('Version Compatibility', () => {
|
||||
test('should work across different validation profiles', () => {
|
||||
const invalidConfig = {
|
||||
rules: {
|
||||
conditions: {
|
||||
values: [{ value1: 'test', operation: 'equals', value2: 'test' }]
|
||||
}
|
||||
}
|
||||
};
|
||||
|
||||
const profiles: Array<'strict' | 'runtime' | 'ai-friendly' | 'minimal'> =
|
||||
['strict', 'runtime', 'ai-friendly', 'minimal'];
|
||||
|
||||
profiles.forEach(profile => {
|
||||
const result = EnhancedConfigValidator.validateWithMode(
|
||||
'nodes-base.switch',
|
||||
invalidConfig,
|
||||
[],
|
||||
'operation',
|
||||
profile
|
||||
);
|
||||
|
||||
// All profiles should catch this critical error
|
||||
const hasCriticalError = result.errors.some(e =>
|
||||
e.message.includes('propertyValues[itemName] is not iterable')
|
||||
);
|
||||
|
||||
expect(hasCriticalError, `Profile ${profile} should catch critical fixedCollection error`).toBe(true);
|
||||
});
|
||||
});
|
||||
});
|
||||
});
|
||||
413
tests/unit/services/workflow-fixed-collection-validation.test.ts
Normal file
413
tests/unit/services/workflow-fixed-collection-validation.test.ts
Normal file
@@ -0,0 +1,413 @@
|
||||
/**
|
||||
* Workflow Fixed Collection Validation Tests
|
||||
* Tests that workflow validation catches fixedCollection structure errors at the workflow level
|
||||
*/
|
||||
|
||||
import { describe, test, expect, beforeEach, vi } from 'vitest';
|
||||
import { WorkflowValidator } from '../../../src/services/workflow-validator';
|
||||
import { EnhancedConfigValidator } from '../../../src/services/enhanced-config-validator';
|
||||
import { NodeRepository } from '../../../src/database/node-repository';
|
||||
|
||||
describe('Workflow FixedCollection Validation', () => {
|
||||
let validator: WorkflowValidator;
|
||||
let mockNodeRepository: any;
|
||||
|
||||
beforeEach(() => {
|
||||
// Create mock repository that returns basic node info for common nodes
|
||||
mockNodeRepository = {
|
||||
getNode: vi.fn().mockImplementation((type: string) => {
|
||||
const normalizedType = type.replace('n8n-nodes-base.', '').replace('nodes-base.', '');
|
||||
switch (normalizedType) {
|
||||
case 'webhook':
|
||||
return {
|
||||
nodeType: 'nodes-base.webhook',
|
||||
displayName: 'Webhook',
|
||||
properties: [
|
||||
{ name: 'path', type: 'string', required: true },
|
||||
{ name: 'httpMethod', type: 'options' }
|
||||
]
|
||||
};
|
||||
case 'switch':
|
||||
return {
|
||||
nodeType: 'nodes-base.switch',
|
||||
displayName: 'Switch',
|
||||
properties: [
|
||||
{ name: 'rules', type: 'fixedCollection', required: true }
|
||||
]
|
||||
};
|
||||
case 'if':
|
||||
return {
|
||||
nodeType: 'nodes-base.if',
|
||||
displayName: 'If',
|
||||
properties: [
|
||||
{ name: 'conditions', type: 'filter', required: true }
|
||||
]
|
||||
};
|
||||
case 'filter':
|
||||
return {
|
||||
nodeType: 'nodes-base.filter',
|
||||
displayName: 'Filter',
|
||||
properties: [
|
||||
{ name: 'conditions', type: 'filter', required: true }
|
||||
]
|
||||
};
|
||||
default:
|
||||
return null;
|
||||
}
|
||||
})
|
||||
};
|
||||
|
||||
validator = new WorkflowValidator(mockNodeRepository, EnhancedConfigValidator);
|
||||
});
|
||||
|
||||
test('should catch invalid Switch node structure in workflow validation', async () => {
|
||||
const workflow = {
|
||||
name: 'Test Workflow with Invalid Switch',
|
||||
nodes: [
|
||||
{
|
||||
id: 'webhook',
|
||||
name: 'Webhook',
|
||||
type: 'n8n-nodes-base.webhook',
|
||||
position: [0, 0] as [number, number],
|
||||
parameters: {
|
||||
path: 'test-webhook'
|
||||
}
|
||||
},
|
||||
{
|
||||
id: 'switch',
|
||||
name: 'Switch',
|
||||
type: 'n8n-nodes-base.switch',
|
||||
position: [200, 0] as [number, number],
|
||||
parameters: {
|
||||
// This is the problematic structure that causes "propertyValues[itemName] is not iterable"
|
||||
rules: {
|
||||
conditions: {
|
||||
values: [
|
||||
{
|
||||
value1: '={{$json.status}}',
|
||||
operation: 'equals',
|
||||
value2: 'active'
|
||||
}
|
||||
]
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
],
|
||||
connections: {
|
||||
Webhook: {
|
||||
main: [[{ node: 'Switch', type: 'main', index: 0 }]]
|
||||
}
|
||||
}
|
||||
};
|
||||
|
||||
const result = await validator.validateWorkflow(workflow, {
|
||||
validateNodes: true,
|
||||
profile: 'ai-friendly'
|
||||
});
|
||||
|
||||
expect(result.valid).toBe(false);
|
||||
expect(result.errors).toHaveLength(1);
|
||||
|
||||
const switchError = result.errors.find(e => e.nodeId === 'switch');
|
||||
expect(switchError).toBeDefined();
|
||||
expect(switchError!.message).toContain('propertyValues[itemName] is not iterable');
|
||||
expect(switchError!.message).toContain('Invalid structure for nodes-base.switch node');
|
||||
});
|
||||
|
||||
test('should catch invalid If node structure in workflow validation', async () => {
|
||||
const workflow = {
|
||||
name: 'Test Workflow with Invalid If',
|
||||
nodes: [
|
||||
{
|
||||
id: 'webhook',
|
||||
name: 'Webhook',
|
||||
type: 'n8n-nodes-base.webhook',
|
||||
position: [0, 0] as [number, number],
|
||||
parameters: {
|
||||
path: 'test-webhook'
|
||||
}
|
||||
},
|
||||
{
|
||||
id: 'if',
|
||||
name: 'If',
|
||||
type: 'n8n-nodes-base.if',
|
||||
position: [200, 0] as [number, number],
|
||||
parameters: {
|
||||
// This is the problematic structure
|
||||
conditions: {
|
||||
values: [
|
||||
{
|
||||
value1: '={{$json.age}}',
|
||||
operation: 'largerEqual',
|
||||
value2: 18
|
||||
}
|
||||
]
|
||||
}
|
||||
}
|
||||
}
|
||||
],
|
||||
connections: {
|
||||
Webhook: {
|
||||
main: [[{ node: 'If', type: 'main', index: 0 }]]
|
||||
}
|
||||
}
|
||||
};
|
||||
|
||||
const result = await validator.validateWorkflow(workflow, {
|
||||
validateNodes: true,
|
||||
profile: 'ai-friendly'
|
||||
});
|
||||
|
||||
expect(result.valid).toBe(false);
|
||||
expect(result.errors).toHaveLength(1);
|
||||
|
||||
const ifError = result.errors.find(e => e.nodeId === 'if');
|
||||
expect(ifError).toBeDefined();
|
||||
expect(ifError!.message).toContain('Invalid structure for nodes-base.if node');
|
||||
});
|
||||
|
||||
test('should accept valid Switch node structure in workflow validation', async () => {
|
||||
const workflow = {
|
||||
name: 'Test Workflow with Valid Switch',
|
||||
nodes: [
|
||||
{
|
||||
id: 'webhook',
|
||||
name: 'Webhook',
|
||||
type: 'n8n-nodes-base.webhook',
|
||||
position: [0, 0] as [number, number],
|
||||
parameters: {
|
||||
path: 'test-webhook'
|
||||
}
|
||||
},
|
||||
{
|
||||
id: 'switch',
|
||||
name: 'Switch',
|
||||
type: 'n8n-nodes-base.switch',
|
||||
position: [200, 0] as [number, number],
|
||||
parameters: {
|
||||
// This is the correct structure
|
||||
rules: {
|
||||
values: [
|
||||
{
|
||||
conditions: {
|
||||
value1: '={{$json.status}}',
|
||||
operation: 'equals',
|
||||
value2: 'active'
|
||||
},
|
||||
outputKey: 'active'
|
||||
}
|
||||
]
|
||||
}
|
||||
}
|
||||
}
|
||||
],
|
||||
connections: {
|
||||
Webhook: {
|
||||
main: [[{ node: 'Switch', type: 'main', index: 0 }]]
|
||||
}
|
||||
}
|
||||
};
|
||||
|
||||
const result = await validator.validateWorkflow(workflow, {
|
||||
validateNodes: true,
|
||||
profile: 'ai-friendly'
|
||||
});
|
||||
|
||||
// Should not have fixedCollection structure errors
|
||||
const hasFixedCollectionError = result.errors.some(e =>
|
||||
e.message.includes('propertyValues[itemName] is not iterable')
|
||||
);
|
||||
expect(hasFixedCollectionError).toBe(false);
|
||||
});
|
||||
|
||||
test('should catch multiple fixedCollection errors in a single workflow', async () => {
|
||||
const workflow = {
|
||||
name: 'Test Workflow with Multiple Invalid Structures',
|
||||
nodes: [
|
||||
{
|
||||
id: 'webhook',
|
||||
name: 'Webhook',
|
||||
type: 'n8n-nodes-base.webhook',
|
||||
position: [0, 0] as [number, number],
|
||||
parameters: {
|
||||
path: 'test-webhook'
|
||||
}
|
||||
},
|
||||
{
|
||||
id: 'switch',
|
||||
name: 'Switch',
|
||||
type: 'n8n-nodes-base.switch',
|
||||
position: [200, 0] as [number, number],
|
||||
parameters: {
|
||||
rules: {
|
||||
conditions: {
|
||||
values: [{ value1: 'test', operation: 'equals', value2: 'test' }]
|
||||
}
|
||||
}
|
||||
}
|
||||
},
|
||||
{
|
||||
id: 'if',
|
||||
name: 'If',
|
||||
type: 'n8n-nodes-base.if',
|
||||
position: [400, 0] as [number, number],
|
||||
parameters: {
|
||||
conditions: {
|
||||
values: [{ value1: 'test', operation: 'equals', value2: 'test' }]
|
||||
}
|
||||
}
|
||||
},
|
||||
{
|
||||
id: 'filter',
|
||||
name: 'Filter',
|
||||
type: 'n8n-nodes-base.filter',
|
||||
position: [600, 0] as [number, number],
|
||||
parameters: {
|
||||
conditions: {
|
||||
values: [{ value1: 'test', operation: 'equals', value2: 'test' }]
|
||||
}
|
||||
}
|
||||
}
|
||||
],
|
||||
connections: {
|
||||
Webhook: {
|
||||
main: [[{ node: 'Switch', type: 'main', index: 0 }]]
|
||||
},
|
||||
Switch: {
|
||||
main: [
|
||||
[{ node: 'If', type: 'main', index: 0 }],
|
||||
[{ node: 'Filter', type: 'main', index: 0 }]
|
||||
]
|
||||
}
|
||||
}
|
||||
};
|
||||
|
||||
const result = await validator.validateWorkflow(workflow, {
|
||||
validateNodes: true,
|
||||
profile: 'ai-friendly'
|
||||
});
|
||||
|
||||
expect(result.valid).toBe(false);
|
||||
expect(result.errors.length).toBeGreaterThanOrEqual(3); // At least one error for each problematic node
|
||||
|
||||
// Check that each problematic node has an error
|
||||
const switchError = result.errors.find(e => e.nodeId === 'switch');
|
||||
const ifError = result.errors.find(e => e.nodeId === 'if');
|
||||
const filterError = result.errors.find(e => e.nodeId === 'filter');
|
||||
|
||||
expect(switchError).toBeDefined();
|
||||
expect(ifError).toBeDefined();
|
||||
expect(filterError).toBeDefined();
|
||||
});
|
||||
|
||||
test('should provide helpful statistics about fixedCollection errors', async () => {
|
||||
const workflow = {
|
||||
name: 'Test Workflow Statistics',
|
||||
nodes: [
|
||||
{
|
||||
id: 'webhook',
|
||||
name: 'Webhook',
|
||||
type: 'n8n-nodes-base.webhook',
|
||||
position: [0, 0] as [number, number],
|
||||
parameters: { path: 'test' }
|
||||
},
|
||||
{
|
||||
id: 'bad-switch',
|
||||
name: 'Bad Switch',
|
||||
type: 'n8n-nodes-base.switch',
|
||||
position: [200, 0] as [number, number],
|
||||
parameters: {
|
||||
rules: {
|
||||
conditions: { values: [{ value1: 'test', operation: 'equals', value2: 'test' }] }
|
||||
}
|
||||
}
|
||||
},
|
||||
{
|
||||
id: 'good-switch',
|
||||
name: 'Good Switch',
|
||||
type: 'n8n-nodes-base.switch',
|
||||
position: [400, 0] as [number, number],
|
||||
parameters: {
|
||||
rules: {
|
||||
values: [{ conditions: { value1: 'test', operation: 'equals', value2: 'test' }, outputKey: 'out' }]
|
||||
}
|
||||
}
|
||||
}
|
||||
],
|
||||
connections: {
|
||||
Webhook: {
|
||||
main: [
|
||||
[{ node: 'Bad Switch', type: 'main', index: 0 }],
|
||||
[{ node: 'Good Switch', type: 'main', index: 0 }]
|
||||
]
|
||||
}
|
||||
}
|
||||
};
|
||||
|
||||
const result = await validator.validateWorkflow(workflow, {
|
||||
validateNodes: true,
|
||||
profile: 'ai-friendly'
|
||||
});
|
||||
|
||||
expect(result.statistics.totalNodes).toBe(3);
|
||||
expect(result.statistics.enabledNodes).toBe(3);
|
||||
expect(result.valid).toBe(false); // Should be invalid due to the bad switch
|
||||
|
||||
// Should have at least one error for the bad switch
|
||||
const badSwitchError = result.errors.find(e => e.nodeId === 'bad-switch');
|
||||
expect(badSwitchError).toBeDefined();
|
||||
|
||||
// Should not have errors for the good switch or webhook
|
||||
const goodSwitchError = result.errors.find(e => e.nodeId === 'good-switch');
|
||||
const webhookError = result.errors.find(e => e.nodeId === 'webhook');
|
||||
|
||||
// These might have other validation errors, but not fixedCollection errors
|
||||
if (goodSwitchError) {
|
||||
expect(goodSwitchError.message).not.toContain('propertyValues[itemName] is not iterable');
|
||||
}
|
||||
if (webhookError) {
|
||||
expect(webhookError.message).not.toContain('propertyValues[itemName] is not iterable');
|
||||
}
|
||||
});
|
||||
|
||||
test('should work with different validation profiles', async () => {
|
||||
const workflow = {
|
||||
name: 'Test Profile Compatibility',
|
||||
nodes: [
|
||||
{
|
||||
id: 'switch',
|
||||
name: 'Switch',
|
||||
type: 'n8n-nodes-base.switch',
|
||||
position: [0, 0] as [number, number],
|
||||
parameters: {
|
||||
rules: {
|
||||
conditions: {
|
||||
values: [{ value1: 'test', operation: 'equals', value2: 'test' }]
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
],
|
||||
connections: {}
|
||||
};
|
||||
|
||||
const profiles: Array<'strict' | 'runtime' | 'ai-friendly' | 'minimal'> =
|
||||
['strict', 'runtime', 'ai-friendly', 'minimal'];
|
||||
|
||||
for (const profile of profiles) {
|
||||
const result = await validator.validateWorkflow(workflow, {
|
||||
validateNodes: true,
|
||||
profile
|
||||
});
|
||||
|
||||
// All profiles should catch this critical error
|
||||
const hasCriticalError = result.errors.some(e =>
|
||||
e.message.includes('propertyValues[itemName] is not iterable')
|
||||
);
|
||||
|
||||
expect(hasCriticalError, `Profile ${profile} should catch critical fixedCollection error`).toBe(true);
|
||||
expect(result.valid, `Profile ${profile} should mark workflow as invalid`).toBe(false);
|
||||
}
|
||||
});
|
||||
});
|
||||
Reference in New Issue
Block a user