diff --git a/CHANGELOG.md b/CHANGELOG.md index 4fc19ae..8315e72 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,43 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +## [2.22.21] - 2025-11-20 + +### 🐛 Bug Fixes + +**Fix Empty Settings Object Validation Error (#431)** + +Fixed critical bug where `n8n_update_partial_workflow` tool failed with "request/body must NOT have additional properties" error when workflows had no settings or only non-whitelisted settings properties. + +#### Root Cause +- `cleanWorkflowForUpdate()` in `src/services/n8n-validation.ts` was sending empty `settings: {}` objects to the n8n API +- n8n API rejects empty settings objects as "additional properties" violation +- Issue occurred when: + - Workflow had no settings property + - Workflow had only non-whitelisted settings (e.g., only `callerPolicy`) + +#### Changes +- **Primary Fix**: Modified `cleanWorkflowForUpdate()` to delete `settings` property when empty after filtering + - Instead of sending `settings: {}`, the property is now omitted entirely + - Added safeguards in lines 193-199 and 201-204 +- **Secondary Fix**: Enhanced `applyUpdateSettings()` in `workflow-diff-engine.ts` to prevent creating empty settings objects + - Only creates/updates settings if operation provides actual properties +- **Test Updates**: Fixed 3 incorrect tests that expected empty settings objects + - Updated to expect settings property to be omitted instead + - Added 2 new comprehensive tests for edge cases + +#### Testing +- All 75 unit tests in `n8n-validation.test.ts` passing +- New tests cover: + - Workflows with no settings → omits property + - Workflows with only non-whitelisted settings → omits property + - Workflows with mixed settings → keeps only whitelisted properties + +**Related Issues**: #431, #248 (n8n API design limitation) +**Related n8n Issue**: n8n-io/n8n#19587 (closed as NOT_PLANNED - MCP server issue) + +Conceived by Romuald Członkowski - https://www.aiadvisors.pl/en + ## [2.22.20] - 2025-11-19 ### 🔄 Dependencies diff --git a/package.json b/package.json index 37d177b..26a2dd3 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "n8n-mcp", - "version": "2.22.20", + "version": "2.22.21", "description": "Integration between n8n workflow automation and Model Context Protocol (MCP)", "main": "dist/index.js", "types": "dist/index.d.ts", diff --git a/src/services/n8n-validation.ts b/src/services/n8n-validation.ts index 439a017..f0aa22e 100644 --- a/src/services/n8n-validation.ts +++ b/src/services/n8n-validation.ts @@ -189,10 +189,18 @@ export function cleanWorkflowForUpdate(workflow: Workflow): Partial { filteredSettings[key] = (cleanedWorkflow.settings as any)[key]; } } - cleanedWorkflow.settings = filteredSettings; + + // Only include settings if it has properties after filtering + // n8n API rejects empty settings objects + if (Object.keys(filteredSettings).length > 0) { + cleanedWorkflow.settings = filteredSettings; + } else { + delete cleanedWorkflow.settings; + } } else { - // No settings provided - use empty object for safety - cleanedWorkflow.settings = {}; + // No settings provided - remove the property entirely + // n8n API rejects empty settings objects + delete cleanedWorkflow.settings; } return cleanedWorkflow; diff --git a/src/services/workflow-diff-engine.ts b/src/services/workflow-diff-engine.ts index 9b78e95..3538dd8 100644 --- a/src/services/workflow-diff-engine.ts +++ b/src/services/workflow-diff-engine.ts @@ -861,10 +861,14 @@ export class WorkflowDiffEngine { // Metadata operation appliers private applyUpdateSettings(workflow: Workflow, operation: UpdateSettingsOperation): void { - if (!workflow.settings) { - workflow.settings = {}; + // Only create/update settings if operation provides actual properties + // This prevents creating empty settings objects that would be rejected by n8n API + if (operation.settings && Object.keys(operation.settings).length > 0) { + if (!workflow.settings) { + workflow.settings = {}; + } + Object.assign(workflow.settings, operation.settings); } - Object.assign(workflow.settings, operation.settings); } private applyUpdateName(workflow: Workflow, operation: UpdateNameOperation): void { diff --git a/tests/unit/services/n8n-validation.test.ts b/tests/unit/services/n8n-validation.test.ts index 1d5e9e5..b0e5cdc 100644 --- a/tests/unit/services/n8n-validation.test.ts +++ b/tests/unit/services/n8n-validation.test.ts @@ -367,7 +367,7 @@ describe('n8n-validation', () => { expect(cleaned.name).toBe('Test Workflow'); }); - it('should add empty settings object for cloud API compatibility', () => { + it('should omit settings property when no settings provided (Issue #431)', () => { const workflow = { name: 'Test Workflow', nodes: [], @@ -375,7 +375,7 @@ describe('n8n-validation', () => { } as any; const cleaned = cleanWorkflowForUpdate(workflow); - expect(cleaned.settings).toEqual({}); + expect(cleaned).not.toHaveProperty('settings'); }); it('should filter settings to safe properties to prevent API errors (Issue #248 - final fix)', () => { @@ -467,7 +467,48 @@ describe('n8n-validation', () => { } as any; const cleaned = cleanWorkflowForUpdate(workflow); - expect(cleaned.settings).toEqual({}); + expect(cleaned).not.toHaveProperty('settings'); + }); + + it('should omit settings when only non-whitelisted properties exist (Issue #431)', () => { + const workflow = { + name: 'Test Workflow', + nodes: [], + connections: {}, + settings: { + callerPolicy: 'workflowsFromSameOwner' as const, // Filtered out + timeSavedPerExecution: 5, // Filtered out (UI-only) + someOtherProperty: 'value', // Filtered out + }, + } as any; + + const cleaned = cleanWorkflowForUpdate(workflow); + // All properties were filtered out, so settings should not exist + // to avoid "must NOT have additional properties" API error + expect(cleaned).not.toHaveProperty('settings'); + }); + + it('should preserve whitelisted settings when mixed with non-whitelisted (Issue #431)', () => { + const workflow = { + name: 'Test Workflow', + nodes: [], + connections: {}, + settings: { + executionOrder: 'v1' as const, // Whitelisted + callerPolicy: 'workflowsFromSameOwner' as const, // Filtered out + timezone: 'America/New_York', // Whitelisted + someOtherProperty: 'value', // Filtered out + }, + } as any; + + const cleaned = cleanWorkflowForUpdate(workflow); + // Should keep only whitelisted properties + expect(cleaned.settings).toEqual({ + executionOrder: 'v1', + timezone: 'America/New_York' + }); + expect(cleaned.settings).not.toHaveProperty('callerPolicy'); + expect(cleaned.settings).not.toHaveProperty('someOtherProperty'); }); }); }); @@ -1346,7 +1387,7 @@ describe('n8n-validation', () => { expect(forUpdate).not.toHaveProperty('active'); expect(forUpdate).not.toHaveProperty('tags'); expect(forUpdate).not.toHaveProperty('meta'); - expect(forUpdate.settings).toEqual({}); // Settings replaced with empty object for API compatibility + expect(forUpdate).not.toHaveProperty('settings'); // Settings omitted when not present (Issue #431) expect(validateWorkflowStructure(forUpdate)).toEqual([]); }); });