fix: validation bugs — If/Switch version check, Set false positive, bare expressions (#675, #676, #677) (#678)

- #675: Wire `validateConditionNodeStructure` into `WorkflowValidator` with version-conditional checks (If v2.2+ requires options, v2.0-2.1 validates operators, v1.x skipped; Switch v3.2+ requires options) - #676: Fix `validateSet` to check `assignments.assignments` (v3+) alongside `config.values` (v1/v2), eliminating false positive warnings - #677: Add anchored heuristic pre-pass in `ExpressionValidator` detecting bare `$json`, `$node`, `$input`, `$execution`, `$workflow`, `$prevNode`, `$env`, `$now`, `$today`, `$itemIndex`, `$runIndex` references missing `={{ }}` 25 new tests across 3 test files. Conceived by Romuald Członkowski - https://www.aiadvisors.pl/en Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2026-03-31 14:43:09 +00:00 · 2026-03-30 12:29:04 +02:00
parent 6e4a9d520d
commit 20ebfbb0fc
9 changed files with 433 additions and 95 deletions
--- a/tests/unit/services/expression-validator.test.ts
+++ b/tests/unit/services/expression-validator.test.ts
@@ -105,6 +105,78 @@ describe('ExpressionValidator', () => {
    });
  });

+  describe('bare expression detection', () => {
+    it('should warn on bare $json.name', () => {
+      const params = { value: '$json.name' };
+      const result = ExpressionValidator.validateNodeExpressions(params, defaultContext);
+      expect(result.warnings.some(w => w.includes('unwrapped expression'))).toBe(true);
+    });
+
+    it('should warn on bare $node["Webhook"].json', () => {
+      const params = { value: '$node["Webhook"].json' };
+      const result = ExpressionValidator.validateNodeExpressions(params, defaultContext);
+      expect(result.warnings.some(w => w.includes('unwrapped expression'))).toBe(true);
+    });
+
+    it('should warn on bare $now', () => {
+      const params = { value: '$now' };
+      const result = ExpressionValidator.validateNodeExpressions(params, defaultContext);
+      expect(result.warnings.some(w => w.includes('unwrapped expression'))).toBe(true);
+    });
+
+    it('should warn on bare $execution.id', () => {
+      const params = { value: '$execution.id' };
+      const result = ExpressionValidator.validateNodeExpressions(params, defaultContext);
+      expect(result.warnings.some(w => w.includes('unwrapped expression'))).toBe(true);
+    });
+
+    it('should warn on bare $env.API_KEY', () => {
+      const params = { value: '$env.API_KEY' };
+      const result = ExpressionValidator.validateNodeExpressions(params, defaultContext);
+      expect(result.warnings.some(w => w.includes('unwrapped expression'))).toBe(true);
+    });
+
+    it('should warn on bare $input.item.json.field', () => {
+      const params = { value: '$input.item.json.field' };
+      const result = ExpressionValidator.validateNodeExpressions(params, defaultContext);
+      expect(result.warnings.some(w => w.includes('unwrapped expression'))).toBe(true);
+    });
+
+    it('should NOT warn on properly wrapped ={{ $json.name }}', () => {
+      const params = { value: '={{ $json.name }}' };
+      const result = ExpressionValidator.validateNodeExpressions(params, defaultContext);
+      expect(result.warnings.some(w => w.includes('unwrapped expression'))).toBe(false);
+    });
+
+    it('should NOT warn on properly wrapped {{ $json.name }}', () => {
+      const params = { value: '{{ $json.name }}' };
+      const result = ExpressionValidator.validateNodeExpressions(params, defaultContext);
+      expect(result.warnings.some(w => w.includes('unwrapped expression'))).toBe(false);
+    });
+
+    it('should NOT warn when $json appears mid-string', () => {
+      const params = { value: 'The $json data is ready' };
+      const result = ExpressionValidator.validateNodeExpressions(params, defaultContext);
+      expect(result.warnings.some(w => w.includes('unwrapped expression'))).toBe(false);
+    });
+
+    it('should NOT warn on plain text', () => {
+      const params = { value: 'Hello World' };
+      const result = ExpressionValidator.validateNodeExpressions(params, defaultContext);
+      expect(result.warnings.some(w => w.includes('unwrapped expression'))).toBe(false);
+    });
+
+    it('should detect bare expression in nested structure', () => {
+      const params = {
+        assignments: {
+          assignments: [{ value: '$json.name' }]
+        }
+      };
+      const result = ExpressionValidator.validateNodeExpressions(params, defaultContext);
+      expect(result.warnings.some(w => w.includes('unwrapped expression'))).toBe(true);
+    });
+  });
+
  describe('edge cases', () => {
    it('should handle empty expressions', () => {
      const result = ExpressionValidator.validateExpression('{{ }}', defaultContext);
--- a/tests/unit/services/node-specific-validators.test.ts
+++ b/tests/unit/services/node-specific-validators.test.ts
@@ -2384,6 +2384,73 @@ return [{"json": {"result": result}}]
    });
  });

