mirror of
https://github.com/czlonkowski/n8n-mcp.git
synced 2026-03-20 17:33:08 +00:00
fix: resolve empty settings validation error in workflow updates (#431)
Fixed critical bug where n8n_update_partial_workflow failed with "request/body
must NOT have additional properties" error when workflows had no settings or
only non-whitelisted settings.
Root Cause:
- cleanWorkflowForUpdate() was sending empty settings: {} objects
- n8n API rejects empty settings as additional properties violation
- Occurred when workflow had no settings or only filtered properties
Changes:
- Modified cleanWorkflowForUpdate() to delete settings property when empty
- Enhanced applyUpdateSettings() to prevent creating empty settings
- Fixed 3 incorrect tests expecting empty settings objects
- Added 2 comprehensive tests for edge cases
Testing:
- All 75 unit tests in n8n-validation.test.ts passing
- Build and type checking successful
- New tests cover all empty settings scenarios
Related: #431, #248
Related n8n issue: n8n-io/n8n#19587
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude <noreply@anthropic.com>
Conceived by Romuald Członkowski - https://www.aiadvisors.pl/en
This commit is contained in:
@@ -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([]);
|
||||
});
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user