From 4c217088f576cf516bb6789df5f6b340281ce47e Mon Sep 17 00:00:00 2001 From: czlonkowski <56956555+czlonkowski@users.noreply.github.com> Date: Tue, 15 Jul 2025 09:13:32 +0200 Subject: [PATCH] fix: resolve partial update validation/execution discrepancy (issue #45) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Remove default settings logic from cleanWorkflowForUpdate that was causing "settings must NOT have additional properties" error - The function now only removes read-only fields without adding any properties - Add comprehensive test coverage in test-issue-45-fix.ts - Add documentation explaining the difference between create and update functions - Bump version to 2.7.14 This fixes the issue where n8n_update_partial_workflow would pass validation but fail during execution when workflows didn't have settings defined. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude --- docs/CHANGELOG.md | 9 ++ package.json | 3 +- src/scripts/test-issue-45-fix.ts | 165 +++++++++++++++++++++++++++++++ src/services/n8n-validation.ts | 18 +++- 4 files changed, 189 insertions(+), 6 deletions(-) create mode 100644 src/scripts/test-issue-45-fix.ts diff --git a/docs/CHANGELOG.md b/docs/CHANGELOG.md index 9f50f62..1fee139 100644 --- a/docs/CHANGELOG.md +++ b/docs/CHANGELOG.md @@ -5,6 +5,15 @@ 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.7.14] - 2025-07-15 + +### Fixed +- **Partial Update Tool**: Fixed validation/execution discrepancy that caused "settings must NOT have additional properties" error (Issue #45) + - Removed logic in `cleanWorkflowForUpdate` that was incorrectly adding default settings to workflows + - The function now only removes read-only fields without adding any new properties + - This fixes the issue where partial updates would pass validation but fail during execution + - Added comprehensive test coverage in `test-issue-45-fix.ts` + ## [2.7.13] - 2025-07-11 ### Fixed diff --git a/package.json b/package.json index da95549..4e67bd3 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "n8n-mcp", - "version": "2.7.13", + "version": "2.7.14", "description": "Integration between n8n workflow automation and Model Context Protocol (MCP)", "main": "dist/index.js", "bin": { @@ -46,6 +46,7 @@ "migrate:fts5": "node dist/scripts/migrate-nodes-fts.js", "test:mcp:update-partial": "node dist/scripts/test-mcp-n8n-update-partial.js", "test:update-partial:debug": "node dist/scripts/test-update-partial-debug.js", + "test:issue-45-fix": "node dist/scripts/test-issue-45-fix.js", "test:auth-logging": "tsx scripts/test-auth-logging.ts", "sanitize:templates": "node dist/scripts/sanitize-templates.js", "db:rebuild": "node dist/scripts/rebuild-database.js", diff --git a/src/scripts/test-issue-45-fix.ts b/src/scripts/test-issue-45-fix.ts new file mode 100644 index 0000000..0d24e4c --- /dev/null +++ b/src/scripts/test-issue-45-fix.ts @@ -0,0 +1,165 @@ +#!/usr/bin/env node +/** + * Test for Issue #45 Fix: Partial Update Tool Validation/Execution Discrepancy + * + * This test verifies that the cleanWorkflowForUpdate function no longer adds + * default settings to workflows during updates, which was causing the n8n API + * to reject requests with "settings must NOT have additional properties". + */ + +import { config } from 'dotenv'; +import { logger } from '../utils/logger'; +import { cleanWorkflowForUpdate, cleanWorkflowForCreate } from '../services/n8n-validation'; +import { Workflow } from '../types/n8n-api'; + +// Load environment variables +config(); + +function testCleanWorkflowFunctions() { + logger.info('Testing Issue #45 Fix: cleanWorkflowForUpdate should not add default settings\n'); + + // Test 1: cleanWorkflowForUpdate with workflow without settings + logger.info('=== Test 1: cleanWorkflowForUpdate without settings ==='); + const workflowWithoutSettings: Workflow = { + id: 'test-123', + name: 'Test Workflow', + nodes: [], + connections: {}, + active: false, + createdAt: '2024-01-01T00:00:00.000Z', + updatedAt: '2024-01-01T00:00:00.000Z', + versionId: 'version-123' + }; + + const cleanedUpdate = cleanWorkflowForUpdate(workflowWithoutSettings); + + if ('settings' in cleanedUpdate) { + logger.error('❌ FAIL: cleanWorkflowForUpdate added settings when it should not have'); + logger.error(' Found settings:', JSON.stringify(cleanedUpdate.settings)); + } else { + logger.info('✅ PASS: cleanWorkflowForUpdate did not add settings'); + } + + // Test 2: cleanWorkflowForUpdate with existing settings + logger.info('\n=== Test 2: cleanWorkflowForUpdate with existing settings ==='); + const workflowWithSettings: Workflow = { + ...workflowWithoutSettings, + settings: { + executionOrder: 'v1', + saveDataErrorExecution: 'none', + saveDataSuccessExecution: 'none', + saveManualExecutions: false, + saveExecutionProgress: false + } + }; + + const cleanedUpdate2 = cleanWorkflowForUpdate(workflowWithSettings); + + if ('settings' in cleanedUpdate2) { + const settingsMatch = JSON.stringify(cleanedUpdate2.settings) === JSON.stringify(workflowWithSettings.settings); + if (settingsMatch) { + logger.info('✅ PASS: cleanWorkflowForUpdate preserved existing settings without modification'); + } else { + logger.error('❌ FAIL: cleanWorkflowForUpdate modified existing settings'); + logger.error(' Original:', JSON.stringify(workflowWithSettings.settings)); + logger.error(' Cleaned:', JSON.stringify(cleanedUpdate2.settings)); + } + } else { + logger.error('❌ FAIL: cleanWorkflowForUpdate removed existing settings'); + } + + // Test 3: cleanWorkflowForUpdate with partial settings + logger.info('\n=== Test 3: cleanWorkflowForUpdate with partial settings ==='); + const workflowWithPartialSettings: Workflow = { + ...workflowWithoutSettings, + settings: { + executionOrder: 'v1' + // Missing other default properties + } + }; + + const cleanedUpdate3 = cleanWorkflowForUpdate(workflowWithPartialSettings); + + if ('settings' in cleanedUpdate3) { + const settingsKeys = cleanedUpdate3.settings ? Object.keys(cleanedUpdate3.settings) : []; + const hasOnlyExecutionOrder = settingsKeys.length === 1 && + cleanedUpdate3.settings?.executionOrder === 'v1'; + if (hasOnlyExecutionOrder) { + logger.info('✅ PASS: cleanWorkflowForUpdate preserved partial settings without adding defaults'); + } else { + logger.error('❌ FAIL: cleanWorkflowForUpdate added default properties to partial settings'); + logger.error(' Original keys:', Object.keys(workflowWithPartialSettings.settings || {})); + logger.error(' Cleaned keys:', settingsKeys); + } + } else { + logger.error('❌ FAIL: cleanWorkflowForUpdate removed partial settings'); + } + + // Test 4: Verify cleanWorkflowForCreate still adds defaults + logger.info('\n=== Test 4: cleanWorkflowForCreate should add default settings ==='); + const newWorkflow = { + name: 'New Workflow', + nodes: [], + connections: {} + }; + + const cleanedCreate = cleanWorkflowForCreate(newWorkflow); + + if ('settings' in cleanedCreate && cleanedCreate.settings) { + const hasDefaults = + cleanedCreate.settings.executionOrder === 'v1' && + cleanedCreate.settings.saveDataErrorExecution === 'all' && + cleanedCreate.settings.saveDataSuccessExecution === 'all' && + cleanedCreate.settings.saveManualExecutions === true && + cleanedCreate.settings.saveExecutionProgress === true; + + if (hasDefaults) { + logger.info('✅ PASS: cleanWorkflowForCreate correctly adds default settings'); + } else { + logger.error('❌ FAIL: cleanWorkflowForCreate added settings but not with correct defaults'); + logger.error(' Settings:', JSON.stringify(cleanedCreate.settings)); + } + } else { + logger.error('❌ FAIL: cleanWorkflowForCreate did not add default settings'); + } + + // Test 5: Verify read-only fields are removed + logger.info('\n=== Test 5: cleanWorkflowForUpdate removes read-only fields ==='); + const workflowWithReadOnly: any = { + ...workflowWithoutSettings, + staticData: { some: 'data' }, + pinData: { node1: 'data' }, + tags: ['tag1', 'tag2'], + isArchived: true, + usedCredentials: ['cred1'], + sharedWithProjects: ['proj1'], + triggerCount: 5, + shared: true, + active: true + }; + + const cleanedReadOnly = cleanWorkflowForUpdate(workflowWithReadOnly); + + const removedFields = [ + 'id', 'createdAt', 'updatedAt', 'versionId', 'meta', + 'staticData', 'pinData', 'tags', 'isArchived', + 'usedCredentials', 'sharedWithProjects', 'triggerCount', + 'shared', 'active' + ]; + + const hasRemovedFields = removedFields.some(field => field in cleanedReadOnly); + + if (!hasRemovedFields) { + logger.info('✅ PASS: cleanWorkflowForUpdate correctly removed all read-only fields'); + } else { + const foundFields = removedFields.filter(field => field in cleanedReadOnly); + logger.error('❌ FAIL: cleanWorkflowForUpdate did not remove these fields:', foundFields); + } + + logger.info('\n=== Test Summary ==='); + logger.info('All tests completed. The fix ensures that cleanWorkflowForUpdate only removes fields'); + logger.info('without adding default settings, preventing the n8n API validation error.'); +} + +// Run the tests +testCleanWorkflowFunctions(); \ No newline at end of file diff --git a/src/services/n8n-validation.ts b/src/services/n8n-validation.ts index 814e61e..b699509 100644 --- a/src/services/n8n-validation.ts +++ b/src/services/n8n-validation.ts @@ -93,6 +93,19 @@ export function cleanWorkflowForCreate(workflow: Partial): Partial { const { // Remove read-only/computed fields @@ -116,11 +129,6 @@ export function cleanWorkflowForUpdate(workflow: Workflow): Partial { ...cleanedWorkflow } = workflow as any; - // Ensure settings are present - if (!cleanedWorkflow.settings) { - cleanedWorkflow.settings = defaultWorkflowSettings; - } - return cleanedWorkflow; }