mirror of
https://github.com/czlonkowski/n8n-mcp.git
synced 2026-04-03 08:03:07 +00:00
fix: correct operator names, connection types, and implement __patch_find_replace (#665, #659, #642) (#672)
Three critical fixes in n8n_update_partial_workflow: - **#665**: Replace incorrect `isNotEmpty`/`isEmpty` operator names with `notEmpty`/`empty` across validators, sanitizer, docs, and error messages. Add auto-correction in sanitizer. Unknown operators silently returned false in n8n's execution engine. - **#659**: Remap numeric `targetInput` values (e.g., "0") to "main" in addConnection. Relax sourceOutput remapping guard for redundant sourceOutput+sourceIndex combinations. Also resolves #653 (dangling connections caused by malformed type:"0" connections). - **#642**: Implement __patch_find_replace for surgical string edits in updateNode. Previously stored patch objects literally as jsCode, producing [object Object]. Now reads current value, applies find/replace sequentially, writes back the string. Conceived by Romuald Członkowski - https://www.aiadvisors.pl/en Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This commit is contained in:
committed by
GitHub
parent
de2abaf89d
commit
6be9ffa53e
@@ -445,7 +445,7 @@ describe('Integration: Real-World Type Structure Validation', () => {
|
||||
expect(sheetIdErrors).toBe(0);
|
||||
});
|
||||
|
||||
it('should validate all filter operations including exists/notExists/isNotEmpty', async () => {
|
||||
it('should validate all filter operations including exists/notExists/notEmpty', async () => {
|
||||
const templates = db.prepare(`
|
||||
SELECT id, name, workflow_json_compressed
|
||||
FROM templates
|
||||
|
||||
@@ -102,9 +102,9 @@ describe('Node Sanitizer', () => {
|
||||
const sanitized = sanitizeNode(node);
|
||||
const condition = (sanitized.parameters.conditions as any).conditions[0];
|
||||
|
||||
// Should fix operator structure
|
||||
expect(condition.operator.type).toBe('boolean'); // Inferred data type (isEmpty/isNotEmpty are boolean ops)
|
||||
expect(condition.operator.operation).toBe('isNotEmpty'); // Moved to operation field
|
||||
// Should fix operator structure and auto-correct isNotEmpty to notEmpty
|
||||
expect(condition.operator.type).toBe('string'); // Inferred data type (default)
|
||||
expect(condition.operator.operation).toBe('notEmpty'); // Moved to operation field and auto-corrected
|
||||
});
|
||||
|
||||
it('should add singleValue for unary operators', () => {
|
||||
@@ -253,6 +253,39 @@ describe('Node Sanitizer', () => {
|
||||
expect(condition.operator.type).toBe('string');
|
||||
expect(condition.operator.operation).toBe('equals');
|
||||
});
|
||||
|
||||
it('should auto-correct isNotEmpty to notEmpty', () => {
|
||||
const node: WorkflowNode = {
|
||||
id: 'test-if-autocorrect',
|
||||
name: 'IF AutoCorrect',
|
||||
type: 'n8n-nodes-base.if',
|
||||
typeVersion: 2.2,
|
||||
position: [0, 0],
|
||||
parameters: {
|
||||
conditions: {
|
||||
conditions: [
|
||||
{
|
||||
id: 'condition1',
|
||||
leftValue: '={{ $json.value }}',
|
||||
rightValue: '',
|
||||
operator: {
|
||||
type: 'string',
|
||||
operation: 'isNotEmpty' // Legacy operator name
|
||||
}
|
||||
}
|
||||
]
|
||||
}
|
||||
}
|
||||
};
|
||||
|
||||
const sanitized = sanitizeNode(node);
|
||||
const condition = (sanitized.parameters.conditions as any).conditions[0];
|
||||
|
||||
// Should auto-correct isNotEmpty to notEmpty
|
||||
expect(condition.operator.operation).toBe('notEmpty');
|
||||
expect(condition.operator.type).toBe('string');
|
||||
expect(condition.operator.singleValue).toBe(true); // notEmpty is unary
|
||||
});
|
||||
});
|
||||
|
||||
describe('validateNodeMetadata', () => {
|
||||
@@ -370,7 +403,7 @@ describe('Node Sanitizer', () => {
|
||||
rightValue: '',
|
||||
operator: {
|
||||
type: 'string',
|
||||
operation: 'isNotEmpty'
|
||||
operation: 'notEmpty'
|
||||
// Missing singleValue: true
|
||||
}
|
||||
}
|
||||
@@ -444,7 +477,7 @@ describe('Node Sanitizer', () => {
|
||||
rightValue: '',
|
||||
operator: {
|
||||
type: 'string',
|
||||
operation: 'isNotEmpty',
|
||||
operation: 'notEmpty',
|
||||
singleValue: true
|
||||
}
|
||||
}
|
||||
|
||||
@@ -427,6 +427,158 @@ describe('WorkflowDiffEngine', () => {
|
||||
expect(result.errors![0].message).toContain('Missing required parameter \'updates\'');
|
||||
expect(result.errors![0].message).toContain('Correct structure:');
|
||||
});
|
||||
|
||||
it('should apply __patch_find_replace to string properties (#642)', async () => {
|
||||
const workflow = JSON.parse(JSON.stringify(baseWorkflow));
|
||||
workflow.nodes.push({
|
||||
id: 'code-1',
|
||||
name: 'Code',
|
||||
type: 'n8n-nodes-base.code',
|
||||
typeVersion: 1,
|
||||
position: [900, 300],
|
||||
parameters: { jsCode: 'const x = 1;\nreturn x + 2;' }
|
||||
});
|
||||
|
||||
const result = await diffEngine.applyDiff(workflow, {
|
||||
id: 'test',
|
||||
operations: [{
|
||||
type: 'updateNode' as const,
|
||||
nodeName: 'Code',
|
||||
updates: {
|
||||
'parameters.jsCode': {
|
||||
__patch_find_replace: [
|
||||
{ find: 'x + 2', replace: 'x + 3' }
|
||||
]
|
||||
}
|
||||
}
|
||||
}]
|
||||
});
|
||||
|
||||
expect(result.success).toBe(true);
|
||||
const codeNode = result.workflow.nodes.find((n: any) => n.name === 'Code');
|
||||
expect(codeNode?.parameters.jsCode).toBe('const x = 1;\nreturn x + 3;');
|
||||
});
|
||||
|
||||
it('should apply multiple sequential __patch_find_replace patches', async () => {
|
||||
const workflow = JSON.parse(JSON.stringify(baseWorkflow));
|
||||
workflow.nodes.push({
|
||||
id: 'code-1',
|
||||
name: 'Code',
|
||||
type: 'n8n-nodes-base.code',
|
||||
typeVersion: 1,
|
||||
position: [900, 300],
|
||||
parameters: { jsCode: 'const a = 1;\nconst b = 2;\nreturn a + b;' }
|
||||
});
|
||||
|
||||
const result = await diffEngine.applyDiff(workflow, {
|
||||
id: 'test',
|
||||
operations: [{
|
||||
type: 'updateNode' as const,
|
||||
nodeName: 'Code',
|
||||
updates: {
|
||||
'parameters.jsCode': {
|
||||
__patch_find_replace: [
|
||||
{ find: 'const a = 1', replace: 'const a = 10' },
|
||||
{ find: 'const b = 2', replace: 'const b = 20' }
|
||||
]
|
||||
}
|
||||
}
|
||||
}]
|
||||
});
|
||||
|
||||
expect(result.success).toBe(true);
|
||||
const codeNode = result.workflow.nodes.find((n: any) => n.name === 'Code');
|
||||
expect(codeNode?.parameters.jsCode).toBe('const a = 10;\nconst b = 20;\nreturn a + b;');
|
||||
});
|
||||
|
||||
it('should reject __patch_find_replace on non-string properties', async () => {
|
||||
const workflow = JSON.parse(JSON.stringify(baseWorkflow));
|
||||
workflow.nodes.push({
|
||||
id: 'code-1',
|
||||
name: 'Code',
|
||||
type: 'n8n-nodes-base.code',
|
||||
typeVersion: 1,
|
||||
position: [900, 300],
|
||||
parameters: { retryCount: 3 }
|
||||
});
|
||||
|
||||
const result = await diffEngine.applyDiff(workflow, {
|
||||
id: 'test',
|
||||
operations: [{
|
||||
type: 'updateNode' as const,
|
||||
nodeName: 'Code',
|
||||
updates: {
|
||||
'parameters.retryCount': {
|
||||
__patch_find_replace: [
|
||||
{ find: '3', replace: '5' }
|
||||
]
|
||||
}
|
||||
}
|
||||
}]
|
||||
});
|
||||
|
||||
expect(result.success).toBe(false);
|
||||
expect(result.errors?.[0]?.message).toContain('__patch_find_replace');
|
||||
});
|
||||
|
||||
it('should reject __patch_find_replace with invalid format', async () => {
|
||||
const workflow = JSON.parse(JSON.stringify(baseWorkflow));
|
||||
workflow.nodes.push({
|
||||
id: 'code-1',
|
||||
name: 'Code',
|
||||
type: 'n8n-nodes-base.code',
|
||||
typeVersion: 1,
|
||||
position: [900, 300],
|
||||
parameters: { jsCode: 'const x = 1;' }
|
||||
});
|
||||
|
||||
const result = await diffEngine.applyDiff(workflow, {
|
||||
id: 'test',
|
||||
operations: [{
|
||||
type: 'updateNode' as const,
|
||||
nodeName: 'Code',
|
||||
updates: {
|
||||
'parameters.jsCode': {
|
||||
__patch_find_replace: 'not an array'
|
||||
}
|
||||
}
|
||||
}]
|
||||
});
|
||||
|
||||
expect(result.success).toBe(false);
|
||||
expect(result.errors?.[0]?.message).toContain('must be an array');
|
||||
});
|
||||
|
||||
it('should warn when __patch_find_replace find string not found', async () => {
|
||||
const workflow = JSON.parse(JSON.stringify(baseWorkflow));
|
||||
workflow.nodes.push({
|
||||
id: 'code-1',
|
||||
name: 'Code',
|
||||
type: 'n8n-nodes-base.code',
|
||||
typeVersion: 1,
|
||||
position: [900, 300],
|
||||
parameters: { jsCode: 'const x = 1;' }
|
||||
});
|
||||
|
||||
const result = await diffEngine.applyDiff(workflow, {
|
||||
id: 'test',
|
||||
operations: [{
|
||||
type: 'updateNode' as const,
|
||||
nodeName: 'Code',
|
||||
updates: {
|
||||
'parameters.jsCode': {
|
||||
__patch_find_replace: [
|
||||
{ find: 'nonexistent text', replace: 'something' }
|
||||
]
|
||||
}
|
||||
}
|
||||
}]
|
||||
});
|
||||
|
||||
expect(result.success).toBe(true);
|
||||
expect(result.warnings).toBeDefined();
|
||||
expect(result.warnings!.some(w => w.message.includes('not found'))).toBe(true);
|
||||
});
|
||||
});
|
||||
|
||||
describe('MoveNode Operation', () => {
|
||||
@@ -766,6 +918,97 @@ describe('WorkflowDiffEngine', () => {
|
||||
expect(result.errors![0].message).toContain('HTTP Request');
|
||||
expect(result.errors![0].message).toContain('Slack');
|
||||
});
|
||||
|
||||
it('should remap numeric targetInput to main (#659)', async () => {
|
||||
const workflow = JSON.parse(JSON.stringify(baseWorkflow));
|
||||
workflow.nodes.push({
|
||||
id: 'code-1',
|
||||
name: 'Code',
|
||||
type: 'n8n-nodes-base.code',
|
||||
typeVersion: 1,
|
||||
position: [900, 300],
|
||||
parameters: {}
|
||||
});
|
||||
|
||||
const result = await diffEngine.applyDiff(workflow, {
|
||||
id: 'test',
|
||||
operations: [{
|
||||
type: 'addConnection' as const,
|
||||
source: 'Slack',
|
||||
target: 'Code',
|
||||
sourceOutput: 'main',
|
||||
targetInput: '0',
|
||||
sourceIndex: 0,
|
||||
targetIndex: 0
|
||||
}]
|
||||
});
|
||||
|
||||
expect(result.success).toBe(true);
|
||||
expect(result.workflow.connections['Slack']['main'][0][0].type).toBe('main');
|
||||
});
|
||||
|
||||
it('should remap sourceOutput 0 with explicit sourceIndex 0 (#659)', async () => {
|
||||
const workflow = JSON.parse(JSON.stringify(baseWorkflow));
|
||||
workflow.nodes.push({
|
||||
id: 'code-1',
|
||||
name: 'Code',
|
||||
type: 'n8n-nodes-base.code',
|
||||
typeVersion: 1,
|
||||
position: [900, 300],
|
||||
parameters: {}
|
||||
});
|
||||
|
||||
const result = await diffEngine.applyDiff(workflow, {
|
||||
id: 'test',
|
||||
operations: [{
|
||||
type: 'addConnection' as const,
|
||||
source: 'Slack',
|
||||
target: 'Code',
|
||||
sourceOutput: '0',
|
||||
sourceIndex: 0,
|
||||
targetIndex: 0
|
||||
}]
|
||||
});
|
||||
|
||||
expect(result.success).toBe(true);
|
||||
expect(result.workflow.connections['Slack']['main']).toBeDefined();
|
||||
expect(result.workflow.connections['Slack']['0']).toBeUndefined();
|
||||
expect(result.workflow.connections['Slack']['main'][0][0].type).toBe('main');
|
||||
});
|
||||
|
||||
it('should preserve named targetInput like ai_tool', async () => {
|
||||
const workflow = JSON.parse(JSON.stringify(baseWorkflow));
|
||||
workflow.nodes.push({
|
||||
id: 'agent-1',
|
||||
name: 'AI Agent',
|
||||
type: '@n8n/n8n-nodes-langchain.agent',
|
||||
typeVersion: 1,
|
||||
position: [900, 300],
|
||||
parameters: {}
|
||||
});
|
||||
workflow.nodes.push({
|
||||
id: 'tool-1',
|
||||
name: 'Calculator',
|
||||
type: '@n8n/n8n-nodes-langchain.toolCalculator',
|
||||
typeVersion: 1,
|
||||
position: [1100, 300],
|
||||
parameters: {}
|
||||
});
|
||||
|
||||
const result = await diffEngine.applyDiff(workflow, {
|
||||
id: 'test',
|
||||
operations: [{
|
||||
type: 'addConnection' as const,
|
||||
source: 'Calculator',
|
||||
target: 'AI Agent',
|
||||
sourceOutput: 'ai_tool',
|
||||
targetInput: 'ai_tool'
|
||||
}]
|
||||
});
|
||||
|
||||
expect(result.success).toBe(true);
|
||||
expect(result.workflow.connections['Calculator']['ai_tool'][0][0].type).toBe('ai_tool');
|
||||
});
|
||||
});
|
||||
|
||||
describe('RemoveConnection Operation', () => {
|
||||
|
||||
Reference in New Issue
Block a user