From 881a14fa4d29c45f3071f9ec2d8d1782937d7b26 Mon Sep 17 00:00:00 2001 From: James Gee <1285296+geemanjs@users.noreply.github.com> Date: Wed, 5 Aug 2026 17:37:11 +0200 Subject: [PATCH] feat(core): Add projectConflictPolicy to package import (#35488) --- .../import-package-request.dto.test.ts | 42 +++++- .../packages/import-package-request.dto.ts | 2 + packages/@n8n/cli/docs/commands/package.md | 2 + .../cli/src/__tests__/package-error.test.ts | 13 ++ .../cli/src/__tests__/package-import.test.ts | 15 +++ packages/@n8n/cli/src/client.ts | 1 + .../@n8n/cli/src/commands/package/import.ts | 7 + .../cli/src/commands/package/package-error.ts | 4 + .../__tests__/fixtures/import-request.ts | 32 +++++ .../import-data-tables.integration.test.ts | 22 +--- .../import-folders.integration.test.ts | 38 ++---- .../import-package.integration.test.ts | 69 +--------- .../import-projects.integration.test.ts | 123 ++++++++++++++---- .../import-subworkflows.integration.test.ts | 35 +---- ...ort-workflow-with-tags.integration.test.ts | 21 +-- ...orkflow-with-variables.integration.test.ts | 22 +--- .../__tests__/import-blocked.error.test.ts | 12 ++ .../engine/import-blocked.error.ts | 1 + .../engine/project-package-importer.ts | 24 +++- .../__tests__/project-conflict-policy.test.ts | 15 +++ .../__tests__/project-importer.test.ts | 93 ++++++++++++- .../project/project-conflict-policy.ts | 22 ++++ .../entities/project/project-import.types.ts | 21 ++- .../entities/project/project-importer.ts | 123 +++++++++++++----- .../n8n-packages/n8n-packages.types.ts | 32 ++++- .../n8n-packages/n8n-packages.handler.ts | 1 + .../spec/paths/n8n-packages.import.yml | 36 ++++- .../spec/schemas/importBlockingIssue.yml | 23 ++++ 28 files changed, 609 insertions(+), 242 deletions(-) create mode 100644 packages/cli/src/modules/n8n-packages/__tests__/fixtures/import-request.ts create mode 100644 packages/cli/src/modules/n8n-packages/entities/project/__tests__/project-conflict-policy.test.ts create mode 100644 packages/cli/src/modules/n8n-packages/entities/project/project-conflict-policy.ts diff --git a/packages/@n8n/api-types/src/dto/packages/__tests__/import-package-request.dto.test.ts b/packages/@n8n/api-types/src/dto/packages/__tests__/import-package-request.dto.test.ts index 5e0b74140c1..abce132541f 100644 --- a/packages/@n8n/api-types/src/dto/packages/__tests__/import-package-request.dto.test.ts +++ b/packages/@n8n/api-types/src/dto/packages/__tests__/import-package-request.dto.test.ts @@ -16,6 +16,7 @@ describe('ImportPackageRequestDto', () => { workflowPublishingPolicy: 'preserve-published-state', workflowIdPolicy: 'source', missingNodeTypeMode: 'fail', + projectConflictPolicy: 'merge', folderConflictPolicy: 'merge', dataTableMatchingMode: 'by-id', dataTableMissingMode: 'create', @@ -44,6 +45,7 @@ describe('ImportPackageRequestDto', () => { workflowPublishingPolicy: 'preserve-published-state', workflowIdPolicy: 'source', missingNodeTypeMode: 'fail', + projectConflictPolicy: 'merge', folderConflictPolicy: 'merge', dataTableMatchingMode: 'by-id', dataTableMissingMode: 'create', @@ -74,6 +76,7 @@ describe('ImportPackageRequestDto', () => { workflowPublishingPolicy: 'preserve-published-state', workflowIdPolicy: 'source', missingNodeTypeMode: 'fail', + projectConflictPolicy: 'merge', folderConflictPolicy: 'merge', dataTableMatchingMode: 'by-id', dataTableMissingMode: 'create', @@ -103,6 +106,7 @@ describe('ImportPackageRequestDto', () => { workflowPublishingPolicy: 'preserve-published-state', workflowIdPolicy: 'source', missingNodeTypeMode: 'fail', + projectConflictPolicy: 'merge', folderConflictPolicy: 'merge', dataTableMatchingMode: 'by-id', dataTableMissingMode: 'create', @@ -299,6 +303,42 @@ describe('ImportPackageRequestDto', () => { }); }); + // Both conflict policies behave identically at the DTO layer, so one table covers them. + describe.each([ + { field: 'projectConflictPolicy', values: ['merge', 'fail', 'overwrite'], expected: 'merge' }, + { field: 'folderConflictPolicy', values: ['merge', 'fail'], expected: 'merge' }, + ] as const)('$field', ({ field, values, expected }) => { + it(`defaults to ${expected ?? 'undefined'} when omitted`, () => { + const result = ImportPackageRequestDto.safeParse({ workflowConflictPolicy: 'fail' }); + expect(result.success).toBe(true); + if (result.success) expect(result.data[field]).toBe(expected); + }); + + it('accepts every supported value', () => { + for (const value of values) { + const result = ImportPackageRequestDto.safeParse({ + workflowConflictPolicy: 'fail', + [field]: value, + }); + expect(result.success).toBe(true); + if (result.success) expect(result.data[field]).toBe(value); + } + }); + + it('rejects an unsupported value', () => { + expect( + ImportPackageRequestDto.safeParse({ + workflowConflictPolicy: 'fail', + [field]: 'not-a-policy', + }).success, + ).toBe(false); + }); + + it('is accepted as a multipart form field', () => { + expect(IMPORT_PACKAGE_REQUEST_FORM_FIELDS).toContain(field); + }); + }); + describe('missingNodeTypeMode', () => { it('defaults to "fail" when omitted', () => { const result = ImportPackageRequestDto.safeParse({ workflowConflictPolicy: 'fail' }); @@ -400,7 +440,7 @@ describe('ImportPackageRequestDto', () => { { field: 'workflowPublishingPolicy', expected: 'preserve-published-state' }, { field: 'workflowIdPolicy', expected: 'source' }, { field: 'missingNodeTypeMode', expected: 'fail' }, - { field: 'folderConflictPolicy', expected: 'merge' }, + { field: 'projectConflictPolicy', expected: 'merge' }, { field: 'dataTableMatchingMode', expected: 'by-id' }, { field: 'dataTableMissingMode', expected: 'create' }, { field: 'dataTableSchemaConflictPolicy', expected: 'keep-existing' }, diff --git a/packages/@n8n/api-types/src/dto/packages/import-package-request.dto.ts b/packages/@n8n/api-types/src/dto/packages/import-package-request.dto.ts index b9f85e62507..32a62592e9f 100644 --- a/packages/@n8n/api-types/src/dto/packages/import-package-request.dto.ts +++ b/packages/@n8n/api-types/src/dto/packages/import-package-request.dto.ts @@ -13,6 +13,7 @@ export const IMPORT_PACKAGE_REQUEST_FORM_FIELDS = [ 'workflowPublishingPolicy', 'workflowIdPolicy', 'missingNodeTypeMode', + 'projectConflictPolicy', 'folderConflictPolicy', 'dataTableMatchingMode', 'dataTableMissingMode', @@ -101,6 +102,7 @@ export class ImportPackageRequestDto extends Z.class({ ), workflowIdPolicy: optionalEnum(['new', 'source'], 'source'), missingNodeTypeMode: optionalEnum(['fail', 'import-anyway'], 'fail'), + projectConflictPolicy: optionalEnum(['merge', 'fail', 'overwrite'], 'merge'), folderConflictPolicy: optionalEnum(['merge', 'fail'], 'merge'), dataTableMatchingMode: optionalEnum(['by-id'], 'by-id'), dataTableMissingMode: optionalEnum(['create', 'must-preexist', 'do-nothing'], 'create'), diff --git a/packages/@n8n/cli/docs/commands/package.md b/packages/@n8n/cli/docs/commands/package.md index 8bfbade0487..268add75135 100644 --- a/packages/@n8n/cli/docs/commands/package.md +++ b/packages/@n8n/cli/docs/commands/package.md @@ -63,6 +63,7 @@ n8n-cli package import --file=export.n8np --workflow-conflict-policy=fail --bind | `--workflow-publishing-policy` | Whether imported workflows end up published. `preserve-published-state` (instance default) never publishes drafts — an updated workflow is republished only when it was already published and the package workflow is published too; `match-source` follows the package workflow's published flag; `publish-all` publishes every imported workflow; `unpublish-all` leaves new workflows unpublished and unpublishes updated ones. | | `--workflow-id-policy` | Whether imported workflows keep their source ID (`source`) or receive a new one (`new`). | | `--missing-node-type-mode` | What to do when a workflow uses a node type — or a version of a node type — this instance does not have. `fail` (instance default) rejects the import before anything is written, listing every missing node type and the workflows that use it; `import-anyway` imports the package, but the affected workflows are never published by the import, regardless of the publishing policy. | +| `--project-conflict-policy` | What to do when a project in the package already exists on the instance, matched by ID (project packages only): `merge` (instance default) is purely additive — the existing project's name, description, icon and custom span attributes are left alone and the package's contents are added alongside; `overwrite` replaces those details with the package's, and a detail the package omits is left as it is, not cleared; `fail` rejects the import before anything is written. | | `--folder-conflict-policy` | What to do when a package folder already exists in the target project: `merge` (default, reuse the existing folder and merge the package's children into it) or `fail`. Requires a folders-enabled license when the package contains folders. | | `--credential-matching-mode` | How credential references are matched on the target instance: `id-only` (default, match by id), `name-and-type` (match by exact name and type), or `type-only` (match by type). For `name-and-type` and `type-only`, candidates are ranked by scope — owned by the target project, then shared into it, then global — and ties within a scope use the most recently updated credential. | | `--credential-missing-mode` | What to do when a referenced credential cannot be resolved. `create-stub` (instance default) creates empty placeholder credentials in the target project; `must-preexist` requires every referenced credential to already exist. | @@ -89,6 +90,7 @@ When the import is blocked, the command exits non-zero and lists the blocking issues. Examples: - a workflow conflict under `--workflow-conflict-policy=fail` +- a project that already exists under `--project-conflict-policy=fail` - an unresolved credential under `--credential-missing-mode=must-preexist` - a schema-incompatible data table - a workflow using a node type this instance does not have under diff --git a/packages/@n8n/cli/src/__tests__/package-error.test.ts b/packages/@n8n/cli/src/__tests__/package-error.test.ts index b1f2b1fea08..98a24733050 100644 --- a/packages/@n8n/cli/src/__tests__/package-error.test.ts +++ b/packages/@n8n/cli/src/__tests__/package-error.test.ts @@ -29,6 +29,19 @@ describe('toPackagesError', () => { expect(hint).toContain('e1'); }); + it('lists project-conflict issues for a 409', () => { + const result = toPackagesError( + new ApiError(409, 'Import blocked', undefined, { + issues: [ + { type: 'project-conflict', kind: 'fail-policy', sourceProjectId: 'P1', name: 'brie' }, + ], + }), + ); + + const hint = (result as ApiError).hint ?? ''; + expect(hint).toContain('project "brie" (source P1) already exists on this instance'); + }); + it('lists credential-unresolved issues for a 422', () => { const result = toPackagesError( new ApiError(422, 'Import blocked', undefined, { diff --git a/packages/@n8n/cli/src/__tests__/package-import.test.ts b/packages/@n8n/cli/src/__tests__/package-import.test.ts index 8d2457ba94d..5a795d1bbee 100644 --- a/packages/@n8n/cli/src/__tests__/package-import.test.ts +++ b/packages/@n8n/cli/src/__tests__/package-import.test.ts @@ -19,6 +19,7 @@ interface ImportFlags { workflowPublishingPolicy?: string; workflowIdPolicy?: string; missingNodeTypeMode?: string; + projectConflictPolicy?: string; folderConflictPolicy?: string; credentialMatchingMode?: string; credentialMissingMode?: string; @@ -69,6 +70,7 @@ describe('package import command', () => { workflowPublishingPolicy: 'publish-all', workflowIdPolicy: 'new', missingNodeTypeMode: 'import-anyway', + projectConflictPolicy: 'overwrite', folderConflictPolicy: 'merge', credentialMatchingMode: 'id-only', credentialMissingMode: 'create-stub', @@ -97,6 +99,7 @@ describe('package import command', () => { workflowPublishingPolicy: 'publish-all', workflowIdPolicy: 'new', missingNodeTypeMode: 'import-anyway', + projectConflictPolicy: 'overwrite', folderConflictPolicy: 'merge', credentialMatchingMode: 'id-only', credentialMissingMode: 'create-stub', @@ -203,6 +206,18 @@ describe('package import command', () => { ); }); + it('resolves the --project-conflict-policy alias', async () => { + const importPackage = await runWithArgv([ + '--file=/tmp/export.n8np', + '--project-conflict-policy=merge', + ]); + + expect(importPackage).toHaveBeenCalledWith( + expect.anything(), + expect.objectContaining({ projectConflictPolicy: 'merge' }), + ); + }); + it('rejects the removed --conflict-policy flag', async () => { const config = await Config.load(packageRoot); const command = new PackageImport( diff --git a/packages/@n8n/cli/src/client.ts b/packages/@n8n/cli/src/client.ts index 76f270b7b75..ada4ac81b04 100644 --- a/packages/@n8n/cli/src/client.ts +++ b/packages/@n8n/cli/src/client.ts @@ -19,6 +19,7 @@ export interface ImportPackageFields { workflowPublishingPolicy?: string; workflowIdPolicy?: string; missingNodeTypeMode?: string; + projectConflictPolicy?: string; folderConflictPolicy?: string; dataTableMatchingMode?: string; dataTableMissingMode?: string; diff --git a/packages/@n8n/cli/src/commands/package/import.ts b/packages/@n8n/cli/src/commands/package/import.ts index 47ecb9d062c..4e1eccb83e5 100644 --- a/packages/@n8n/cli/src/commands/package/import.ts +++ b/packages/@n8n/cli/src/commands/package/import.ts @@ -50,6 +50,12 @@ export default class PackageImport extends BaseCommand { options: ['fail', 'import-anyway'], aliases: ['missing-node-type-mode'], }), + projectConflictPolicy: Flags.string({ + description: + "What to do when a project in the package already exists on the instance (default on the instance: merge). merge keeps the existing project's details and adds the package's contents alongside; overwrite replaces those details with the package's; fail rejects the import. Project packages only", + options: ['merge', 'fail', 'overwrite'], + aliases: ['project-conflict-policy'], + }), folderConflictPolicy: Flags.string({ description: 'What to do when a package folder already exists in the target project', options: ['merge', 'fail'], @@ -140,6 +146,7 @@ export default class PackageImport extends BaseCommand { workflowPublishingPolicy: flags.workflowPublishingPolicy, workflowIdPolicy: flags.workflowIdPolicy, missingNodeTypeMode: flags.missingNodeTypeMode, + projectConflictPolicy: flags.projectConflictPolicy, folderConflictPolicy: flags.folderConflictPolicy, credentialMatchingMode: flags.credentialMatchingMode, credentialMissingMode: flags.credentialMissingMode, diff --git a/packages/@n8n/cli/src/commands/package/package-error.ts b/packages/@n8n/cli/src/commands/package/package-error.ts index 390128625be..003866b3d65 100644 --- a/packages/@n8n/cli/src/commands/package/package-error.ts +++ b/packages/@n8n/cli/src/commands/package/package-error.ts @@ -7,6 +7,7 @@ type BlockingIssue = existingWorkflowId: string; name: string; } + | { type: 'project-conflict'; sourceProjectId: string; name: string } | { type: 'credential-unresolved'; kind: string; sourceId: string; usedByWorkflows: string[] } | { type: 'variable-unresolved'; name: string; usedByWorkflows: string[] } | { type: 'variable-conflict'; name: string; projectId?: string; usedByWorkflows: string[] } @@ -39,6 +40,9 @@ function formatIssue(issue: unknown): string { if (it.type === 'workflow-conflict') { return `workflow "${it.name}" (source ${it.sourceWorkflowId}) already exists as ${it.existingWorkflowId}`; } + if (it.type === 'project-conflict') { + return `project "${it.name}" (source ${it.sourceProjectId}) already exists on this instance`; + } if (it.type === 'credential-unresolved') { const usedBy = Array.isArray(it.usedByWorkflows) ? it.usedByWorkflows.join(', ') : ''; return `credential ${it.sourceId} unresolved (${it.kind}), used by workflow(s) ${usedBy}`; diff --git a/packages/cli/src/modules/n8n-packages/__tests__/fixtures/import-request.ts b/packages/cli/src/modules/n8n-packages/__tests__/fixtures/import-request.ts new file mode 100644 index 00000000000..3a90b7bb7ab --- /dev/null +++ b/packages/cli/src/modules/n8n-packages/__tests__/fixtures/import-request.ts @@ -0,0 +1,32 @@ +import type { ImportPackageRequest } from '../../n8n-packages.types'; + +/** + * Every import option a request must carry, so a test states only the ones it exercises. Adding an + * option to `ImportPackageRequest` means editing this once rather than every suite's helper. + * + * `variableParentPolicy` is deliberately absent: project packages reject it outright, so it has to + * stay opt-in per suite. + */ +const DEFAULTS = { + credentialMatchingMode: 'id-only', + credentialMissingMode: 'must-preexist', + workflowConflictPolicy: 'fail', + workflowPublishingPolicy: 'preserve-published-state', + workflowIdPolicy: 'new', + missingNodeTypeMode: 'fail', + projectConflictPolicy: 'merge', + folderConflictPolicy: 'merge', + dataTableMatchingMode: 'by-id', + dataTableMissingMode: 'create', + dataTableSchemaConflictPolicy: 'keep-existing', + variableMissingMode: 'do-nothing', + variableConflictPolicy: 'keep-existing', + tagMissingMode: 'create', + tagConflictPolicy: 'skip', +} as const satisfies Partial; + +export function importPackageRequest( + overrides: Pick & Partial, +): ImportPackageRequest { + return { ...DEFAULTS, ...overrides }; +} diff --git a/packages/cli/src/modules/n8n-packages/__tests__/import-data-tables.integration.test.ts b/packages/cli/src/modules/n8n-packages/__tests__/import-data-tables.integration.test.ts index e91689b7277..fc94dc7ee7f 100644 --- a/packages/cli/src/modules/n8n-packages/__tests__/import-data-tables.integration.test.ts +++ b/packages/cli/src/modules/n8n-packages/__tests__/import-data-tables.integration.test.ts @@ -20,6 +20,7 @@ import { LicenseMocker } from '@test-integration/license'; import { initNodeTypes } from '@test-integration/utils'; import { N8nPackagesService } from '../n8n-packages.service'; +import { importPackageRequest } from './fixtures/import-request'; import type { ImportPackageRequest } from '../n8n-packages.types'; import type { PackageDataTableRequirement } from '../spec/requirements.schema'; import type { SerializedDataTable } from '../spec/serialized/data-table.schema'; @@ -63,24 +64,9 @@ type ImportParams = { user: User; projectId: string; packageBuffer: Buffer } & P >; async function importPackage(params: ImportParams) { - return await service.importPackage({ - credentialMatchingMode: 'id-only', - credentialMissingMode: 'must-preexist', - workflowConflictPolicy: 'fail', - workflowPublishingPolicy: 'preserve-published-state', - workflowIdPolicy: 'new', - missingNodeTypeMode: 'fail', - folderConflictPolicy: 'merge', - dataTableMatchingMode: 'by-id', - dataTableMissingMode: 'create', - dataTableSchemaConflictPolicy: 'keep-existing', - variableMissingMode: 'do-nothing', - variableConflictPolicy: 'keep-existing', - variableParentPolicy: 'project', - tagMissingMode: 'create', - tagConflictPolicy: 'skip', - ...params, - }); + return await service.importPackage( + importPackageRequest({ variableParentPolicy: 'project', ...params }), + ); } /** A package holding `tables` plus one workflow per table referencing it. */ diff --git a/packages/cli/src/modules/n8n-packages/__tests__/import-folders.integration.test.ts b/packages/cli/src/modules/n8n-packages/__tests__/import-folders.integration.test.ts index 16f8050677a..748e957dc67 100644 --- a/packages/cli/src/modules/n8n-packages/__tests__/import-folders.integration.test.ts +++ b/packages/cli/src/modules/n8n-packages/__tests__/import-folders.integration.test.ts @@ -13,7 +13,8 @@ import { LicenseMocker } from '@test-integration/license'; import { initNodeTypes } from '@test-integration/utils'; import { N8nPackagesService } from '../n8n-packages.service'; -import type { FolderConflictPolicy, ImportPackageRequest } from '../n8n-packages.types'; +import { importPackageRequest } from './fixtures/import-request'; +import type { FolderConflictPolicy } from '../n8n-packages.types'; import { buildEntityPackageBuffer, credentialRequirementsFromWorkflows, @@ -32,29 +33,18 @@ type FolderImportParams = { }; async function importFolders(params: FolderImportParams) { - const request: ImportPackageRequest = { - user: params.user, - projectId: params.projectId, - folderId: params.folderId, - packageBuffer: params.packageBuffer, - apiKeyScopes: params.apiKeyScopes, - credentialMatchingMode: 'id-only', - credentialMissingMode: 'must-preexist', - workflowConflictPolicy: 'new-version', - workflowPublishingPolicy: 'preserve-published-state', - workflowIdPolicy: 'new', - missingNodeTypeMode: 'fail', - folderConflictPolicy: params.folderConflictPolicy ?? 'merge', - dataTableMatchingMode: 'by-id', - dataTableMissingMode: 'create', - dataTableSchemaConflictPolicy: 'keep-existing', - variableMissingMode: 'do-nothing', - variableConflictPolicy: 'keep-existing', - variableParentPolicy: 'project', - tagMissingMode: 'create', - tagConflictPolicy: 'skip', - }; - return await Container.get(N8nPackagesService).importPackage(request); + return await Container.get(N8nPackagesService).importPackage( + importPackageRequest({ + user: params.user, + projectId: params.projectId, + folderId: params.folderId, + packageBuffer: params.packageBuffer, + apiKeyScopes: params.apiKeyScopes, + workflowConflictPolicy: 'new-version', + folderConflictPolicy: params.folderConflictPolicy ?? 'merge', + variableParentPolicy: 'project', + }), + ); } const licenseMocker = new LicenseMocker(); diff --git a/packages/cli/src/modules/n8n-packages/__tests__/import-package.integration.test.ts b/packages/cli/src/modules/n8n-packages/__tests__/import-package.integration.test.ts index f8e49f29491..6a7a04376c6 100644 --- a/packages/cli/src/modules/n8n-packages/__tests__/import-package.integration.test.ts +++ b/packages/cli/src/modules/n8n-packages/__tests__/import-package.integration.test.ts @@ -36,12 +36,8 @@ import { initNodeTypes } from '@test-integration/utils'; import { TarPackageWriter } from '../io/tar/tar-package-writer'; import { N8nPackagesService } from '../n8n-packages.service'; +import { importPackageRequest } from './fixtures/import-request'; import { - FolderConflictPolicy, - DataTableMatchingMode, - DataTableMissingMode, - DataTableSchemaConflictPolicy, - MissingNodeTypeMode, WorkflowConflictPolicy, WorkflowIdPolicy, WorkflowPublishingPolicy, @@ -59,66 +55,13 @@ import { import { streamToBuffer } from './utils/tar-support'; import type { SerializedWorkflow } from '../spec/serialized/workflow.schema'; -type ImportPackageParams = Omit< - ImportPackageRequest, - | 'credentialMatchingMode' - | 'credentialMissingMode' - | 'bindings' - | 'workflowConflictPolicy' - | 'workflowPublishingPolicy' - | 'workflowIdPolicy' - | 'missingNodeTypeMode' - | 'folderConflictPolicy' - | 'dataTableMatchingMode' - | 'dataTableMissingMode' - | 'dataTableSchemaConflictPolicy' - | 'variableMissingMode' - | 'variableConflictPolicy' - | 'variableParentPolicy' - | 'tagMissingMode' - | 'tagConflictPolicy' -> & - Partial< - Pick< - ImportPackageRequest, - | 'credentialMatchingMode' - | 'credentialMissingMode' - | 'bindings' - | 'workflowConflictPolicy' - | 'workflowPublishingPolicy' - | 'workflowIdPolicy' - | 'missingNodeTypeMode' - | 'folderConflictPolicy' - | 'dataTableMatchingMode' - | 'dataTableMissingMode' - | 'dataTableSchemaConflictPolicy' - | 'variableMissingMode' - | 'variableConflictPolicy' - | 'variableParentPolicy' - | 'tagMissingMode' - | 'tagConflictPolicy' - > - >; +type ImportPackageParams = Pick & + Partial; async function importPackage(params: ImportPackageParams) { - return await Container.get(N8nPackagesService).importPackage({ - credentialMatchingMode: 'id-only', - credentialMissingMode: 'must-preexist', - workflowConflictPolicy: WorkflowConflictPolicy.Fail, - workflowPublishingPolicy: WorkflowPublishingPolicy.PreservePublishedState, - workflowIdPolicy: WorkflowIdPolicy.New, - missingNodeTypeMode: MissingNodeTypeMode.Fail, - folderConflictPolicy: FolderConflictPolicy.Merge, - dataTableMatchingMode: DataTableMatchingMode.ById, - dataTableMissingMode: DataTableMissingMode.Create, - dataTableSchemaConflictPolicy: DataTableSchemaConflictPolicy.KeepExisting, - variableMissingMode: 'do-nothing', - variableConflictPolicy: 'keep-existing', - variableParentPolicy: 'project', - tagMissingMode: 'create', - tagConflictPolicy: 'skip', - ...params, - }); + return await Container.get(N8nPackagesService).importPackage( + importPackageRequest({ variableParentPolicy: 'project', ...params }), + ); } /** diff --git a/packages/cli/src/modules/n8n-packages/__tests__/import-projects.integration.test.ts b/packages/cli/src/modules/n8n-packages/__tests__/import-projects.integration.test.ts index 642cbd691b2..9d03db63a32 100644 --- a/packages/cli/src/modules/n8n-packages/__tests__/import-projects.integration.test.ts +++ b/packages/cli/src/modules/n8n-packages/__tests__/import-projects.integration.test.ts @@ -34,6 +34,7 @@ import { LicenseMocker } from '@test-integration/license'; import { initNodeTypes } from '@test-integration/utils'; import { N8nPackagesService } from '../n8n-packages.service'; +import { importPackageRequest } from './fixtures/import-request'; import type { ImportPackageRequest } from '../n8n-packages.types'; import { buildEntityPackageBuffer, @@ -55,27 +56,15 @@ async function importProjects( apiKeyScopes?: string[], overrides?: Partial, ) { - const request: ImportPackageRequest = { - user, - packageBuffer, - apiKeyScopes, - credentialMatchingMode: 'id-only', - credentialMissingMode: 'must-preexist', - workflowConflictPolicy: 'new-version', - workflowPublishingPolicy: 'preserve-published-state', - workflowIdPolicy: 'new', - missingNodeTypeMode: 'fail', - folderConflictPolicy: 'merge', - dataTableMatchingMode: 'by-id', - dataTableMissingMode: 'create', - dataTableSchemaConflictPolicy: 'keep-existing', - variableMissingMode: 'do-nothing', - variableConflictPolicy: 'keep-existing', - tagMissingMode: 'create', - tagConflictPolicy: 'skip', - ...overrides, - }; - return await Container.get(N8nPackagesService).importPackage(request); + return await Container.get(N8nPackagesService).importPackage( + importPackageRequest({ + user, + packageBuffer, + apiKeyScopes, + workflowConflictPolicy: 'new-version', + ...overrides, + }), + ); } const licenseMocker = new LicenseMocker(); @@ -212,6 +201,8 @@ describe('project shell import', () => { }, ], }), + undefined, + { projectConflictPolicy: 'overwrite' }, ); expect(result.projects).toEqual([ @@ -221,6 +212,92 @@ describe('project shell import', () => { expect((await findProject('P1'))?.name).toBe('brie renamed'); }); + describe('projectConflictPolicy', () => { + /** A renamed P1 carrying a workflow, plus a second project that does not exist on the target. */ + const reimportPackage = async (extraProject = false) => + await buildEntityPackageBuffer({ + projects: [ + { + target: 'projects/brie', + project: serializedProject({ + id: 'P1', + name: 'brie renamed', + description: 'from package', + }), + }, + ...(extraProject + ? [ + { + target: 'projects/stilton', + project: serializedProject({ id: 'P2', name: 'stilton' }), + }, + ] + : []), + ], + workflows: [ + { + target: 'projects/brie/workflows/wf', + workflow: serializedWorkflow({ id: 'WF', name: 'wf' }), + }, + ], + }); + + beforeEach(async () => { + await importProjects( + owner, + await buildEntityPackageBuffer({ + projects: [ + { target: 'projects/brie', project: serializedProject({ id: 'P1', name: 'brie' }) }, + ], + }), + ); + }); + + it('leaves the existing project details untouched under merge, still importing its contents', async () => { + const result = await importProjects(owner, await reimportPackage(), undefined, { + projectConflictPolicy: 'merge', + }); + + expect(result.projects).toEqual([ + { sourceProjectId: 'P1', localId: 'P1', name: 'brie', status: 'skipped' }, + ]); + const project = await findProject('P1'); + expect(project?.name).toBe('brie'); + expect(project?.description).toBeNull(); + expect(result.workflows[0]).toMatchObject({ projectId: 'P1', status: 'created' }); + }); + + it('refuses the whole package under fail, writing nothing for any project', async () => { + const blocked = importProjects(owner, await reimportPackage(true), undefined, { + projectConflictPolicy: 'fail', + }); + + await expect(blocked).rejects.toBeInstanceOf(ConflictError); + // Only the conflict is reported: the refused project decides the import before any + // project's contents are planned, so P2 is never created either. + await expect(blocked).rejects.toMatchObject({ + meta: { + issues: [ + { + type: 'project-conflict', + kind: 'fail-policy', + sourceProjectId: 'P1', + name: 'brie renamed', + }, + ], + }, + }); + + const project = await findProject('P1'); + expect(project?.name).toBe('brie'); + expect(project?.description).toBeNull(); + expect(await findProject('P2')).toBeNull(); + // Counted across every project: `workflowIdPolicy: 'new'` mints a fresh id, so counting + // the package's source id would pass even if the workflow had been written. + expect(await Container.get(WorkflowRepository).count()).toBe(0); + }); + }); + it('rejects a project package whose manifest project id disagrees with its project.json', async () => { const packageBuffer = await buildEntityPackageBuffer({ projects: [ @@ -1745,7 +1822,9 @@ describe('project custom span attributes (LIGO-862)', () => { ], }); - const result = await importProjects(owner, packageBuffer); + const result = await importProjects(owner, packageBuffer, undefined, { + projectConflictPolicy: 'overwrite', + }); expect(result.projects).toEqual([ { sourceProjectId: project.id, localId: project.id, name: project.name, status: 'updated' }, diff --git a/packages/cli/src/modules/n8n-packages/__tests__/import-subworkflows.integration.test.ts b/packages/cli/src/modules/n8n-packages/__tests__/import-subworkflows.integration.test.ts index d637ef8a589..244ed637e01 100644 --- a/packages/cli/src/modules/n8n-packages/__tests__/import-subworkflows.integration.test.ts +++ b/packages/cli/src/modules/n8n-packages/__tests__/import-subworkflows.integration.test.ts @@ -9,21 +9,8 @@ import { LicenseMocker } from '@test-integration/license'; import { initNodeTypes } from '@test-integration/utils'; import { N8nPackagesService } from '../n8n-packages.service'; -import { - DataTableMatchingMode, - DataTableMissingMode, - DataTableSchemaConflictPolicy, - FolderConflictPolicy, - MissingNodeTypeMode, - TagConflictPolicy, - TagMissingMode, - VariableConflictPolicy, - VariableMissingMode, - WorkflowConflictPolicy, - WorkflowIdPolicy, - WorkflowPublishingPolicy, - type ImportPackageRequest, -} from '../n8n-packages.types'; +import { importPackageRequest } from './fixtures/import-request'; +import { WorkflowIdPolicy, type ImportPackageRequest } from '../n8n-packages.types'; import { buildImportPackageBuffer, serializedWorkflow, @@ -37,23 +24,7 @@ type ImportPackageParams = Pick Partial>; async function importPackage(params: ImportPackageParams) { - return await Container.get(N8nPackagesService).importPackage({ - credentialMatchingMode: 'id-only', - credentialMissingMode: 'must-preexist', - workflowConflictPolicy: WorkflowConflictPolicy.Fail, - workflowPublishingPolicy: WorkflowPublishingPolicy.PreservePublishedState, - workflowIdPolicy: WorkflowIdPolicy.New, - missingNodeTypeMode: MissingNodeTypeMode.Fail, - folderConflictPolicy: FolderConflictPolicy.Merge, - dataTableMatchingMode: DataTableMatchingMode.ById, - dataTableMissingMode: DataTableMissingMode.Create, - dataTableSchemaConflictPolicy: DataTableSchemaConflictPolicy.KeepExisting, - variableMissingMode: VariableMissingMode.DoNothing, - variableConflictPolicy: VariableConflictPolicy.KeepExisting, - tagMissingMode: TagMissingMode.Create, - tagConflictPolicy: TagConflictPolicy.Skip, - ...params, - }); + return await Container.get(N8nPackagesService).importPackage(importPackageRequest(params)); } /** Builds a package where `workflows` carry Execute Sub-workflow refs + the derived requirements. */ diff --git a/packages/cli/src/modules/n8n-packages/__tests__/import-workflow-with-tags.integration.test.ts b/packages/cli/src/modules/n8n-packages/__tests__/import-workflow-with-tags.integration.test.ts index 3fc89650847..42fac879db7 100644 --- a/packages/cli/src/modules/n8n-packages/__tests__/import-workflow-with-tags.integration.test.ts +++ b/packages/cli/src/modules/n8n-packages/__tests__/import-workflow-with-tags.integration.test.ts @@ -21,6 +21,7 @@ import { initNodeTypes } from '@test-integration/utils'; import { N8nPackagesService } from '../n8n-packages.service'; import type { ImportPackageRequest } from '../n8n-packages.types'; import { buildEntityPackageBuffer, serializedWorkflow } from './fixtures/package-fixtures'; +import { importPackageRequest } from './fixtures/import-request'; import { streamToBuffer } from './utils/tar-support'; let service: N8nPackagesService; @@ -61,23 +62,9 @@ type ImportParams = { user: User; projectId?: string; packageBuffer: Buffer } & >; async function importPackage(params: ImportParams) { - return await service.importPackage({ - credentialMatchingMode: 'id-only', - credentialMissingMode: 'must-preexist', - workflowConflictPolicy: 'fail', - workflowPublishingPolicy: 'preserve-published-state', - workflowIdPolicy: 'new', - folderConflictPolicy: 'merge', - dataTableMatchingMode: 'by-id', - dataTableMissingMode: 'create', - dataTableSchemaConflictPolicy: 'keep-existing', - variableMissingMode: 'do-nothing', - variableConflictPolicy: 'keep-existing', - missingNodeTypeMode: 'fail', - tagMissingMode: 'create', - tagConflictPolicy: 'skip', - ...params, - }); + return await service.importPackage( + importPackageRequest({ variableParentPolicy: 'project', ...params }), + ); } async function exportWorkflowPackage( diff --git a/packages/cli/src/modules/n8n-packages/__tests__/import-workflow-with-variables.integration.test.ts b/packages/cli/src/modules/n8n-packages/__tests__/import-workflow-with-variables.integration.test.ts index 1b6059c434f..e31ff545dcf 100644 --- a/packages/cli/src/modules/n8n-packages/__tests__/import-workflow-with-variables.integration.test.ts +++ b/packages/cli/src/modules/n8n-packages/__tests__/import-workflow-with-variables.integration.test.ts @@ -21,6 +21,7 @@ import { LicenseMocker } from '@test-integration/license'; import { initNodeTypes } from '@test-integration/utils'; import { N8nPackagesService } from '../n8n-packages.service'; +import { importPackageRequest } from './fixtures/import-request'; import type { ImportPackageRequest } from '../n8n-packages.types'; import { streamToBuffer } from './utils/tar-support'; import { buildWorkflowReferencingVariables } from './utils/test-builders'; @@ -63,24 +64,9 @@ type ImportParams = { user: User; projectId?: string; packageBuffer: Buffer } & >; async function importPackage(params: ImportParams) { - return await service.importPackage({ - credentialMatchingMode: 'id-only', - credentialMissingMode: 'must-preexist', - workflowConflictPolicy: 'fail', - workflowPublishingPolicy: 'preserve-published-state', - workflowIdPolicy: 'new', - folderConflictPolicy: 'merge', - dataTableMatchingMode: 'by-id', - dataTableMissingMode: 'create', - dataTableSchemaConflictPolicy: 'keep-existing', - variableMissingMode: 'do-nothing', - variableConflictPolicy: 'keep-existing', - variableParentPolicy: 'project', - missingNodeTypeMode: 'fail', - tagMissingMode: 'create', - tagConflictPolicy: 'skip', - ...params, - }); + return await service.importPackage( + importPackageRequest({ variableParentPolicy: 'project', ...params }), + ); } async function exportWorkflowPackage( diff --git a/packages/cli/src/modules/n8n-packages/engine/__tests__/import-blocked.error.test.ts b/packages/cli/src/modules/n8n-packages/engine/__tests__/import-blocked.error.test.ts index 08077b785c5..23677599a2d 100644 --- a/packages/cli/src/modules/n8n-packages/engine/__tests__/import-blocked.error.test.ts +++ b/packages/cli/src/modules/n8n-packages/engine/__tests__/import-blocked.error.test.ts @@ -13,6 +13,13 @@ const folderConflict: BlockingIssue = { expectedParentFolderId: 'anchor', }; +const projectConflict: BlockingIssue = { + type: 'project-conflict', + kind: 'fail-policy', + sourceProjectId: 'p1', + name: 'billing', +}; + const credentialUnresolved: BlockingIssue = { type: 'credential-unresolved', kind: 'not_found', @@ -42,6 +49,11 @@ describe('toImportBlockedError', () => { expect(error).toBeInstanceOf(ConflictError); }); + it('maps a project-conflict to 409 Conflict', () => { + const error = toImportBlockedError([projectConflict]); + expect(error).toBeInstanceOf(ConflictError); + }); + it('still maps credential-only blocks to 422', () => { const error = toImportBlockedError([credentialUnresolved]); expect(error).toBeInstanceOf(UnprocessableRequestError); diff --git a/packages/cli/src/modules/n8n-packages/engine/import-blocked.error.ts b/packages/cli/src/modules/n8n-packages/engine/import-blocked.error.ts index f9fd3da19e0..2cab91336e7 100644 --- a/packages/cli/src/modules/n8n-packages/engine/import-blocked.error.ts +++ b/packages/cli/src/modules/n8n-packages/engine/import-blocked.error.ts @@ -16,6 +16,7 @@ export function toImportBlockedError( issue.type === 'workflow-conflict' || issue.type === 'workflow-id-conflict' || issue.type === 'workflow-folder-conflict' || + issue.type === 'project-conflict' || issue.type === 'folder-conflict' || issue.type === 'variable-conflict' || (issue.type === 'data-table-unresolved' && diff --git a/packages/cli/src/modules/n8n-packages/engine/project-package-importer.ts b/packages/cli/src/modules/n8n-packages/engine/project-package-importer.ts index 09e018fc9d0..8cebd626973 100644 --- a/packages/cli/src/modules/n8n-packages/engine/project-package-importer.ts +++ b/packages/cli/src/modules/n8n-packages/engine/project-package-importer.ts @@ -13,6 +13,7 @@ import { collectPlannedWorkflowBindings } from '../entities/workflow/workflow-im import { WorkflowPublisher } from '../entities/workflow/workflow-publisher'; import type { PackageReader } from '../io/package-reader'; import type { + BlockingIssue, ImportBindingMap, ImportedFolderSummary, ImportedWorkflowSummary, @@ -23,6 +24,7 @@ import type { } from '../n8n-packages.types'; import { mergeBindings } from '../n8n-packages.types'; import { assertPackageImportApiKeyScopes, assertTagWritesAllowed } from './import-gates'; +import { toImportBlockedError } from './import-blocked.error'; import { needsBundledVariableValues, placeByLayout } from './package-layout'; import { ImportOrchestrator, @@ -64,7 +66,21 @@ export class ProjectPackageImporter { this.assertAdequatePermissions(request, manifest); const projects = await this.packageParser.getProjects(reader); - const projectPlan = await this.projectImporter.plan(request.user, projects); + const projectPlan = await this.projectImporter.plan( + request.user, + projects, + request.projectConflictPolicy, + ); + // A refused project decides the whole import, so report it before reading any project's + // contents — planning work that is certain to be discarded only delays the same failure. + if (projectPlan.conflicts.length > 0) { + throw toImportBlockedError( + projectPlan.conflicts.map( + (conflict): BlockingIssue => ({ type: 'project-conflict', ...conflict }), + ), + ); + } + const bundledVariables = needsBundledVariableValues( request, (manifest.requirements?.variables?.length ?? 0) > 0, @@ -74,7 +90,9 @@ export class ProjectPackageImporter { // Projects the user is creating (vs matching an existing one). They will be admin of these, // so publish is always allowed and the project need not exist while its contents are planned. const pendingCreateIds = new Set( - projectPlan.filter((item) => item.action === 'create').map((item) => item.sourceProjectId), + projectPlan.items + .filter((item) => item.action === 'create') + .map((item) => item.sourceProjectId), ); // Plan and validate every project's contents before writing anything, so a blocking issue in @@ -102,7 +120,7 @@ export class ProjectPackageImporter { { apiKeyScopes: request.apiKeyScopes }, ); - const projectSummaries = await this.projectImporter.apply(request.user, projectPlan); + const projectSummaries = await this.projectImporter.apply(request.user, projectPlan.items); // Resolve every project's workflow ids up front so a sub-workflow reference // that points into another project resolves when its parent is applied. diff --git a/packages/cli/src/modules/n8n-packages/entities/project/__tests__/project-conflict-policy.test.ts b/packages/cli/src/modules/n8n-packages/entities/project/__tests__/project-conflict-policy.test.ts new file mode 100644 index 00000000000..1532f41598b --- /dev/null +++ b/packages/cli/src/modules/n8n-packages/entities/project/__tests__/project-conflict-policy.test.ts @@ -0,0 +1,15 @@ +import { decideMatchedProject } from '../project-conflict-policy'; + +describe('decideMatchedProject', () => { + it('skips without blocking under merge', () => { + expect(decideMatchedProject('merge')).toEqual({ action: 'skip', blocked: false }); + }); + + it('blocks under fail', () => { + expect(decideMatchedProject('fail')).toMatchObject({ blocked: true }); + }); + + it('updates under overwrite', () => { + expect(decideMatchedProject('overwrite')).toEqual({ action: 'update', blocked: false }); + }); +}); diff --git a/packages/cli/src/modules/n8n-packages/entities/project/__tests__/project-importer.test.ts b/packages/cli/src/modules/n8n-packages/entities/project/__tests__/project-importer.test.ts index ad5dc061891..0709388a528 100644 --- a/packages/cli/src/modules/n8n-packages/entities/project/__tests__/project-importer.test.ts +++ b/packages/cli/src/modules/n8n-packages/entities/project/__tests__/project-importer.test.ts @@ -4,7 +4,7 @@ import { mock } from 'vitest-mock-extended'; import type { ProjectService } from '@/services/project.service.ee'; -import type { ProjectPlanItem } from '../project-import.types'; +import type { PreparedProject, ProjectPlanItem } from '../project-import.types'; import { ProjectImporter } from '../project-importer'; const user = mock({ id: 'user-1' }); @@ -75,3 +75,94 @@ describe('ProjectImporter.apply — custom span attributes', () => { ); }); }); + +describe('ProjectImporter.plan — projectConflictPolicy', () => { + const projectCreator = mock({ + id: 'user-1', + role: { slug: 'global:owner', scopes: [{ slug: 'project:create' }] }, + }); + + const packaged: PreparedProject = { + sourceProjectId: 'proj-1', + name: 'billing renamed', + description: 'from the package', + }; + + function makeImporter(existing?: Project) { + const projectService = mock(); + projectService.findProject.mockResolvedValue(existing ?? null); + projectService.getProjectWithScope.mockResolvedValue(existing ?? null); + const licenseState = mock(); + licenseState.isLicensed.mockReturnValue(true); + return { importer: new ProjectImporter(projectService, licenseState), projectService }; + } + + const existingProject = () => mock({ id: 'proj-1', name: 'billing', type: 'team' }); + + it('plans an update under overwrite', async () => { + const { importer } = makeImporter(existingProject()); + + const plan = await importer.plan(user, [packaged], 'overwrite'); + + expect(plan.conflicts).toEqual([]); + expect(plan.items).toEqual([ + expect.objectContaining({ action: 'update', name: 'billing renamed' }), + ]); + }); + + it('plans a skip carrying the existing name under merge', async () => { + const { importer } = makeImporter(existingProject()); + + const plan = await importer.plan(user, [packaged], 'merge'); + + expect(plan.conflicts).toEqual([]); + expect(plan.items).toEqual([ + expect.objectContaining({ action: 'skip', name: 'billing renamed', existingName: 'billing' }), + ]); + }); + + it('records a conflict and plans nothing under fail', async () => { + const { importer } = makeImporter(existingProject()); + + const plan = await importer.plan(user, [packaged], 'fail'); + + expect(plan.items).toEqual([]); + expect(plan.conflicts).toEqual([ + { kind: 'fail-policy', sourceProjectId: 'proj-1', name: 'billing renamed' }, + ]); + }); + + it.each(['merge', 'fail', 'overwrite'] as const)( + 'plans a create for an absent project under %s', + async (policy) => { + const { importer } = makeImporter(); + + const plan = await importer.plan(projectCreator, [packaged], policy); + + expect(plan.conflicts).toEqual([]); + expect(plan.items).toEqual([expect.objectContaining({ action: 'create' })]); + }, + ); +}); + +describe('ProjectImporter.apply — merge', () => { + it('reports a skipped project under the name it already has, without writing', async () => { + const projectService = mock(); + const importer = new ProjectImporter(projectService, mock()); + + const summaries = await importer.apply(user, [ + { + action: 'skip', + sourceProjectId: 'proj-1', + name: 'billing renamed', + existingName: 'billing', + }, + ]); + + expect(summaries).toEqual([ + { sourceProjectId: 'proj-1', localId: 'proj-1', name: 'billing', status: 'skipped' }, + ]); + expect(projectService.updateProject).not.toHaveBeenCalled(); + expect(projectService.createTeamProject).not.toHaveBeenCalled(); + }); +}); diff --git a/packages/cli/src/modules/n8n-packages/entities/project/project-conflict-policy.ts b/packages/cli/src/modules/n8n-packages/entities/project/project-conflict-policy.ts new file mode 100644 index 00000000000..0d30fb0c731 --- /dev/null +++ b/packages/cli/src/modules/n8n-packages/entities/project/project-conflict-policy.ts @@ -0,0 +1,22 @@ +import type { ProjectPlannedAction } from './project-import.types'; +import type { ProjectConflictPolicy } from '../../n8n-packages.types'; + +/** + * A conflict policy's verdict for one matched project. `blocked` aborts the import and is set + * only by the `fail` policy; `action` decides the matched project's own details. Its contents + * merge in either way. + */ +export interface MatchedProjectDecision { + action: Extract; + blocked: boolean; +} + +const PROJECT_CONFLICT_POLICIES: Record = { + merge: { action: 'skip', blocked: false }, + fail: { action: 'skip', blocked: true }, + overwrite: { action: 'update', blocked: false }, +}; + +export function decideMatchedProject(policy: ProjectConflictPolicy): MatchedProjectDecision { + return PROJECT_CONFLICT_POLICIES[policy]; +} diff --git a/packages/cli/src/modules/n8n-packages/entities/project/project-import.types.ts b/packages/cli/src/modules/n8n-packages/entities/project/project-import.types.ts index 2ab2bdb67fb..ee8fb93ea2b 100644 --- a/packages/cli/src/modules/n8n-packages/entities/project/project-import.types.ts +++ b/packages/cli/src/modules/n8n-packages/entities/project/project-import.types.ts @@ -1,3 +1,4 @@ +import type { ProjectConflict } from '../../n8n-packages.types'; import type { SerializedProject } from '../../spec/serialized/project.schema'; export interface PreparedProject { @@ -8,13 +9,27 @@ export interface PreparedProject { customTelemetryTags?: SerializedProject['customTelemetryTags']; } -export type ProjectPlannedAction = 'create' | 'update'; +export type ProjectPlannedAction = 'create' | 'update' | 'skip'; -export interface ProjectPlanItem { - action: ProjectPlannedAction; +/** The package's own details for one project, as they would be written. */ +export interface ProjectPlanFields { sourceProjectId: string; name: string; description?: string; icon?: SerializedProject['icon']; customTelemetryTags?: SerializedProject['customTelemetryTags']; } + +/** + * The decided action for one package project. Discriminated by `action` so that `skip` — which + * leaves the matched project in place — always carries the name it already has on the target, + * and the writing actions cannot claim one. + */ +export type ProjectPlanItem = + | ({ action: 'create' | 'update' } & ProjectPlanFields) + | ({ action: 'skip'; existingName: string } & ProjectPlanFields); + +export interface ProjectImportPlan { + items: ProjectPlanItem[]; + conflicts: ProjectConflict[]; +} diff --git a/packages/cli/src/modules/n8n-packages/entities/project/project-importer.ts b/packages/cli/src/modules/n8n-packages/entities/project/project-importer.ts index aad3bf6c22b..c3aa1e9ac60 100644 --- a/packages/cli/src/modules/n8n-packages/entities/project/project-importer.ts +++ b/packages/cli/src/modules/n8n-packages/entities/project/project-importer.ts @@ -2,12 +2,23 @@ import { LicenseState } from '@n8n/backend-common'; import type { User } from '@n8n/db'; import { Service } from '@n8n/di'; import { hasGlobalScope } from '@n8n/permissions'; +import { UnexpectedError } from 'n8n-workflow'; import { ForbiddenError } from '@/errors/response-errors/forbidden.error'; import { ProjectService } from '@/services/project.service.ee'; -import type { PreparedProject, ProjectPlanItem } from './project-import.types'; -import type { ImportedProjectSummary } from '../../n8n-packages.types'; +import { decideMatchedProject } from './project-conflict-policy'; +import type { + PreparedProject, + ProjectImportPlan, + ProjectPlanFields, + ProjectPlanItem, +} from './project-import.types'; +import type { + ImportedProjectSummary, + ProjectConflict, + ProjectConflictPolicy, +} from '../../n8n-packages.types'; @Service() export class ProjectImporter { @@ -16,8 +27,14 @@ export class ProjectImporter { private readonly licenseState: LicenseState, ) {} - async plan(user: User, projects: PreparedProject[]): Promise { + async plan( + user: User, + projects: PreparedProject[], + policy: ProjectConflictPolicy, + ): Promise { const items: ProjectPlanItem[] = []; + const conflicts: ProjectConflict[] = []; + const decision = decideMatchedProject(policy); for (const project of projects) { const existing = await this.projectService.findProject(project.sourceProjectId); @@ -29,6 +46,7 @@ export class ProjectImporter { `Cannot import project "${project.name}": its id belongs to a non-team project on this instance.`, ); } + // Required for `merge` too: the package's contents are still written into the project. const authorized = await this.projectService.getProjectWithScope(user, existing.id, [ 'project:update', ]); @@ -37,49 +55,86 @@ export class ProjectImporter { `You do not have permission to update the existing project "${project.name}".`, ); } - items.push({ action: 'update', ...toFields(project) }); + + if (decision.blocked) { + conflicts.push({ + kind: 'fail-policy', + sourceProjectId: project.sourceProjectId, + name: project.name, + }); + continue; + } + + items.push( + decision.action === 'update' + ? { action: 'update', ...toFields(project) } + : { action: 'skip', existingName: existing.name, ...toFields(project) }, + ); } else { this.assertCanCreateProjects(user); items.push({ action: 'create', ...toFields(project) }); } } - return items; + return { items, conflicts }; } async apply(user: User, items: ProjectPlanItem[]): Promise { const summaries: ImportedProjectSummary[] = []; for (const item of items) { - if (item.action === 'create') { - const project = await this.projectService.createTeamProject( - user, - { name: item.name, icon: item.icon }, - { - id: item.sourceProjectId, - description: item.description ?? null, + switch (item.action) { + case 'skip': + // `merge`: the existing project is left untouched; only its contents come in. + summaries.push({ + sourceProjectId: item.sourceProjectId, + localId: item.sourceProjectId, + name: item.existingName, + status: 'skipped', + }); + break; + + case 'create': { + const project = await this.projectService.createTeamProject( + user, + { name: item.name, icon: item.icon }, + { + id: item.sourceProjectId, + description: item.description ?? null, + customTelemetryTags: item.customTelemetryTags, + }, + ); + summaries.push({ + sourceProjectId: item.sourceProjectId, + localId: project.id, + name: project.name, + status: 'created', + }); + break; + } + + case 'update': + await this.projectService.updateProject(item.sourceProjectId, { + name: item.name, + icon: item.icon, + description: item.description, customTelemetryTags: item.customTelemetryTags, - }, - ); - summaries.push({ - sourceProjectId: item.sourceProjectId, - localId: project.id, - name: project.name, - status: 'created', - }); - } else { - await this.projectService.updateProject(item.sourceProjectId, { - name: item.name, - icon: item.icon, - description: item.description, - customTelemetryTags: item.customTelemetryTags, - }); - summaries.push({ - sourceProjectId: item.sourceProjectId, - localId: item.sourceProjectId, - name: item.name, - status: 'updated', - }); + }); + summaries.push({ + sourceProjectId: item.sourceProjectId, + localId: item.sourceProjectId, + name: item.name, + status: 'updated', + }); + break; + + default: { + // A new action must decide for itself whether it writes; never fall into `update`. + const unhandled: never = item; + throw new UnexpectedError( + `Unhandled project import action: ${JSON.stringify(unhandled)}`, + ); + } } } @@ -98,7 +153,7 @@ export class ProjectImporter { } } -function toFields(project: PreparedProject): Omit { +function toFields(project: PreparedProject): ProjectPlanFields { return { sourceProjectId: project.sourceProjectId, name: project.name, diff --git a/packages/cli/src/modules/n8n-packages/n8n-packages.types.ts b/packages/cli/src/modules/n8n-packages/n8n-packages.types.ts index 7db44f9c1e5..cd61402aba0 100644 --- a/packages/cli/src/modules/n8n-packages/n8n-packages.types.ts +++ b/packages/cli/src/modules/n8n-packages/n8n-packages.types.ts @@ -44,6 +44,19 @@ export const WorkflowIdPolicy = { Source: 'source', } as const; +export const ProjectConflictPolicy = { + /** Reuses a matched project (by id) as-is — its name, description, icon and tags stay untouched — and merges the package's contents into it. */ + Merge: 'merge', + /** Fails the import if any package project already exists on this instance. */ + Fail: 'fail', + /** + * Replaces a matched project's own details with the package's, then merges the package's contents + * into it. Only the details the package carries are written — a detail the package omits (an + * unset description or icon, or a field a package predating it never had) is left as it is. + */ + Overwrite: 'overwrite', +} as const; + export const FolderConflictPolicy = { /** Reuses an already-imported folder (matched by id) as-is and merges the package's children into it; otherwise creates it. */ Merge: 'merge', @@ -135,6 +148,9 @@ export type WorkflowConflictPolicy = export type WorkflowIdPolicy = (typeof WorkflowIdPolicy)[keyof typeof WorkflowIdPolicy]; +export type ProjectConflictPolicy = + (typeof ProjectConflictPolicy)[keyof typeof ProjectConflictPolicy]; + export type FolderConflictPolicy = (typeof FolderConflictPolicy)[keyof typeof FolderConflictPolicy]; export type MissingNodeTypeMode = (typeof MissingNodeTypeMode)[keyof typeof MissingNodeTypeMode]; @@ -181,6 +197,7 @@ export type ImportPackageRequest = { apiKeyScopes?: string[]; } & ImportCredentialProperties & ImportWorkflowProperties & + ImportProjectProperties & ImportFolderProperties & ImportDataTableProperties & ImportVariableProperties & @@ -198,6 +215,11 @@ export type ImportWorkflowProperties = { missingNodeTypeMode: MissingNodeTypeMode; }; +/** Only project packages define projects; a workflow package imports into an existing project. */ +export type ImportProjectProperties = { + projectConflictPolicy: ProjectConflictPolicy; +}; + export type ImportFolderProperties = { folderConflictPolicy: FolderConflictPolicy; }; @@ -331,8 +353,9 @@ export interface ImportedFolderSummary { export interface ImportedProjectSummary { sourceProjectId: string; localId: string; + /** The project's name on the target — the package's under `overwrite`, the existing one under `merge`. */ name: string; - status: 'created' | 'updated'; + status: 'created' | 'updated' | 'skipped'; } /** @@ -355,6 +378,7 @@ export type BlockingIssue = actualType?: string; usedByWorkflows: string[]; } + | ({ type: 'project-conflict' } & ProjectConflict) | ({ type: 'folder-conflict' } & FolderConflict) | ({ type: 'data-table-unresolved' } & DataTableResolutionFailure) | ({ type: 'tag-unresolved' } & TagResolutionFailure) @@ -369,6 +393,12 @@ export type BlockingIssue = usedByWorkflows: string[]; }; +export interface ProjectConflict { + kind: 'fail-policy'; + sourceProjectId: string; + name: string; +} + export interface FolderConflict { kind: 'parent-mismatch' | 'id-in-other-project' | 'fail-policy'; sourceFolderId: string; diff --git a/packages/cli/src/public-api/v1/handlers/n8n-packages/n8n-packages.handler.ts b/packages/cli/src/public-api/v1/handlers/n8n-packages/n8n-packages.handler.ts index 804d77242c7..d720a025d0d 100644 --- a/packages/cli/src/public-api/v1/handlers/n8n-packages/n8n-packages.handler.ts +++ b/packages/cli/src/public-api/v1/handlers/n8n-packages/n8n-packages.handler.ts @@ -205,6 +205,7 @@ const n8nPackagesHandlers: N8nPackagesHandlers = { workflowPublishingPolicy: payload.data.workflowPublishingPolicy, workflowIdPolicy: payload.data.workflowIdPolicy, missingNodeTypeMode: payload.data.missingNodeTypeMode, + projectConflictPolicy: payload.data.projectConflictPolicy, folderConflictPolicy: payload.data.folderConflictPolicy, dataTableMatchingMode: payload.data.dataTableMatchingMode, dataTableMissingMode: payload.data.dataTableMissingMode, diff --git a/packages/cli/src/public-api/v1/handlers/n8n-packages/spec/paths/n8n-packages.import.yml b/packages/cli/src/public-api/v1/handlers/n8n-packages/spec/paths/n8n-packages.import.yml index 0ba86ba0c5d..e09e4d7c770 100644 --- a/packages/cli/src/public-api/v1/handlers/n8n-packages/spec/paths/n8n-packages.import.yml +++ b/packages/cli/src/public-api/v1/handlers/n8n-packages/spec/paths/n8n-packages.import.yml @@ -11,8 +11,8 @@ post: Imports a gzip-compressed tar package (`.n8np`) into the target project. Send the archive as the multipart field `package`. Optional routing uses form fields `projectId` and `folderId` (omit or send empty for defaults). Every optional - mode/policy field (credential, workflow, folder, data table, variable, and tag) - takes its default when omitted. The required + mode/policy field (credential, workflow, project, folder, data table, variable, + and tag) takes its default when omitted. The required `workflowConflictPolicy` field controls what happens when a package workflow matches an existing workflow by source id in the target project. Maximum upload size is `N8N_ENDPOINTS_PAYLOAD_SIZE_MAX` MB (default 16). @@ -143,6 +143,24 @@ post: `publish-all` publishes every imported workflow. `unpublish-all` leaves new workflows inactive and unpublishes updated workflows that were published. + projectConflictPolicy: + type: string + enum: + - merge + - fail + - overwrite + default: merge + description: > + What to do when a project the package defines already exists here + (matched by id), and — unless `folderConflictPolicy` overrides it — how + its contents are treated. `merge` (default) leaves the existing + project's name, description, icon and custom span attributes untouched + and adds the package's contents alongside. `overwrite` replaces those + details with the package's; a detail the package omits is left as it is, + not cleared. `fail` rejects the import before anything is written, with + one `project-conflict` issue per existing project. A package project id + belonging to a personal project, or to a team project the caller cannot + update, always rejects the import. Ignored for workflow packages. folderConflictPolicy: type: string enum: @@ -464,8 +482,9 @@ post: projects: type: array description: > - Project shells created or matched-and-updated. Present for project - packages; empty when importing loose workflows/folders. + Project shells created, or matched and then updated or left as-is + (see `projectConflictPolicy`). Present for project packages; empty + when importing loose workflows/folders. items: type: object required: @@ -482,12 +501,19 @@ post: description: Project id on the target instance (equal to `sourceProjectId`; project ids are reused). name: type: string + description: > + The project's name on the target: the package's under + `overwrite`, the pre-existing one under `merge`. status: type: string enum: - created - updated - description: Import outcome for this package project. + - skipped + description: > + Import outcome for this package project. `skipped` means the + project already existed and `projectConflictPolicy=merge` left + its details untouched — its contents were still imported. credentials: type: object description: > diff --git a/packages/cli/src/public-api/v1/handlers/n8n-packages/spec/schemas/importBlockingIssue.yml b/packages/cli/src/public-api/v1/handlers/n8n-packages/spec/schemas/importBlockingIssue.yml index 25eeb33c8aa..ad57ee896c4 100644 --- a/packages/cli/src/public-api/v1/handlers/n8n-packages/spec/schemas/importBlockingIssue.yml +++ b/packages/cli/src/public-api/v1/handlers/n8n-packages/spec/schemas/importBlockingIssue.yml @@ -85,6 +85,29 @@ oneOf: description: Folder the import was requested to land in. name: type: string + - type: object + description: > + A project defined by the package that already exists on this instance, under + `projectConflictPolicy=fail`. + required: + - type + - kind + - sourceProjectId + - name + properties: + type: + type: string + enum: + - project-conflict + kind: + type: string + enum: + - fail-policy + sourceProjectId: + type: string + name: + type: string + description: The project's name as it appears in the package. - type: object description: > A package folder that cannot be imported as-is. `kind` distinguishes the cause: