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

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

.github/workflows/release.yml

@@ -79,6 +79,38 @@ jobs:
             echo "ℹ️ No version change detected"
           fi
 
+      - name: Validate version against npm registry
+        if: steps.check.outputs.changed == 'true'
+        run: |
+          CURRENT_VERSION="${{ steps.check.outputs.version }}"
+
+          # Get latest version from npm (handle package not found)
+          NPM_VERSION=$(npm view n8n-mcp version 2>/dev/null || echo "0.0.0")
+
+          echo "Current version: $CURRENT_VERSION"
+          echo "NPM registry version: $NPM_VERSION"
+
+          # Check if version already exists in npm
+          if [ "$CURRENT_VERSION" = "$NPM_VERSION" ]; then
+            echo "❌ Error: Version $CURRENT_VERSION already published to npm"
+            echo "Please bump the version in package.json before releasing"
+            exit 1
+          fi
+
+          # Simple semver comparison (assumes format: major.minor.patch)
+          # Compare if current version is greater than npm version
+          if [ "$NPM_VERSION" != "0.0.0" ]; then
+            # Sort versions and check if current is not the highest
+            HIGHEST=$(printf '%s\n%s' "$NPM_VERSION" "$CURRENT_VERSION" | sort -V | tail -n1)
+            if [ "$HIGHEST" != "$CURRENT_VERSION" ]; then
+              echo "❌ Error: Version $CURRENT_VERSION is not greater than npm version $NPM_VERSION"
+              echo "Please use a higher version number"
+              exit 1
+            fi
+          fi
+
+          echo "✅ Version $CURRENT_VERSION is valid (higher than npm version $NPM_VERSION)"
+
   extract-changelog:
     name: Extract Changelog
     runs-on: ubuntu-latest
@@ -206,8 +238,8 @@ jobs:
           echo "id=$RELEASE_ID" >> $GITHUB_OUTPUT
           echo "upload_url=https://uploads.github.com/repos/${{ github.repository }}/releases/$RELEASE_ID/assets{?name,label}" >> $GITHUB_OUTPUT
 
-  build-and-test:
-    name: Build and Test
+  build-and-verify:
+    name: Build and Verify
     runs-on: ubuntu-latest
     needs: detect-version-change
     if: needs.detect-version-change.outputs.version-changed == 'true'
@@ -226,22 +258,28 @@ jobs:
       
       - name: Build project
         run: npm run build
-      
-      - name: Rebuild database
-        run: npm run rebuild
-      
-      - name: Run tests
-        run: npm test
-        env:
-          CI: true
-      
+
+      # Database is already built and committed during development
+      # Rebuilding here causes segfault due to memory pressure (exit code 139)
+      - name: Verify database exists
+        run: |
+          if [ ! -f "data/nodes.db" ]; then
+            echo "❌ Error: data/nodes.db not found"
+            echo "Please run 'npm run rebuild' locally and commit the database"
+            exit 1
+          fi
+          echo "✅ Database exists ($(du -h data/nodes.db | cut -f1))"
+
+      # Skip tests - they already passed in PR before merge
+      # Running them again on the same commit adds no safety, only time (~6-7 min)
+
       - name: Run type checking
         run: npm run typecheck
 
   publish-npm:
     name: Publish to NPM
     runs-on: ubuntu-latest
-    needs: [detect-version-change, build-and-test, create-release]
+    needs: [detect-version-change, build-and-verify, create-release]
     if: needs.detect-version-change.outputs.version-changed == 'true'
     steps:
       - name: Checkout repository
@@ -259,10 +297,16 @@ jobs:
       
       - name: Build project
         run: npm run build
-      
-      - name: Rebuild database
-        run: npm run rebuild
-      
+
+      # Database is already built and committed during development
+      - name: Verify database exists
+        run: |
+          if [ ! -f "data/nodes.db" ]; then
+            echo "❌ Error: data/nodes.db not found"
+            exit 1
+          fi
+          echo "✅ Database exists ($(du -h data/nodes.db | cut -f1))"
+
       - name: Sync runtime version
         run: npm run sync:runtime-version
       
@@ -324,7 +368,7 @@ jobs:
   build-docker:
     name: Build and Push Docker Images
     runs-on: ubuntu-latest