+  describe('validateSet', () => {
+    it('should not warn when Set v3 has populated assignments', () => {
+      context.config = {
+        mode: 'manual',
+        assignments: {
+          assignments: [
+            { id: '1', name: 'status', value: 'active', type: 'string' }
+          ]
+        }
+      };
+
+      NodeSpecificValidators.validateSet(context);
+
+      const fieldWarnings = context.warnings.filter(w => w.message.includes('no fields configured'));
+      expect(fieldWarnings).toHaveLength(0);
+    });
+
+    it('should not warn when Set v2 has populated values', () => {
+      context.config = {
+        mode: 'manual',
+        values: {
+          string: [{ name: 'field', value: 'val' }]
+        }
+      };
+
+      NodeSpecificValidators.validateSet(context);
+
+      const fieldWarnings = context.warnings.filter(w => w.message.includes('no fields configured'));
+      expect(fieldWarnings).toHaveLength(0);
+    });
+
+    it('should warn when Set v3 has empty assignments array', () => {
+      context.config = {
+        mode: 'manual',
+        assignments: { assignments: [] }
+      };
+
+      NodeSpecificValidators.validateSet(context);
+
+      const fieldWarnings = context.warnings.filter(w => w.message.includes('no fields configured'));
+      expect(fieldWarnings).toHaveLength(1);
+    });
+
+    it('should warn when Set manual mode has no values or assignments', () => {
+      context.config = {
+        mode: 'manual'
+      };
+
+      NodeSpecificValidators.validateSet(context);
+
+      const fieldWarnings = context.warnings.filter(w => w.message.includes('no fields configured'));
+      expect(fieldWarnings).toHaveLength(1);
+    });
+
+    it('should not warn when Set manual mode has jsonOutput', () => {
+      context.config = {
+        mode: 'manual',
+        jsonOutput: '{"key":"value"}'
+      };
+
+      NodeSpecificValidators.validateSet(context);
+
+      const fieldWarnings = context.warnings.filter(w => w.message.includes('no fields configured'));
+      expect(fieldWarnings).toHaveLength(0);
+    });
+  });
+
  describe('validateAIAgent', () => {
    let context: NodeValidationContext;

--- a/tests/unit/services/workflow-validator.test.ts
+++ b/tests/unit/services/workflow-validator.test.ts
@@ -4,6 +4,7 @@ import { NodeRepository } from '@/database/node-repository';
 import { EnhancedConfigValidator } from '@/services/enhanced-config-validator';
 import { ExpressionValidator } from '@/services/expression-validator';
 import { createWorkflow } from '@tests/utils/builders/workflow.builder';
+import { validateConditionNodeStructure } from '@/services/n8n-validation';

 // Mock dependencies
 vi.mock('@/database/node-repository');
@@ -743,4 +744,156 @@ describe('WorkflowValidator', () => {
      expect(result.statistics.validConnections).toBe(3);
    });
  });
