Merge pull request #289 from czlonkowski/fix/validation-warning-system-redesign

fix: resolve validation warning system false positives (96.5% noise reduction)

CHANGELOG.md

@@ -5,6 +5,60 @@ All notable changes to this project will be documented in this file.
 The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
 and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
 
+## [2.18.0] - 2025-10-08
+
+### 🎯 Validation Warning System Redesign
+
+**Fixed critical validation warning system that was generating 96.5% false positives.**
+
+This release fundamentally fixes the validation warning system that was overwhelming users and AI assistants with false warnings about properties they never configured. The system now achieves >90% signal-to-noise ratio (up from 3%).
+
+#### Problem
+
+The validation system was warning about properties with default values as if the user had configured them:
+- HTTP Request with 2 properties → 29 warnings (96% false positives)
+- Webhook with 1 property → 6 warnings (83% false positives)
+- Overall signal-to-noise ratio: 3%
+
+#### Fixed
+
+- **User Property Tracking** - System now distinguishes between user-provided properties and system defaults
+- **UI Property Filtering** - No longer validates UI-only elements (notice, callout, infoBox)
+- **Improved Messages** - Warnings now explain visibility requirements (e.g., "Requires: sendBody=true")
+- **Profile-Aware Filtering** - Each validation profile shows appropriate warnings
+  - `minimal`: Only errors + critical security warnings
+  - `runtime`: Errors + security warnings (filters property visibility noise)
+  - `ai-friendly`: Balanced helpful warnings (default)
+  - `strict`: All warnings + suggestions
+
+#### Results
+
+After fix (verified with n8n-mcp-tester):
+- HTTP Request with 2 properties → 1 warning (96.5% noise reduction)
+- Webhook with 1 property → 1 warning (83% noise reduction)
+- Overall signal-to-noise ratio: >90%
+
+#### Changed
+
+- `src/services/config-validator.ts`
+  - Added `UI_ONLY_TYPES` constant to filter UI properties
+  - Added `userProvidedKeys` parameter to `validate()` method
+  - Added `getVisibilityRequirement()` helper for better error messages
+  - Updated `checkCommonIssues()` to only warn about user-provided properties
+- `src/services/enhanced-config-validator.ts`
+  - Extract user-provided keys before applying defaults
+  - Pass `userProvidedKeys` to base validator
+  - Enhanced profile filtering to remove property visibility warnings in `runtime` and `ai-friendly` profiles
+- `src/mcp-tools-engine.ts`
+  - Extract user-provided keys in `validateNodeOperation()` before calling validator
+
+#### Impact
+
+- **AI Assistants**: Can now trust validation warnings (90%+ useful)
+- **Developers**: Get actionable guidance instead of noise
+- **Workflow Quality**: Real issues are fixed (not buried in false positives)
+- **System Trust**: Validation becomes a valuable tool
+
 ## [2.17.5] - 2025-10-07
 
 ### 🔧 Type Safety

package.json