-    needs: [detect-version-change, build-and-test]
+    needs: [detect-version-change, build-and-verify]
     if: needs.detect-version-change.outputs.version-changed == 'true'
     permissions:
       contents: read

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.5",
+  "version": "2.18.0",
   "description": "Integration between n8n workflow automation and Model Context Protocol (MCP)",
   "main": "dist/index.js",
   "bin": {

package.runtime.json

@@ -1,6 +1,6 @@
 {
   "name": "n8n-mcp-runtime",
-  "version": "2.17.3",
+  "version": "2.17.6",
   "description": "n8n MCP Server Runtime Dependencies Only",
   "private": true,
   "dependencies": {

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;

tests/benchmarks/database-queries.bench.ts

@@ -4,6 +4,17 @@ import { SQLiteStorageService } from '../../src/services/sqlite-storage-service'
 import { NodeFactory } from '../factories/node-factory';
 import { PropertyDefinitionFactory } from '../factories/property-definition-factory';
 
+/**
+ * Database Query Performance Benchmarks
+ *
+ * NOTE: These benchmarks use MOCK DATA (500 artificial test nodes)
+ * created with factories, not the real production database.
+ *
+ * This is useful for tracking database layer performance in isolation,
+ * but may not reflect real-world performance characteristics.
+ *
+ * For end-to-end MCP tool performance with real data, see mcp-tools.bench.ts
+ */
 describe('Database Query Performance', () => {
   let repository: NodeRepository;
   let storage: SQLiteStorageService;

tests/benchmarks/index.ts

@@ -1,7 +1,3 @@
 // Export all benchmark suites
-// Note: Some benchmarks are temporarily disabled due to API changes
-// export * from './node-loading.bench';
 export * from './database-queries.bench';
-// export * from './search-operations.bench';
-// export * from './validation-performance.bench';
-// export * from './mcp-tools.bench';
+export * from './mcp-tools.bench';

tests/benchmarks/mcp-tools.bench.ts

@@ -0,0 +1,169 @@
+import { bench, describe } from 'vitest';
+import { NodeRepository } from '../../src/database/node-repository';
+import { createDatabaseAdapter } from '../../src/database/database-adapter';
+import { EnhancedConfigValidator } from '../../src/services/enhanced-config-validator';
+import { PropertyFilter } from '../../src/services/property-filter';
+import path from 'path';
+
+/**
+ * MCP Tool Performance Benchmarks
+ *
+ * These benchmarks measure end-to-end performance of actual MCP tool operations
+ * using the REAL production database (data/nodes.db with 525+ nodes).
+ *
+ * Unlike database-queries.bench.ts which uses mock data, these benchmarks
+ * reflect what AI assistants actually experience when calling MCP tools,
+ * making this the most meaningful performance metric for the system.
+ */
+describe('MCP Tool Performance (Production Database)', () => {
+  let repository: NodeRepository;
+
+  beforeAll(async () => {
+    // Use REAL production database
+    const dbPath = path.join(__dirname, '../../data/nodes.db');
+    const db = await createDatabaseAdapter(dbPath);
+    repository = new NodeRepository(db);
+    // Initialize similarity services for validation
+    EnhancedConfigValidator.initializeSimilarityServices(repository);
+  });
+
+  /**
+   * search_nodes - Most frequently used tool for node discovery
+   *
+   * This measures:
+   * - Database FTS5 full-text search
+   * - Result filtering and ranking
+   * - Response serialization
+   *
+   * Target: <20ms for common queries
+   */
+  bench('search_nodes - common query (http)', async () => {
+    await repository.searchNodes('http', 'OR', 20);
+  }, {
+    iterations: 100,
+    warmupIterations: 10,
+    warmupTime: 500,
+    time: 3000
+  });
+
+  bench('search_nodes - AI agent query (slack message)', async () => {
+    await repository.searchNodes('slack send message', 'AND', 10);
+  }, {
+    iterations: 100,
+    warmupIterations: 10,
+    warmupTime: 500,
+    time: 3000
+  });
+
+  /**
+   * get_node_essentials - Fast retrieval of node configuration
+   *
+   * This measures:
+   * - Database node lookup
+   * - Property filtering (essentials only)
+   * - Response formatting
+   *
+   * Target: <10ms for most nodes
+   */
+  bench('get_node_essentials - HTTP Request node', async () => {
+    const node = await repository.getNodeByType('n8n-nodes-base.httpRequest');
+    if (node && node.properties) {
+      PropertyFilter.getEssentials(node.properties, node.nodeType);
+    }
+  }, {
+    iterations: 200,
+    warmupIterations: 20,
+    warmupTime: 500,
+    time: 3000
+  });
+
+  bench('get_node_essentials - Slack node', async () => {
+    const node = await repository.getNodeByType('n8n-nodes-base.slack');
+    if (node && node.properties) {
+      PropertyFilter.getEssentials(node.properties, node.nodeType);
+    }
+  }, {
+    iterations: 200,
+    warmupIterations: 20,
+    warmupTime: 500,
+    time: 3000
+  });
+
+  /**
+   * list_nodes - Initial exploration/listing
+   *
+   * This measures:
+   * - Database query with pagination
+   * - Result serialization
+   * - Category filtering
+   *
+   * Target: <15ms for first page
+   */
+  bench('list_nodes - first 50 nodes', async () => {
+    await repository.getAllNodes(50);
+  }, {
+    iterations: 100,
+    warmupIterations: 10,
+    warmupTime: 500,
+    time: 3000
+  });
+
+  bench('list_nodes - AI tools only', async () => {
+    await repository.getAIToolNodes();
+  }, {
+    iterations: 100,
+    warmupIterations: 10,
+    warmupTime: 500,
+    time: 3000
+  });
+
+  /**
+   * validate_node_operation - Configuration validation
+   *
+   * This measures:
+   * - Schema lookup
+   * - Validation logic execution
+   * - Error message formatting
+   *
+   * Target: <15ms for simple validations
+   */
+  bench('validate_node_operation - HTTP Request (minimal)', async () => {
+    const node = await repository.getNodeByType('n8n-nodes-base.httpRequest');
+    if (node && node.properties) {
+      EnhancedConfigValidator.validateWithMode(
+        'n8n-nodes-base.httpRequest',
+        {},
+        node.properties,
+        'operation',
+        'ai-friendly'
+      );
+    }
+  }, {
+    iterations: 100,
+    warmupIterations: 10,
+    warmupTime: 500,
+    time: 3000
+  });
+
+  bench('validate_node_operation - HTTP Request (with params)', async () => {
+    const node = await repository.getNodeByType('n8n-nodes-base.httpRequest');
+    if (node && node.properties) {
+      EnhancedConfigValidator.validateWithMode(
+        'n8n-nodes-base.httpRequest',
+        {
+          requestMethod: 'GET',
+          url: 'https://api.example.com',
+          authentication: 'none'
+        },
+        node.properties,
+        'operation',
+        'ai-friendly'
+      );
+    }
+  }, {
+    iterations: 100,
+    warmupIterations: 10,
+    warmupTime: 500,
+    time: 3000
+  });
+});

tests/benchmarks/sample.bench.ts

@@ -1,47 +0,0 @@
-import { bench, describe } from 'vitest';
-
-/**
- * Sample benchmark to verify the setup works correctly
- */
-describe('Sample Benchmarks', () => {
-  bench('array sorting - small', () => {
-    const arr = Array.from({ length: 100 }, () => Math.random());
-    arr.sort((a, b) => a - b);
-  }, {
-    iterations: 1000,
-    warmupIterations: 100
-  });
-
-  bench('array sorting - large', () => {
-    const arr = Array.from({ length: 10000 }, () => Math.random());
-    arr.sort((a, b) => a - b);
-  }, {
-    iterations: 100,
-    warmupIterations: 10
-  });
-
-  bench('string concatenation', () => {
-    let str = '';
-    for (let i = 0; i < 1000; i++) {
-      str += 'a';
-    }
-  }, {
-    iterations: 1000,
-    warmupIterations: 100
-  });
-
-  bench('object creation', () => {
-    const objects = [];
-    for (let i = 0; i < 1000; i++) {
-      objects.push({
-        id: i,
-        name: `Object ${i}`,
-        value: Math.random(),
-        timestamp: Date.now()
-      });
-    }
-  }, {
-    iterations: 1000,
-    warmupIterations: 100
-  });
-});