+
+  // ─── If/Switch conditions validation ──────────────────────────────
+
+  describe('If/Switch conditions validation (validateConditionNodeStructure)', () => {
+    it('If v2.3 missing conditions.options → error', () => {
+      const node = {
+        id: '1', name: 'IF', type: 'n8n-nodes-base.if', typeVersion: 2.3,
+        position: [0, 0] as [number, number],
+        parameters: {
+          conditions: {
+            conditions: [{ leftValue: '={{ $json.x }}', rightValue: 'a', operator: { type: 'string', operation: 'equals' } }],
+            combinator: 'and'
+          }
+        }
+      };
+      const errors = validateConditionNodeStructure(node);
+      expect(errors.length).toBeGreaterThan(0);
+      expect(errors.some(e => e.includes('options'))).toBe(true);
+    });
+
+    it('If v2.3 with complete options → no error', () => {
+      const node = {
+        id: '1', name: 'IF', type: 'n8n-nodes-base.if', typeVersion: 2.3,
+        position: [0, 0] as [number, number],
+        parameters: {
+          conditions: {
+            options: { version: 2, leftValue: '', caseSensitive: true, typeValidation: 'strict' },
+            conditions: [{ leftValue: '={{ $json.x }}', rightValue: 'a', operator: { type: 'string', operation: 'equals' } }],
+            combinator: 'and'
+          }
+        }
+      };
+      const errors = validateConditionNodeStructure(node);
+      expect(errors).toHaveLength(0);
+    });
+
+    it('If v2.0 without options → no error', () => {
+      const node = {
+        id: '1', name: 'IF', type: 'n8n-nodes-base.if', typeVersion: 2.0,
+        position: [0, 0] as [number, number],
+        parameters: {
+          conditions: {
+            conditions: [{ leftValue: '={{ $json.x }}', rightValue: 'a', operator: { type: 'string', operation: 'equals' } }],
+            combinator: 'and'
+          }
+        }
+      };
+      const errors = validateConditionNodeStructure(node);
+      expect(errors).toHaveLength(0);
+    });
+
+    it('If v2.0 with bad operator (missing type) → operator error', () => {
+      const node = {
+        id: '1', name: 'IF', type: 'n8n-nodes-base.if', typeVersion: 2.0,
+        position: [0, 0] as [number, number],
+        parameters: {
+          conditions: {
+            conditions: [{ leftValue: '={{ $json.x }}', rightValue: 'a', operator: { operation: 'equals' } }],
+            combinator: 'and'
+          }
+        }
+      };
+      const errors = validateConditionNodeStructure(node);
+      expect(errors.length).toBeGreaterThan(0);
+      expect(errors.some(e => e.includes('type'))).toBe(true);
+    });
+
+    it('If v1 with old format → no errors', () => {
+      const node = {
+        id: '1', name: 'IF', type: 'n8n-nodes-base.if', typeVersion: 1,
+        position: [0, 0] as [number, number],
+        parameters: {
+          conditions: { string: [{ value1: '={{ $json.x }}', value2: 'a', operation: 'equals' }] }
+        }
+      };
+      const errors = validateConditionNodeStructure(node);
+      expect(errors).toHaveLength(0);
+    });
+
+    it('Switch v3.2 missing rule options → error', () => {
+      const node = {
+        id: '1', name: 'Switch', type: 'n8n-nodes-base.switch', typeVersion: 3.2,
+        position: [0, 0] as [number, number],
+        parameters: {
+          rules: {
+            rules: [{
+              conditions: {
+                conditions: [{ leftValue: '={{ $json.x }}', rightValue: 'a', operator: { type: 'string', operation: 'equals' } }],
+                combinator: 'and'
+              },
+              outputKey: 'Branch 1'
+            }]
+          }
+        }
+      };
+      const errors = validateConditionNodeStructure(node);
+      expect(errors.length).toBeGreaterThan(0);
+      expect(errors.some(e => e.includes('rules.rules[0].conditions.options'))).toBe(true);
+    });
+
+    it('Switch v3.2 with complete options → no error', () => {
+      const node = {
+        id: '1', name: 'Switch', type: 'n8n-nodes-base.switch', typeVersion: 3.2,
+        position: [0, 0] as [number, number],
+        parameters: {
+          rules: {
+            rules: [{
+              conditions: {
+                options: { version: 2, leftValue: '', caseSensitive: true, typeValidation: 'strict' },
+                conditions: [{ leftValue: '={{ $json.x }}', rightValue: 'a', operator: { type: 'string', operation: 'equals' } }],
+                combinator: 'and'
+              },
+              outputKey: 'Branch 1'
+            }]
+          }
+        }
+      };
+      const errors = validateConditionNodeStructure(node);
+      expect(errors).toHaveLength(0);
+    });
+
+    it('If v2.2 with empty parameters (missing conditions) → no error (graceful)', () => {
+      const node = {
+        id: '1', name: 'IF', type: 'n8n-nodes-base.if', typeVersion: 2.2,
+        position: [0, 0] as [number, number],
+        parameters: {}
+      };
+      const errors = validateConditionNodeStructure(node);
+      // Empty parameters are allowed — draft/incomplete nodes are valid at this level
+      expect(errors).toHaveLength(0);
+    });
+
+    it('Switch v3.0 without options → no error', () => {
+      const node = {
+        id: '1', name: 'Switch', type: 'n8n-nodes-base.switch', typeVersion: 3.0,
+        position: [0, 0] as [number, number],
+        parameters: {
+          rules: {
+            rules: [{
+              conditions: {
+                conditions: [{ leftValue: '={{ $json.x }}', rightValue: 'a', operator: { type: 'string', operation: 'equals' } }],
+                combinator: 'and'
+              },
+              outputKey: 'Branch 1'
+            }]
+          }
+        }
+      };
+      const errors = validateConditionNodeStructure(node);
+      expect(errors).toHaveLength(0);
+    });
+  });
 });