@@ -1,6 +1,6 @@
 {
   "name": "n8n-mcp",
-  "version": "2.17.6",
+  "version": "2.18.0",
   "description": "Integration between n8n workflow automation and Model Context Protocol (MCP)",
   "main": "dist/index.js",
   "bin": {

src/mcp-tools-engine.ts

@@ -62,8 +62,12 @@ export class MCPEngine {
         hiddenProperties: []
       };
     }
-    
-    return ConfigValidator.validate(args.nodeType, args.config, node.properties || []);
+
+    // CRITICAL FIX: Extract user-provided keys before validation
+    // This prevents false warnings about default values
+    const userProvidedKeys = new Set(Object.keys(args.config || {}));
+
+    return ConfigValidator.validate(args.nodeType, args.config, node.properties || [], userProvidedKeys);
   }
 
   async validateNodeMinimal(args: any) {

src/services/config-validator.ts

@@ -31,13 +31,19 @@ export interface ValidationWarning {
 }
 
 export class ConfigValidator {
+  /**
+   * UI-only property types that should not be validated as configuration
+   */
+  private static readonly UI_ONLY_TYPES = ['notice', 'callout', 'infoBox', 'info'];
+
   /**
    * Validate a node configuration
    */
   static validate(
-    nodeType: string, 
-    config: Record<string, any>, 
-    properties: any[]
+    nodeType: string,
+    config: Record<string, any>,
+    properties: any[],
+    userProvidedKeys?: Set<string> // NEW: Track user-provided properties to avoid warning about defaults
   ): ValidationResult {
     // Input validation
     if (!config || typeof config !== 'object') {
@@ -46,7 +52,7 @@ export class ConfigValidator {
     if (!properties || !Array.isArray(properties)) {
       throw new TypeError('Properties must be a non-null array');
     }
-    
+
     const errors: ValidationError[] = [];
     const warnings: ValidationWarning[] = [];
     const suggestions: string[] = [];
@@ -69,8 +75,8 @@ export class ConfigValidator {
     this.performNodeSpecificValidation(nodeType, config, errors, warnings, suggestions, autofix);
     
     // Check for common issues
-    this.checkCommonIssues(nodeType, config, properties, warnings, suggestions);
-    
+    this.checkCommonIssues(nodeType, config, properties, warnings, suggestions, userProvidedKeys);
+
     // Security checks
     this.performSecurityChecks(nodeType, config, warnings);
     
@@ -493,30 +499,48 @@ export class ConfigValidator {
     config: Record<string, any>,
     properties: any[],
     warnings: ValidationWarning[],
-    suggestions: string[]
+    suggestions: string[],
+    userProvidedKeys?: Set<string> // NEW: Only warn about user-provided properties
   ): void {
     // Skip visibility checks for Code nodes as they have simple property structure
     if (nodeType === 'nodes-base.code') {
       // Code nodes don't have complex displayOptions, so skip visibility warnings
       return;
     }
-    
+
     // Check for properties that won't be used
     const visibleProps = properties.filter(p => this.isPropertyVisible(p, config));
     const configuredKeys = Object.keys(config);
-    
+
     for (const key of configuredKeys) {
       // Skip internal properties that are always present
       if (key === '@version' || key.startsWith('_')) {
         continue;
       }
-      
+
+      // CRITICAL FIX: Only warn about properties the user actually provided, not defaults
+      if (userProvidedKeys && !userProvidedKeys.has(key)) {
+        continue; // Skip properties that were added as defaults
+      }
+
+      // Find the property definition
+      const prop = properties.find(p => p.name === key);
+
+      // Skip UI-only properties (notice, callout, etc.) - they're not configuration
+      if (prop && this.UI_ONLY_TYPES.includes(prop.type)) {
+        continue;
+      }
+
+      // Check if property is visible with current settings
       if (!visibleProps.find(p => p.name === key)) {
+        // Get visibility requirements for better error message
+        const visibilityReq = this.getVisibilityRequirement(prop, config);
+
         warnings.push({
           type: 'inefficient',
           property: key,
-          message: `Property '${key}' is configured but won't be used due to current settings`,
-          suggestion: 'Remove this property or adjust other settings to make it visible'
+          message: `Property '${prop?.displayName || key}' won't be used - not visible with current settings`,
+          suggestion: visibilityReq || 'Remove this property or adjust other settings to make it visible'
         });
       }
     }
@@ -565,6 +589,36 @@ export class ConfigValidator {
     }
   }
   
+  /**
+   * Get visibility requirement for a property
+   * Explains what needs to be set for the property to be visible
+   */
+  private static getVisibilityRequirement(prop: any, config: Record<string, any>): string | undefined {
+    if (!prop || !prop.displayOptions?.show) {
+      return undefined;
+    }
+
+    const requirements: string[] = [];
+    for (const [field, values] of Object.entries(prop.displayOptions.show)) {
+      const expectedValues = Array.isArray(values) ? values : [values];
+      const currentValue = config[field];
+
+      // Only include if the current value doesn't match
+      if (!expectedValues.includes(currentValue)) {
+        const valueStr = expectedValues.length === 1
+          ? `"${expectedValues[0]}"`
+          : expectedValues.map(v => `"${v}"`).join(' or ');
+        requirements.push(`${field}=${valueStr}`);
+      }
+    }
+
+    if (requirements.length === 0) {
+      return undefined;
+    }
+
+    return `Requires: ${requirements.join(', ')}`;
+  }
+
   /**
    * Basic JavaScript syntax validation
    */

src/services/enhanced-config-validator.ts

@@ -78,6 +78,9 @@ export class EnhancedConfigValidator extends ConfigValidator {
     // Extract operation context from config
     const operationContext = this.extractOperationContext(config);
 
+    // Extract user-provided keys before applying defaults (CRITICAL FIX for warning system)
+    const userProvidedKeys = new Set(Object.keys(config));
+
     // Filter properties based on mode and operation, and get config with defaults
     const { properties: filteredProperties, configWithDefaults } = this.filterPropertiesByMode(
       properties,
@@ -87,7 +90,8 @@ export class EnhancedConfigValidator extends ConfigValidator {
     );
 
     // Perform base validation on filtered properties with defaults applied
-    const baseResult = super.validate(nodeType, configWithDefaults, filteredProperties);
+    // Pass userProvidedKeys to prevent warnings about default values
+    const baseResult = super.validate(nodeType, configWithDefaults, filteredProperties, userProvidedKeys);
     
     // Enhance the result
     const enhancedResult: EnhancedValidationResult = {
@@ -469,22 +473,32 @@ export class EnhancedConfigValidator extends ConfigValidator {
       case 'minimal':
         // Only keep missing required errors
         result.errors = result.errors.filter(e => e.type === 'missing_required');
-        result.warnings = [];
+        // Keep ONLY critical warnings (security and deprecated)
+        result.warnings = result.warnings.filter(w =>
+          w.type === 'security' || w.type === 'deprecated'
+        );
         result.suggestions = [];
         break;
-        
+
       case 'runtime':
         // Keep critical runtime errors only
-        result.errors = result.errors.filter(e => 
-          e.type === 'missing_required' || 
+        result.errors = result.errors.filter(e =>
+          e.type === 'missing_required' ||
           e.type === 'invalid_value' ||
           (e.type === 'invalid_type' && e.message.includes('undefined'))
         );
-        // Keep only security warnings
-        result.warnings = result.warnings.filter(w => w.type === 'security');
+        // Keep security and deprecated warnings, REMOVE property visibility warnings
+        result.warnings = result.warnings.filter(w => {
+          if (w.type === 'security' || w.type === 'deprecated') return true;
+          // FILTER OUT property visibility warnings (too noisy)
+          if (w.type === 'inefficient' && w.message && w.message.includes('not visible')) {
+            return false;
+          }
+          return false;
+        });
         result.suggestions = [];
         break;
-        
+
       case 'strict':
         // Keep everything, add more suggestions
         if (result.warnings.length === 0 && result.errors.length === 0) {
@@ -494,14 +508,28 @@ export class EnhancedConfigValidator extends ConfigValidator {
         // Require error handling for external service nodes
         this.enforceErrorHandlingForProfile(result, profile);
         break;
-        
+
       case 'ai-friendly':
       default:
         // Current behavior - balanced for AI agents
         // Filter out noise but keep helpful warnings
-        result.warnings = result.warnings.filter(w => 
-          w.type !== 'inefficient' || !w.property?.startsWith('_')
-        );
+        result.warnings = result.warnings.filter(w => {
+          // Keep security and deprecated warnings
+          if (w.type === 'security' || w.type === 'deprecated') return true;
+          // Keep missing common properties
+          if (w.type === 'missing_common') return true;
+          // Keep best practice warnings
+          if (w.type === 'best_practice') return true;
+          // FILTER OUT inefficient warnings about property visibility (now fixed at source)
+          if (w.type === 'inefficient' && w.message && w.message.includes('not visible')) {
+            return false; // These are now rare due to userProvidedKeys fix
+          }
+          // Filter out internal property warnings
+          if (w.type === 'inefficient' && w.property?.startsWith('_')) {
+            return false;
+          }
+          return true;
+        });
         // Add error handling suggestions for AI-friendly profile
         this.addErrorHandlingSuggestions(result);
         break;