mirror of
https://github.com/czlonkowski/n8n-mcp.git
synced 2026-03-23 10:53:07 +00:00
Add transferWorkflow diff operation to move workflows between n8n projects:
- TransferWorkflowOperation type with destinationProjectId field
- WorkflowDiffEngine validates and tracks transfer intent
- Handler calls PUT /workflows/{id}/transfer after update
- N8nApiClient.transferWorkflow() method
- Zod schema validates destinationProjectId is non-empty
- Tool description and documentation updated
- inferIntentFromOperations case for transfer
Also fixes two pre-existing bugs found during review:
- continueOnError path now properly extracts/propagates activation flags
- Debug log in updateConnectionReferences shows correct old name
Based on work by @djakielski in PR #645.
Conceived by Romuald Członkowski - https://www.aiadvisors.pl/en
This commit is contained in:
committed by
GitHub
parent
14962a39b6
commit
47a1cb135d
@@ -76,6 +76,7 @@ describe('handlers-workflow-diff', () => {
|
||||
listTags: vi.fn().mockResolvedValue({ data: [] }),
|
||||
createTag: vi.fn(),
|
||||
updateWorkflowTags: vi.fn().mockResolvedValue([]),
|
||||
transferWorkflow: vi.fn().mockResolvedValue(undefined),
|
||||
};
|
||||
|
||||
// Setup mock diff engine
|
||||
@@ -1002,6 +1003,186 @@ describe('handlers-workflow-diff', () => {
|
||||
});
|
||||
});
|
||||
|
||||
describe('Project Transfer via Dedicated API', () => {
|
||||
it('should call transferWorkflow when diffResult has transferToProjectId', async () => {
|
||||
const testWorkflow = createTestWorkflow();
|
||||
const updatedWorkflow = { ...testWorkflow };
|
||||
|
||||
mockApiClient.getWorkflow.mockResolvedValue(testWorkflow);
|
||||
mockDiffEngine.applyDiff.mockResolvedValue({
|
||||
success: true,
|
||||
workflow: updatedWorkflow,
|
||||
operationsApplied: 1,
|
||||
message: 'Success',
|
||||
errors: [],
|
||||
transferToProjectId: 'project-abc-123',
|
||||
});
|
||||
mockApiClient.updateWorkflow.mockResolvedValue(updatedWorkflow);
|
||||
|
||||
const result = await handleUpdatePartialWorkflow({
|
||||
id: 'test-workflow-id',
|
||||
operations: [{ type: 'transferWorkflow', destinationProjectId: 'project-abc-123' }],
|
||||
}, mockRepository);
|
||||
|
||||
expect(result.success).toBe(true);
|
||||
expect(mockApiClient.transferWorkflow).toHaveBeenCalledWith('test-workflow-id', 'project-abc-123');
|
||||
expect(result.message).toContain('transferred to project');
|
||||
});
|
||||
|
||||
it('should NOT call transferWorkflow when transferToProjectId is absent', async () => {
|
||||
const testWorkflow = createTestWorkflow();
|
||||
const updatedWorkflow = { ...testWorkflow };
|
||||
|
||||
mockApiClient.getWorkflow.mockResolvedValue(testWorkflow);
|
||||
mockDiffEngine.applyDiff.mockResolvedValue({
|
||||
success: true,
|
||||
workflow: updatedWorkflow,
|
||||
operationsApplied: 1,
|
||||
message: 'Success',
|
||||
errors: [],
|
||||
});
|
||||
mockApiClient.updateWorkflow.mockResolvedValue(updatedWorkflow);
|
||||
|
||||
await handleUpdatePartialWorkflow({
|
||||
id: 'test-workflow-id',
|
||||
operations: [{ type: 'updateName', name: 'New Name' }],
|
||||
}, mockRepository);
|
||||
|
||||
expect(mockApiClient.transferWorkflow).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('should return success false with saved true when transfer fails', async () => {
|
||||
const testWorkflow = createTestWorkflow();
|
||||
const updatedWorkflow = { ...testWorkflow };
|
||||
|
||||
mockApiClient.getWorkflow.mockResolvedValue(testWorkflow);
|
||||
mockDiffEngine.applyDiff.mockResolvedValue({
|
||||
success: true,
|
||||
workflow: updatedWorkflow,
|
||||
operationsApplied: 1,
|
||||
message: 'Success',
|
||||
errors: [],
|
||||
transferToProjectId: 'project-bad-id',
|
||||
});
|
||||
mockApiClient.updateWorkflow.mockResolvedValue(updatedWorkflow);
|
||||
mockApiClient.transferWorkflow.mockRejectedValue(new Error('Project not found'));
|
||||
|
||||
const result = await handleUpdatePartialWorkflow({
|
||||
id: 'test-workflow-id',
|
||||
operations: [{ type: 'transferWorkflow', destinationProjectId: 'project-bad-id' }],
|
||||
}, mockRepository);
|
||||
|
||||
expect(result.success).toBe(false);
|
||||
expect(result.saved).toBe(true);
|
||||
expect(result.error).toBe('Workflow updated successfully but project transfer failed');
|
||||
expect(result.details).toEqual({
|
||||
workflowUpdated: true,
|
||||
transferError: 'Project not found',
|
||||
});
|
||||
});
|
||||
|
||||
it('should return Unknown error when non-Error value is thrown during transfer', async () => {
|
||||
const testWorkflow = createTestWorkflow();
|
||||
const updatedWorkflow = { ...testWorkflow };
|
||||
|
||||
mockApiClient.getWorkflow.mockResolvedValue(testWorkflow);
|
||||
mockDiffEngine.applyDiff.mockResolvedValue({
|
||||
success: true,
|
||||
workflow: updatedWorkflow,
|
||||
operationsApplied: 1,
|
||||
message: 'Success',
|
||||
errors: [],
|
||||
transferToProjectId: 'project-unknown',
|
||||
});
|
||||
mockApiClient.updateWorkflow.mockResolvedValue(updatedWorkflow);
|
||||
mockApiClient.transferWorkflow.mockRejectedValue('string error');
|
||||
|
||||
const result = await handleUpdatePartialWorkflow({
|
||||
id: 'test-workflow-id',
|
||||
operations: [{ type: 'transferWorkflow', destinationProjectId: 'project-unknown' }],
|
||||
}, mockRepository);
|
||||
|
||||
expect(result.success).toBe(false);
|
||||
expect(result.saved).toBe(true);
|
||||
expect(result.details).toEqual({
|
||||
workflowUpdated: true,
|
||||
transferError: 'Unknown error',
|
||||
});
|
||||
});
|
||||
|
||||
it('should call transferWorkflow BEFORE activateWorkflow', async () => {
|
||||
const testWorkflow = createTestWorkflow({ active: false });
|
||||
const updatedWorkflow = { ...testWorkflow, active: false };
|
||||
const activatedWorkflow = { ...testWorkflow, active: true };
|
||||
|
||||
const callOrder: string[] = [];
|
||||
|
||||
mockApiClient.getWorkflow.mockResolvedValue(testWorkflow);
|
||||
mockDiffEngine.applyDiff.mockResolvedValue({
|
||||
success: true,
|
||||
workflow: updatedWorkflow,
|
||||
operationsApplied: 2,
|
||||
message: 'Success',
|
||||
errors: [],
|
||||
transferToProjectId: 'project-target',
|
||||
shouldActivate: true,
|
||||
});
|
||||
mockApiClient.updateWorkflow.mockResolvedValue(updatedWorkflow);
|
||||
mockApiClient.transferWorkflow.mockImplementation(async () => {
|
||||
callOrder.push('transfer');
|
||||
});
|
||||
mockApiClient.activateWorkflow = vi.fn().mockImplementation(async () => {
|
||||
callOrder.push('activate');
|
||||
return activatedWorkflow;
|
||||
});
|
||||
|
||||
const result = await handleUpdatePartialWorkflow({
|
||||
id: 'test-workflow-id',
|
||||
operations: [
|
||||
{ type: 'transferWorkflow', destinationProjectId: 'project-target' },
|
||||
{ type: 'activateWorkflow' },
|
||||
],
|
||||
}, mockRepository);
|
||||
|
||||
expect(result.success).toBe(true);
|
||||
expect(mockApiClient.transferWorkflow).toHaveBeenCalledWith('test-workflow-id', 'project-target');
|
||||
expect(mockApiClient.activateWorkflow).toHaveBeenCalledWith('test-workflow-id');
|
||||
expect(callOrder).toEqual(['transfer', 'activate']);
|
||||
});
|
||||
|
||||
it('should skip activation when transfer fails', async () => {
|
||||
const testWorkflow = createTestWorkflow({ active: false });
|
||||
const updatedWorkflow = { ...testWorkflow, active: false };
|
||||
|
||||
mockApiClient.getWorkflow.mockResolvedValue(testWorkflow);
|
||||
mockDiffEngine.applyDiff.mockResolvedValue({
|
||||
success: true,
|
||||
workflow: updatedWorkflow,
|
||||
operationsApplied: 2,
|
||||
message: 'Success',
|
||||
errors: [],
|
||||
transferToProjectId: 'project-fail',
|
||||
shouldActivate: true,
|
||||
});
|
||||
mockApiClient.updateWorkflow.mockResolvedValue(updatedWorkflow);
|
||||
mockApiClient.transferWorkflow.mockRejectedValue(new Error('Transfer denied'));
|
||||
mockApiClient.activateWorkflow = vi.fn();
|
||||
|
||||
const result = await handleUpdatePartialWorkflow({
|
||||
id: 'test-workflow-id',
|
||||
operations: [
|
||||
{ type: 'transferWorkflow', destinationProjectId: 'project-fail' },
|
||||
{ type: 'activateWorkflow' },
|
||||
],
|
||||
}, mockRepository);
|
||||
|
||||
expect(result.success).toBe(false);
|
||||
expect(result.saved).toBe(true);
|
||||
expect(result.error).toBe('Workflow updated successfully but project transfer failed');
|
||||
expect(mockApiClient.activateWorkflow).not.toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
|
||||
describe('field name normalization', () => {
|
||||
it('should normalize "name" to "nodeName" for updateNode operations', async () => {
|
||||
const testWorkflow = createTestWorkflow();
|
||||
@@ -1119,4 +1300,4 @@ describe('handlers-workflow-diff', () => {
|
||||
});
|
||||
});
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
@@ -1250,6 +1250,56 @@ describe('N8nApiClient', () => {
|
||||
});
|
||||
});
|
||||
|
||||
describe('transferWorkflow', () => {
|
||||
beforeEach(() => {
|
||||
client = new N8nApiClient(defaultConfig);
|
||||
});
|
||||
|
||||
it('should transfer workflow successfully via PUT', async () => {
|
||||
mockAxiosInstance.put.mockResolvedValue({ data: undefined });
|
||||
|
||||
await client.transferWorkflow('123', 'project-456');
|
||||
|
||||
expect(mockAxiosInstance.put).toHaveBeenCalledWith(
|
||||
'/workflows/123/transfer',
|
||||
{ destinationProjectId: 'project-456' }
|
||||
);
|
||||
});
|
||||
|
||||
it('should throw N8nNotFoundError on 404', async () => {
|
||||
const error = {
|
||||
message: 'Request failed',
|
||||
response: { status: 404, data: { message: 'Workflow not found' } }
|
||||
};
|
||||
await mockAxiosInstance.simulateError('put', error);
|
||||
|
||||
try {
|
||||
await client.transferWorkflow('123', 'project-456');
|
||||
expect.fail('Should have thrown an error');
|
||||
} catch (err) {
|
||||
expect(err).toBeInstanceOf(N8nNotFoundError);
|
||||
expect((err as N8nNotFoundError).message).toContain('not found');
|
||||
expect((err as N8nNotFoundError).statusCode).toBe(404);
|
||||
}
|
||||
});
|
||||
|
||||
it('should throw appropriate error on 403 forbidden', async () => {
|
||||
const error = {
|
||||
message: 'Request failed',
|
||||
response: { status: 403, data: { message: 'Forbidden' } }
|
||||
};
|
||||
await mockAxiosInstance.simulateError('put', error);
|
||||
|
||||
try {
|
||||
await client.transferWorkflow('123', 'project-456');
|
||||
expect.fail('Should have thrown an error');
|
||||
} catch (err) {
|
||||
expect(err).toBeInstanceOf(N8nApiError);
|
||||
expect((err as N8nApiError).statusCode).toBe(403);
|
||||
}
|
||||
});
|
||||
});
|
||||
|
||||
describe('interceptors', () => {
|
||||
let requestInterceptor: any;
|
||||
let responseInterceptor: any;
|
||||
@@ -1317,4 +1367,4 @@ describe('N8nApiClient', () => {
|
||||
expect(result.message).toBe('Bad request');
|
||||
});
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
@@ -17,7 +17,8 @@ import {
|
||||
AddTagOperation,
|
||||
RemoveTagOperation,
|
||||
CleanStaleConnectionsOperation,
|
||||
ReplaceConnectionsOperation
|
||||
ReplaceConnectionsOperation,
|
||||
TransferWorkflowOperation
|
||||
} from '@/types/workflow-diff';
|
||||
import { Workflow } from '@/types/n8n-api';
|
||||
|
||||
@@ -4989,4 +4990,151 @@ describe('WorkflowDiffEngine', () => {
|
||||
expect('nonExistent' in updatedNode).toBe(false);
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
describe('transferWorkflow operation', () => {
|
||||
it('should set transferToProjectId in result for valid transferWorkflow', async () => {
|
||||
const operation: TransferWorkflowOperation = {
|
||||
type: 'transferWorkflow',
|
||||
destinationProjectId: 'project-abc-123'
|
||||
};
|
||||
|
||||
const request: WorkflowDiffRequest = {
|
||||
id: 'test-workflow',
|
||||
operations: [operation]
|
||||
};
|
||||
|
||||
const result = await diffEngine.applyDiff(baseWorkflow, request);
|
||||
|
||||
expect(result.success).toBe(true);
|
||||
expect(result.transferToProjectId).toBe('project-abc-123');
|
||||
});
|
||||
|
||||
it('should fail validation when destinationProjectId is empty', async () => {
|
||||
const operation: TransferWorkflowOperation = {
|
||||
type: 'transferWorkflow',
|
||||
destinationProjectId: ''
|
||||
};
|
||||
|
||||
const request: WorkflowDiffRequest = {
|
||||
id: 'test-workflow',
|
||||
operations: [operation]
|
||||
};
|
||||
|
||||
const result = await diffEngine.applyDiff(baseWorkflow, request);
|
||||
|
||||
expect(result.success).toBe(false);
|
||||
expect(result.errors).toBeDefined();
|
||||
expect(result.errors![0].message).toContain('destinationProjectId');
|
||||
});
|
||||
|
||||
it('should fail validation when destinationProjectId is undefined', async () => {
|
||||
const operation = {
|
||||
type: 'transferWorkflow',
|
||||
destinationProjectId: undefined
|
||||
} as any as TransferWorkflowOperation;
|
||||
|
||||
const request: WorkflowDiffRequest = {
|
||||
id: 'test-workflow',
|
||||
operations: [operation]
|
||||
};
|
||||
|
||||
const result = await diffEngine.applyDiff(baseWorkflow, request);
|
||||
|
||||
expect(result.success).toBe(false);
|
||||
expect(result.errors).toBeDefined();
|
||||
expect(result.errors![0].message).toContain('destinationProjectId');
|
||||
});
|
||||
|
||||
it('should not include transferToProjectId when no transferWorkflow operation is present', async () => {
|
||||
const operation: UpdateNameOperation = {
|
||||
type: 'updateName',
|
||||
name: 'Renamed Workflow'
|
||||
};
|
||||
|
||||
const request: WorkflowDiffRequest = {
|
||||
id: 'test-workflow',
|
||||
operations: [operation]
|
||||
};
|
||||
|
||||
const result = await diffEngine.applyDiff(baseWorkflow, request);
|
||||
|
||||
expect(result.success).toBe(true);
|
||||
expect(result.transferToProjectId).toBeUndefined();
|
||||
});
|
||||
|
||||
it('should combine updateName and transferWorkflow operations', async () => {
|
||||
const operations: WorkflowDiffOperation[] = [
|
||||
{
|
||||
type: 'updateName',
|
||||
name: 'Transferred Workflow'
|
||||
} as UpdateNameOperation,
|
||||
{
|
||||
type: 'transferWorkflow',
|
||||
destinationProjectId: 'project-xyz-789'
|
||||
} as TransferWorkflowOperation
|
||||
];
|
||||
|
||||
const request: WorkflowDiffRequest = {
|
||||
id: 'test-workflow',
|
||||
operations
|
||||
};
|
||||
|
||||
const result = await diffEngine.applyDiff(baseWorkflow, request);
|
||||
|
||||
expect(result.success).toBe(true);
|
||||
expect(result.operationsApplied).toBe(2);
|
||||
expect(result.workflow!.name).toBe('Transferred Workflow');
|
||||
expect(result.transferToProjectId).toBe('project-xyz-789');
|
||||
});
|
||||
|
||||
it('should combine removeTag and transferWorkflow in continueOnError mode', async () => {
|
||||
const operations: WorkflowDiffOperation[] = [
|
||||
{
|
||||
type: 'removeTag',
|
||||
tag: 'non-existent-tag'
|
||||
} as RemoveTagOperation,
|
||||
{
|
||||
type: 'transferWorkflow',
|
||||
destinationProjectId: 'project-target-456'
|
||||
} as TransferWorkflowOperation
|
||||
];
|
||||
|
||||
const request: WorkflowDiffRequest = {
|
||||
id: 'test-workflow',
|
||||
operations,
|
||||
continueOnError: true
|
||||
};
|
||||
|
||||
const result = await diffEngine.applyDiff(baseWorkflow, request);
|
||||
|
||||
expect(result.success).toBe(true);
|
||||
expect(result.transferToProjectId).toBe('project-target-456');
|
||||
});
|
||||
|
||||
it('should fail entire batch in atomic mode when transferWorkflow has empty destinationProjectId alongside updateName', async () => {
|
||||
const operations: WorkflowDiffOperation[] = [
|
||||
{
|
||||
type: 'updateName',
|
||||
name: 'Should Not Apply'
|
||||
} as UpdateNameOperation,
|
||||
{
|
||||
type: 'transferWorkflow',
|
||||
destinationProjectId: ''
|
||||
} as TransferWorkflowOperation
|
||||
];
|
||||
|
||||
const request: WorkflowDiffRequest = {
|
||||
id: 'test-workflow',
|
||||
operations
|
||||
};
|
||||
|
||||
const result = await diffEngine.applyDiff(baseWorkflow, request);
|
||||
|
||||
expect(result.success).toBe(false);
|
||||
expect(result.errors).toBeDefined();
|
||||
expect(result.errors![0].message).toContain('destinationProjectId');
|
||||
// In atomic mode, the workflow should not be returned since the batch failed
|
||||
expect(result.workflow).toBeUndefined();
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user