From 4044d58d130fd2f86e906a1f41cb561ff2c8e31b Mon Sep 17 00:00:00 2001 From: Jan Kalkan Date: Wed, 22 Jul 2026 21:09:27 +0000 Subject: [PATCH] feat(core): Validate missing node types on package import (#34598) Co-authored-by: Claude Fable 5 --- .../import-package-request.dto.test.ts | 43 +++- .../packages/import-package-request.dto.ts | 2 + packages/@n8n/cli/docs/commands/package.md | 1 + .../@n8n/cli/src/__tests__/client.test.ts | 26 +- .../cli/src/__tests__/package-import.test.ts | 3 + packages/@n8n/cli/src/client.ts | 1 + .../@n8n/cli/src/commands/package/import.ts | 7 + .../@n8n/cli/src/commands/package/shared.ts | 12 +- packages/cli/src/__tests__/node-types.test.ts | 124 ++++++++- packages/cli/src/__tests__/utils.test.ts | 23 ++ .../log-streaming-event-relay.test.ts | 2 + .../__tests__/telemetry-event-relay.test.ts | 2 + .../events/relays/telemetry.event-relay.ts | 1 + .../export-folder.integration.test.ts | 5 + .../export-project.integration.test.ts | 4 + ...kflow-with-credentials.integration.test.ts | 3 + ...kflow-with-data-tables.integration.test.ts | 1 + ...orkflow-with-variables.integration.test.ts | 11 +- .../export-workflow.integration.test.ts | 73 ++++++ .../import-data-tables.integration.test.ts | 5 + .../import-folders.integration.test.ts | 5 + .../import-package.integration.test.ts | 242 ++++++++++++++++++ .../import-projects.integration.test.ts | 59 +++++ ...orkflow-with-variables.integration.test.ts | 3 + .../engine/__tests__/import-telemetry.test.ts | 1 + .../engine/import-orchestrator.ts | 46 +++- .../n8n-packages/engine/import-result.ts | 4 +- .../n8n-packages/engine/import-telemetry.ts | 1 + .../__tests__/requirements.types.test.ts | 17 +- .../folder/__tests__/folder.exporter.test.ts | 1 + .../entities/requirements.types.ts | 4 + .../__tests__/missing-node-type-mode.test.ts | 123 +++++++++ .../__tests__/workflow-publisher.test.ts | 44 +++- .../__tests__/workflow.exporter.test.ts | 27 ++ .../workflow/missing-node-type-mode.ts | 52 ++++ .../entities/workflow/node-type-usage.ts | 46 ++++ .../workflow/workflow-import.types.ts | 5 +- .../entities/workflow/workflow-importer.ts | 2 +- .../entities/workflow/workflow-publisher.ts | 10 +- .../workflow-publishing-policy.types.ts | 2 +- .../entities/workflow/workflow.exporter.ts | 5 +- .../n8n-packages/n8n-packages.service.ts | 6 +- .../n8n-packages/n8n-packages.types.ts | 19 +- .../__tests__/requirements.schema.test.ts | 22 ++ .../n8n-packages/spec/requirements.schema.ts | 22 ++ .../__tests__/workflow.schema.test.ts | 31 +++ .../spec/serialized/workflow.schema.ts | 4 +- packages/cli/src/node-types.ts | 105 ++++++-- .../n8n-packages/n8n-packages.handler.ts | 1 + .../spec/paths/n8n-packages.import.yml | 22 +- .../spec/schemas/importBlockingIssue.yml | 26 ++ packages/cli/src/utils.ts | 11 +- .../public-api/n8n-packages-import.test.ts | 70 +++++ .../test/integration/shared/utils/index.ts | 14 + 54 files changed, 1350 insertions(+), 51 deletions(-) create mode 100644 packages/cli/src/modules/n8n-packages/entities/workflow/__tests__/missing-node-type-mode.test.ts create mode 100644 packages/cli/src/modules/n8n-packages/entities/workflow/missing-node-type-mode.ts create mode 100644 packages/cli/src/modules/n8n-packages/entities/workflow/node-type-usage.ts create mode 100644 packages/cli/src/modules/n8n-packages/spec/serialized/__tests__/workflow.schema.test.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 81bacdfd185..8879dfb7754 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 @@ -1,4 +1,7 @@ -import { ImportPackageRequestDto } from '../import-package-request.dto'; +import { + ImportPackageRequestDto, + IMPORT_PACKAGE_REQUEST_FORM_FIELDS, +} from '../import-package-request.dto'; describe('ImportPackageRequestDto', () => { it('accepts omitted routing fields and defaults credential modes', () => { @@ -12,6 +15,7 @@ describe('ImportPackageRequestDto', () => { workflowConflictPolicy: 'fail', workflowPublishingPolicy: 'preserve-published-state', workflowIdPolicy: 'new', + missingNodeTypeMode: 'fail', folderConflictPolicy: 'merge', dataTableMatchingMode: 'by-id', dataTableMissingMode: 'create', @@ -36,6 +40,7 @@ describe('ImportPackageRequestDto', () => { workflowConflictPolicy: 'fail', workflowPublishingPolicy: 'preserve-published-state', workflowIdPolicy: 'new', + missingNodeTypeMode: 'fail', folderConflictPolicy: 'merge', dataTableMatchingMode: 'by-id', dataTableMissingMode: 'create', @@ -62,6 +67,7 @@ describe('ImportPackageRequestDto', () => { workflowConflictPolicy: 'new-version', workflowPublishingPolicy: 'preserve-published-state', workflowIdPolicy: 'new', + missingNodeTypeMode: 'fail', folderConflictPolicy: 'merge', dataTableMatchingMode: 'by-id', dataTableMissingMode: 'create', @@ -87,6 +93,7 @@ describe('ImportPackageRequestDto', () => { workflowConflictPolicy: 'skip', workflowPublishingPolicy: 'preserve-published-state', workflowIdPolicy: 'new', + missingNodeTypeMode: 'fail', folderConflictPolicy: 'merge', dataTableMatchingMode: 'by-id', dataTableMissingMode: 'create', @@ -276,6 +283,40 @@ describe('ImportPackageRequestDto', () => { }); }); + describe('missingNodeTypeMode', () => { + it('defaults to "fail" when omitted', () => { + const result = ImportPackageRequestDto.safeParse({ workflowConflictPolicy: 'fail' }); + expect(result.success).toBe(true); + if (result.success) { + expect(result.data.missingNodeTypeMode).toBe('fail'); + } + }); + + it('accepts "import-anyway"', () => { + const result = ImportPackageRequestDto.safeParse({ + workflowConflictPolicy: 'fail', + missingNodeTypeMode: 'import-anyway', + }); + expect(result.success).toBe(true); + if (result.success) { + expect(result.data.missingNodeTypeMode).toBe('import-anyway'); + } + }); + + it('rejects unsupported missingNodeTypeMode values', () => { + expect( + ImportPackageRequestDto.safeParse({ + workflowConflictPolicy: 'fail', + missingNodeTypeMode: 'skip', + }).success, + ).toBe(false); + }); + + it('is accepted as a multipart form field', () => { + expect(IMPORT_PACKAGE_REQUEST_FORM_FIELDS).toContain('missingNodeTypeMode'); + }); + }); + it.each([ { name: 'non-string projectId', request: { projectId: 1, workflowConflictPolicy: 'fail' } }, { name: 'non-string folderId', request: { folderId: false, workflowConflictPolicy: 'fail' } }, 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 bb7923ee1bb..e439701e65d 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 @@ -12,6 +12,7 @@ export const IMPORT_PACKAGE_REQUEST_FORM_FIELDS = [ 'workflowConflictPolicy', 'workflowPublishingPolicy', 'workflowIdPolicy', + 'missingNodeTypeMode', 'folderConflictPolicy', 'dataTableMatchingMode', 'dataTableMissingMode', @@ -77,6 +78,7 @@ export class ImportPackageRequestDto extends Z.class({ .optional() .default('preserve-published-state'), workflowIdPolicy: z.enum(['new', 'source']).optional().default('new'), + missingNodeTypeMode: z.enum(['fail', 'import-anyway']).optional().default('fail'), folderConflictPolicy: z.enum(['merge', 'fail']).optional().default('merge'), dataTableMatchingMode: z.enum(['by-id']).optional().default('by-id'), dataTableMissingMode: z diff --git a/packages/@n8n/cli/docs/commands/package.md b/packages/@n8n/cli/docs/commands/package.md index 29531a4cc08..e2dfc1bde25 100644 --- a/packages/@n8n/cli/docs/commands/package.md +++ b/packages/@n8n/cli/docs/commands/package.md @@ -57,6 +57,7 @@ n8n-cli package import --file=export.n8np --conflict-policy=fail --bindings='{"c | `--folder` | Target folder ID within the project. Defaults to the project root. | | `--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. | | `--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. | diff --git a/packages/@n8n/cli/src/__tests__/client.test.ts b/packages/@n8n/cli/src/__tests__/client.test.ts index a749cf98c46..d94cae8d219 100644 --- a/packages/@n8n/cli/src/__tests__/client.test.ts +++ b/packages/@n8n/cli/src/__tests__/client.test.ts @@ -144,6 +144,7 @@ describe('N8nClient packages', () => { folderId: '', workflowIdPolicy: 'new', credentialMatchingMode: undefined, + missingNodeTypeMode: undefined, }, ); @@ -155,9 +156,11 @@ describe('N8nClient packages', () => { expect(form.get('workflowConflictPolicy')).toBe('fail'); expect(form.get('projectId')).toBe('proj-1'); expect(form.get('workflowIdPolicy')).toBe('new'); - // Empty/undefined fields are omitted entirely. + // Empty/undefined fields are omitted entirely (an omitted CLI flag + // means the instance default decides). expect(form.has('folderId')).toBe(false); expect(form.has('credentialMatchingMode')).toBe(false); + expect(form.has('missingNodeTypeMode')).toBe(false); const pkg = form.get('package'); expect(pkg).toBeInstanceOf(Blob); @@ -189,6 +192,27 @@ describe('N8nClient packages', () => { expect(form.get('credentialMissingMode')).toBe('create-stub'); }); + it('sends missingNodeTypeMode when provided', async () => { + fetchMock.mockResolvedValue( + jsonResponse(200, { + workflows: [], + bindings: {}, + credentials: { matched: [], stubbed: [] }, + }), + ); + + await client.importPackage( + { buffer: Buffer.from('package-bytes'), filename: 'export.n8np' }, + { + workflowConflictPolicy: 'fail', + missingNodeTypeMode: 'import-anyway', + }, + ); + + const form = (fetchMock.mock.calls[0] as [string, RequestInit])[1].body as FormData; + expect(form.get('missingNodeTypeMode')).toBe('import-anyway'); + }); + it('sends the data table modes when provided', async () => { fetchMock.mockResolvedValue( jsonResponse(200, { diff --git a/packages/@n8n/cli/src/__tests__/package-import.test.ts b/packages/@n8n/cli/src/__tests__/package-import.test.ts index 820a3343866..00a970d1c0a 100644 --- a/packages/@n8n/cli/src/__tests__/package-import.test.ts +++ b/packages/@n8n/cli/src/__tests__/package-import.test.ts @@ -14,6 +14,7 @@ interface ImportFlags { conflictPolicy: string; workflowPublishingPolicy?: string; workflowIdPolicy?: string; + missingNodeTypeMode?: string; folderConflictPolicy?: string; credentialMatchingMode?: string; credentialMissingMode?: string; @@ -59,6 +60,7 @@ describe('package import command', () => { conflictPolicy: 'fail', workflowPublishingPolicy: 'publish-all', workflowIdPolicy: 'new', + missingNodeTypeMode: 'import-anyway', folderConflictPolicy: 'merge', credentialMatchingMode: 'id-only', credentialMissingMode: 'create-stub', @@ -82,6 +84,7 @@ describe('package import command', () => { workflowConflictPolicy: 'fail', workflowPublishingPolicy: 'publish-all', workflowIdPolicy: 'new', + missingNodeTypeMode: 'import-anyway', folderConflictPolicy: 'merge', credentialMatchingMode: 'id-only', credentialMissingMode: 'create-stub', diff --git a/packages/@n8n/cli/src/client.ts b/packages/@n8n/cli/src/client.ts index 0f4f06d4ae1..337a5f6c2d5 100644 --- a/packages/@n8n/cli/src/client.ts +++ b/packages/@n8n/cli/src/client.ts @@ -18,6 +18,7 @@ export interface ImportPackageFields { workflowConflictPolicy: string; workflowPublishingPolicy?: string; workflowIdPolicy?: string; + missingNodeTypeMode?: 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 307dd7968e6..e4752bcfd67 100644 --- a/packages/@n8n/cli/src/commands/package/import.ts +++ b/packages/@n8n/cli/src/commands/package/import.ts @@ -41,6 +41,12 @@ export default class PackageImport extends BaseCommand { options: ['new', 'source'], aliases: ['workflow-id-policy'], }), + missingNodeTypeMode: Flags.string({ + description: + 'What to do when a workflow uses a node type or version this instance does not have (default on the instance: fail). With import-anyway, affected workflows are imported but never published', + options: ['fail', 'import-anyway'], + aliases: ['missing-node-type-mode'], + }), folderConflictPolicy: Flags.string({ description: 'What to do when a package folder already exists in the target project', options: ['merge', 'fail'], @@ -106,6 +112,7 @@ export default class PackageImport extends BaseCommand { workflowConflictPolicy: flags.conflictPolicy, workflowPublishingPolicy: flags.workflowPublishingPolicy, workflowIdPolicy: flags.workflowIdPolicy, + missingNodeTypeMode: flags.missingNodeTypeMode, folderConflictPolicy: flags.folderConflictPolicy, credentialMatchingMode: flags.credentialMatchingMode, credentialMissingMode: flags.credentialMissingMode, diff --git a/packages/@n8n/cli/src/commands/package/shared.ts b/packages/@n8n/cli/src/commands/package/shared.ts index 93150105f88..3c243788e31 100644 --- a/packages/@n8n/cli/src/commands/package/shared.ts +++ b/packages/@n8n/cli/src/commands/package/shared.ts @@ -8,7 +8,13 @@ type BlockingIssue = name: string; } | { type: 'credential-unresolved'; kind: string; sourceId: string; usedByWorkflows: string[] } - | { type: 'variable-unresolved'; name: string; usedByWorkflows: string[] }; + | { type: 'variable-unresolved'; name: string; usedByWorkflows: string[] } + | { + type: 'missing-node-type'; + nodeType: string; + typeVersion: number; + usedByWorkflows: string[]; + }; function formatIssue(issue: unknown): string { if (typeof issue !== 'object' || issue === null) return JSON.stringify(issue); @@ -24,6 +30,10 @@ function formatIssue(issue: unknown): string { const usedBy = Array.isArray(it.usedByWorkflows) ? it.usedByWorkflows.join(', ') : ''; return `variable "${it.name}" unresolved, used by workflow(s) ${usedBy}`; } + if (it.type === 'missing-node-type') { + const usedBy = Array.isArray(it.usedByWorkflows) ? it.usedByWorkflows.join(', ') : ''; + return `node type ${it.nodeType} @ v${it.typeVersion} missing on this instance, used by workflow(s) ${usedBy}`; + } return JSON.stringify(issue); } diff --git a/packages/cli/src/__tests__/node-types.test.ts b/packages/cli/src/__tests__/node-types.test.ts index 831f9c3a3bb..608f6a0f4f2 100644 --- a/packages/cli/src/__tests__/node-types.test.ts +++ b/packages/cli/src/__tests__/node-types.test.ts @@ -1,3 +1,4 @@ +import type { Logger } from '@n8n/backend-common'; import { RoutingNode, UnrecognizedNodeTypeError } from 'n8n-core'; import type { LoadedClass, @@ -11,9 +12,10 @@ import type { LoadNodesAndCredentials } from '@/load-nodes-and-credentials'; import { NodeTypes } from '@/node-types'; describe('NodeTypes', () => { + const logger = mock(); const loadNodesAndCredentials = mock(); - const nodeTypes: NodeTypes = new NodeTypes(loadNodesAndCredentials); + const nodeTypes: NodeTypes = new NodeTypes(logger, loadNodesAndCredentials); const nonVersionedNode: LoadedClass = { sourcePath: '', @@ -102,6 +104,50 @@ describe('NodeTypes', () => { supplyData: undefined, }, }; + // Versioned node whose v1 cannot be used as a tool while v2 can. Plain-object + // descriptions (not mock proxies) so `usableAsTool` reads are real. + const partiallyToolCapableNode: LoadedClass = { + sourcePath: '', + type: { + description: { + name: 'n8n-nodes-base.partiallyToolCapable', + displayName: 'Partially Tool Capable', + } as unknown as INodeTypeDescription, + currentVersion: 2, + nodeVersions: { + 1: { + description: { + name: 'n8n-nodes-base.partiallyToolCapable', + version: 1, + properties: [], + }, + } as unknown as INodeType, + 2: { + description: { + name: 'n8n-nodes-base.partiallyToolCapable', + version: 2, + usableAsTool: true, + properties: [], + }, + } as unknown as INodeType, + }, + getNodeType(version) { + return this.nodeVersions[version === 1 ? 1 : 2]; + }, + }, + }; + const multiVersionNode: LoadedClass = { + sourcePath: '', + type: { + description: { + name: 'n8n-nodes-base.multiVersion', + displayName: 'Multi Version Node', + version: [1, 1.1, 2], + properties: [], + } as unknown as INodeTypeDescription, + supplyData: undefined, + }, + }; const hitlSupportingNode: LoadedClass = { sourcePath: '', type: { @@ -162,6 +208,8 @@ describe('NodeTypes', () => { if (nodeType === 'declarativeNode') return declarativeNode; if (nodeType === 'toolNode') return toolNode; if (nodeType === 'plainToolNode') return plainToolSupportingNode; + if (nodeType === 'partiallyToolCapable') return partiallyToolCapableNode; + if (nodeType === 'multiVersion') return multiVersionNode; if (nodeType === 'hitlNode') return hitlSupportingNode; if (nodeType === 'realTool') return realToolNode; if (nodeType === 'replacementToolNode') return replacementToolNode; @@ -266,6 +314,80 @@ describe('NodeTypes', () => { }); }); + describe('getSupportedVersions', () => { + it('should return the single version of a plain node type', () => { + expect(nodeTypes.getSupportedVersions('n8n-nodes-base.hitlNode')).toEqual([1]); + }); + + it('should return every version of a plain node type with a version array', () => { + expect(nodeTypes.getSupportedVersions('n8n-nodes-base.multiVersion')).toEqual([1, 1.1, 2]); + }); + + it('should return the nodeVersions keys of a versioned node type', () => { + expect(nodeTypes.getSupportedVersions('n8n-nodes-base.versioned')).toEqual([1, 2]); + }); + + it('should return undefined for an unknown node type', () => { + expect(nodeTypes.getSupportedVersions('n8n-nodes-base.unknownNode')).toBeUndefined(); + expect(nodeTypes.getSupportedVersions('invalid-package.unknownNode')).toBeUndefined(); + }); + + it('should resolve a Tool-suffixed name against its base node', () => { + expect(nodeTypes.getSupportedVersions('n8n-nodes-base.plainToolNodeTool')).toEqual([1]); + }); + + it('should not count versions that cannot be used as a tool for a Tool-suffixed name', () => { + // The base node exists at version 1 but is not usableAsTool, so no + // version can satisfy the synthetic tool wrapper … + expect(nodeTypes.getSupportedVersions('n8n-nodes-base.hitlNodeTool')).toEqual([]); + // … while HitlTool wrappers have no usability requirement. + expect(nodeTypes.getSupportedVersions('n8n-nodes-base.hitlNodeHitlTool')).toEqual([1]); + }); + + it('should keep only tool-capable versions of a versioned node for a Tool-suffixed name', () => { + expect(nodeTypes.getSupportedVersions('n8n-nodes-base.partiallyToolCapableTool')).toEqual([ + 2, + ]); + expect(nodeTypes.getSupportedVersions('n8n-nodes-base.partiallyToolCapable')).toEqual([1, 2]); + }); + + it('should warn and return undefined when the node fails to load for another reason', () => { + loadNodesAndCredentials.getNode.mockImplementationOnce(() => { + throw new TypeError('boom'); + }); + + expect(nodeTypes.getSupportedVersions('n8n-nodes-base.hitlNode')).toBeUndefined(); + expect(logger.warn).toHaveBeenCalledWith( + 'Failed to resolve node type while listing supported versions', + { nodeType: 'n8n-nodes-base.hitlNode', error: 'boom' }, + ); + }); + + it('should warn and return undefined when name resolution surfaces a non-node value', () => { + // A hostile name can make `getNode` return a prototype-chain value + // instead of throwing; the reads after it must still fail closed. + loadNodesAndCredentials.getNode.mockReturnValueOnce(Object as never); + + expect(nodeTypes.getSupportedVersions('n8n-nodes-base.poisoned')).toBeUndefined(); + expect(logger.warn).toHaveBeenCalledWith( + 'Failed to resolve node type while listing supported versions', + expect.objectContaining({ nodeType: 'n8n-nodes-base.poisoned' }), + ); + }); + + it('should warn and return undefined when the tool-name check itself throws', () => { + loadNodesAndCredentials.recognizesNode.mockImplementationOnce(() => { + throw new TypeError('boom'); + }); + + expect(nodeTypes.getSupportedVersions('constructor.anythingTool')).toBeUndefined(); + expect(logger.warn).toHaveBeenCalledWith( + 'Failed to resolve node type while listing supported versions', + { nodeType: 'constructor.anythingTool', error: 'boom' }, + ); + }); + }); + describe('getWithSourcePath', () => { it('should return description and source path for existing node', () => { const result = nodeTypes.getWithSourcePath('n8n-nodes-base.nonVersioned', 1); diff --git a/packages/cli/src/__tests__/utils.test.ts b/packages/cli/src/__tests__/utils.test.ts index 392a9030a24..5d448b51653 100644 --- a/packages/cli/src/__tests__/utils.test.ts +++ b/packages/cli/src/__tests__/utils.test.ts @@ -5,6 +5,7 @@ import { shouldAssignExecuteMethod, getAllKeyPaths, isWorkflowIdValid, + satisfiesToolCapability, setMicrosoftObservabilityDefaults, containsExpression, stripToolSuffix, @@ -65,6 +66,28 @@ describe('stripToolSuffix', () => { }); }); +describe('satisfiesToolCapability', () => { + const nodeWith = (usableAsTool: boolean | undefined) => + ({ description: { usableAsTool } }) as INodeType; + + it('exempts HITL tool names from the capability requirement', () => { + expect(satisfiesToolCapability('n8n-nodes-base.gmailHitlTool', nodeWith(undefined))).toBe(true); + }); + + it('accepts a tool name when the resolved node declares usableAsTool', () => { + expect(satisfiesToolCapability('n8n-nodes-base.gmailTool', nodeWith(true))).toBe(true); + }); + + it.each([undefined, false])( + 'rejects a tool name when the resolved node has usableAsTool: %s', + (usableAsTool) => { + expect(satisfiesToolCapability('n8n-nodes-base.gmailTool', nodeWith(usableAsTool))).toBe( + false, + ); + }, + ); +}); + describe('shouldAssignExecuteMethod', () => { it('should return true when node has no execute, poll, trigger, webhook (unless declarative), or methods', () => { const nodeType = { diff --git a/packages/cli/src/events/__tests__/log-streaming-event-relay.test.ts b/packages/cli/src/events/__tests__/log-streaming-event-relay.test.ts index a4320b5a582..9fde1886eee 100644 --- a/packages/cli/src/events/__tests__/log-streaming-event-relay.test.ts +++ b/packages/cli/src/events/__tests__/log-streaming-event-relay.test.ts @@ -72,6 +72,7 @@ describe('LogStreamingEventRelay', () => { credentialMatchingMode: 'id-only', credentialMissingMode: 'must-preexist', workflowPublishingPolicy: 'preserve-published-state', + missingNodeTypeMode: 'fail', dataTableMatchingMode: 'by-id', dataTableMissingMode: 'create', dataTableSchemaConflictPolicy: 'keep-existing', @@ -128,6 +129,7 @@ describe('LogStreamingEventRelay', () => { credentialMatchingMode: 'id-only', credentialMissingMode: 'must-preexist', workflowPublishingPolicy: 'preserve-published-state', + missingNodeTypeMode: 'fail', dataTableMatchingMode: 'by-id', dataTableMissingMode: 'create', dataTableSchemaConflictPolicy: 'keep-existing', diff --git a/packages/cli/src/events/__tests__/telemetry-event-relay.test.ts b/packages/cli/src/events/__tests__/telemetry-event-relay.test.ts index 4021a2c36e9..1afaba6222d 100644 --- a/packages/cli/src/events/__tests__/telemetry-event-relay.test.ts +++ b/packages/cli/src/events/__tests__/telemetry-event-relay.test.ts @@ -2232,6 +2232,7 @@ describe('TelemetryEventRelay', () => { credentialMatchingMode: 'id-only', credentialMissingMode: 'must-preexist', workflowPublishingPolicy: 'preserve-published-state', + missingNodeTypeMode: 'fail', dataTableMatchingMode: 'by-id', dataTableMissingMode: 'create', dataTableSchemaConflictPolicy: 'keep-existing', @@ -2277,6 +2278,7 @@ describe('TelemetryEventRelay', () => { credential_matching_mode: 'id-only', credential_missing_mode: 'must-preexist', workflow_publishing_policy: 'preserve-published-state', + missing_node_type_mode: 'fail', data_table_matching_mode: 'by-id', data_table_missing_mode: 'create', data_table_schema_conflict_policy: 'keep-existing', diff --git a/packages/cli/src/events/relays/telemetry.event-relay.ts b/packages/cli/src/events/relays/telemetry.event-relay.ts index aeea195e420..3f9916039d8 100644 --- a/packages/cli/src/events/relays/telemetry.event-relay.ts +++ b/packages/cli/src/events/relays/telemetry.event-relay.ts @@ -1076,6 +1076,7 @@ export class TelemetryEventRelay extends EventRelay { credential_matching_mode: options.credentialMatchingMode, credential_missing_mode: options.credentialMissingMode, workflow_publishing_policy: options.workflowPublishingPolicy, + missing_node_type_mode: options.missingNodeTypeMode, data_table_matching_mode: options.dataTableMatchingMode, data_table_missing_mode: options.dataTableMissingMode, data_table_schema_conflict_policy: options.dataTableSchemaConflictPolicy, diff --git a/packages/cli/src/modules/n8n-packages/__tests__/export-folder.integration.test.ts b/packages/cli/src/modules/n8n-packages/__tests__/export-folder.integration.test.ts index 824a8302dcf..ec8572613da 100644 --- a/packages/cli/src/modules/n8n-packages/__tests__/export-folder.integration.test.ts +++ b/packages/cli/src/modules/n8n-packages/__tests__/export-folder.integration.test.ts @@ -392,6 +392,10 @@ describe('folder package export — with contained workflows', () => { usedByWorkflows: [workflow.id], }, ], + // Folder packages fold node type usage too (shared WorkflowExporter path). + nodeTypes: [ + { type: 'n8n-nodes-base.httpRequest', typeVersion: 1, usedByWorkflows: [workflow.id] }, + ], }); expect( entries.find((e) => e.name === `${manifest.credentials![0].target}/credential.json`), @@ -426,6 +430,7 @@ describe('folder package export — with contained workflows', () => { }, ]); expect(manifest.requirements).toEqual({ + nodeTypes: expect.any(Array), variables: [ { name: 'API_URL', value: 'https://api.example.com', usedByWorkflows: [workflow.id] }, ], diff --git a/packages/cli/src/modules/n8n-packages/__tests__/export-project.integration.test.ts b/packages/cli/src/modules/n8n-packages/__tests__/export-project.integration.test.ts index 717667f6d4b..a20d5d00242 100644 --- a/packages/cli/src/modules/n8n-packages/__tests__/export-project.integration.test.ts +++ b/packages/cli/src/modules/n8n-packages/__tests__/export-project.integration.test.ts @@ -313,6 +313,9 @@ describe('project package export — with folders / workflows', () => { entries.find((e) => e.name === `${credentialEntry.target}/credential.json`), ).toBeDefined(); expect(manifest.requirements).toEqual({ + nodeTypes: [ + { type: 'n8n-nodes-base.httpRequest', typeVersion: 1, usedByWorkflows: [workflow.id] }, + ], credentials: [ { id: credential.id, @@ -347,6 +350,7 @@ describe('project package export — with folders / workflows', () => { expect(variableEntry.target).toMatch(new RegExp(`^${projectEntry.target}/variables/[^/]+$`)); expect(entries.find((e) => e.name === `${variableEntry.target}/variable.json`)).toBeDefined(); expect(manifest.requirements).toEqual({ + nodeTypes: expect.any(Array), variables: [ { name: 'API_URL', value: 'https://team.example.com', usedByWorkflows: [workflow.id] }, ], diff --git a/packages/cli/src/modules/n8n-packages/__tests__/export-workflow-with-credentials.integration.test.ts b/packages/cli/src/modules/n8n-packages/__tests__/export-workflow-with-credentials.integration.test.ts index 1acd5a5e984..1873ea8f9cb 100644 --- a/packages/cli/src/modules/n8n-packages/__tests__/export-workflow-with-credentials.integration.test.ts +++ b/packages/cli/src/modules/n8n-packages/__tests__/export-workflow-with-credentials.integration.test.ts @@ -76,6 +76,7 @@ describe('workflow package export — with credentials', () => { }, ]); expect(manifest.requirements).toEqual({ + nodeTypes: expect.any(Array), credentials: [ { id: credential.id, @@ -195,6 +196,7 @@ describe('workflow package export — with credentials', () => { expect(manifest.credentials).toBeUndefined(); expect(manifest.requirements).toEqual({ + nodeTypes: expect.any(Array), credentials: [ { id: 'does-not-exist', @@ -241,6 +243,7 @@ describe('workflow package export — with credentials', () => { expect(manifest.credentials).toBeUndefined(); expect(manifest.requirements).toEqual({ + nodeTypes: expect.any(Array), credentials: [ { id: credential.id, diff --git a/packages/cli/src/modules/n8n-packages/__tests__/export-workflow-with-data-tables.integration.test.ts b/packages/cli/src/modules/n8n-packages/__tests__/export-workflow-with-data-tables.integration.test.ts index 7a8450e3171..299c500cc5c 100644 --- a/packages/cli/src/modules/n8n-packages/__tests__/export-workflow-with-data-tables.integration.test.ts +++ b/packages/cli/src/modules/n8n-packages/__tests__/export-workflow-with-data-tables.integration.test.ts @@ -72,6 +72,7 @@ describe('workflow package export — with data tables', () => { const { manifest, entries } = await readExport(stream); expect(manifest.requirements).toEqual({ + nodeTypes: expect.any(Array), dataTables: [ { id: dataTable.id, diff --git a/packages/cli/src/modules/n8n-packages/__tests__/export-workflow-with-variables.integration.test.ts b/packages/cli/src/modules/n8n-packages/__tests__/export-workflow-with-variables.integration.test.ts index 83b89691d11..ad4d3d6c8bd 100644 --- a/packages/cli/src/modules/n8n-packages/__tests__/export-workflow-with-variables.integration.test.ts +++ b/packages/cli/src/modules/n8n-packages/__tests__/export-workflow-with-variables.integration.test.ts @@ -72,6 +72,7 @@ describe('workflow package export — with variables', () => { }, ]); expect(manifest.requirements).toEqual({ + nodeTypes: expect.any(Array), variables: [ { name: 'API_URL', @@ -121,6 +122,7 @@ describe('workflow package export — with variables', () => { }, ]); expect(manifest.requirements).toEqual({ + nodeTypes: expect.any(Array), variables: [{ name: 'API_URL', usedByWorkflows: [workflow.id] }], }); expect(manifest.requirements!.variables![0]).not.toHaveProperty('value'); @@ -154,6 +156,7 @@ describe('workflow package export — with variables', () => { }, ]); expect(manifest.requirements).toEqual({ + nodeTypes: expect.any(Array), variables: [ { name: 'API_URL', @@ -192,6 +195,7 @@ describe('workflow package export — with variables', () => { expect(manifest).not.toHaveProperty('variables'); expect(manifest.requirements).toEqual({ + nodeTypes: expect.any(Array), variables: [{ name: 'API_URL', usedByWorkflows: [workflow.id] }], }); expect(manifest.requirements!.variables![0]).not.toHaveProperty('value'); @@ -292,6 +296,7 @@ describe('workflow package export — with variables', () => { }, ]); expect(manifest.requirements).toEqual({ + nodeTypes: expect.any(Array), variables: [{ name: 'legacy-key', value: 'legacy-value', usedByWorkflows: [workflow.id] }], }); expect(variableFiles(entries)).toHaveLength(1); @@ -311,6 +316,7 @@ describe('workflow package export — with variables', () => { expect(manifest).not.toHaveProperty('variables'); expect(manifest.requirements).toEqual({ + nodeTypes: expect.any(Array), variables: [{ name: 'DOES_NOT_EXIST', usedByWorkflows: [workflow.id] }], }); expect(variableFiles(entries)).toEqual([]); @@ -337,6 +343,7 @@ describe('workflow package export — with variables', () => { expect(manifest).not.toHaveProperty('variables'); expect(manifest.requirements).toEqual({ + nodeTypes: expect.any(Array), variables: [{ name: 'API_URL', usedByWorkflows: [workflow.id] }], }); expect(variableFiles(entries)).toEqual([]); @@ -446,6 +453,7 @@ describe('workflow package export — with variables', () => { expect(manifest).not.toHaveProperty('variables'); expect(manifest.requirements).toEqual({ + nodeTypes: expect.any(Array), variables: [{ name: 'PRIVATE_VAR', usedByWorkflows: [workflow.id] }], }); expect(variableFiles(entries)).toEqual([]); @@ -488,7 +496,7 @@ describe('workflow package export — with variables', () => { const { manifest } = await readExport(stream); expect(manifest).not.toHaveProperty('variables'); - expect(manifest.requirements).toBeUndefined(); + expect(manifest.requirements).toEqual({ nodeTypes: expect.any(Array) }); }); it('allows a value-less export of referenced variables when values are excluded', async () => { @@ -510,6 +518,7 @@ describe('workflow package export — with variables', () => { const { manifest, entries } = await readExport(stream); expect(manifest.requirements).toEqual({ + nodeTypes: expect.any(Array), variables: [{ name: 'API_URL', usedByWorkflows: [workflow.id] }], }); // Stubs still travel (name/type only) — the scope gate is value-only. diff --git a/packages/cli/src/modules/n8n-packages/__tests__/export-workflow.integration.test.ts b/packages/cli/src/modules/n8n-packages/__tests__/export-workflow.integration.test.ts index 6d79a13a00e..8a06e671af3 100644 --- a/packages/cli/src/modules/n8n-packages/__tests__/export-workflow.integration.test.ts +++ b/packages/cli/src/modules/n8n-packages/__tests__/export-workflow.integration.test.ts @@ -111,6 +111,58 @@ describe('workflow package export', () => { } }); + it('folds node type usage into manifest.requirements.nodeTypes', async () => { + const owner = await createOwner(); + const project = await createTeamProject('Project A', owner); + const trigger = (id: string) => ({ + id, + name: `Trigger ${id}`, + type: 'n8n-nodes-base.manualTrigger', + typeVersion: 1, + position: [0, 0] as [number, number], + parameters: {}, + }); + const wfA = await createWorkflow( + { name: 'Alpha', nodes: [trigger('t1')], connections: {} }, + project, + ); + const wfB = await createWorkflow( + { + name: 'Beta', + nodes: [ + trigger('t2'), + { + id: 's1', + name: 'Set', + type: 'n8n-nodes-base.set', + typeVersion: 3.4, + position: [200, 0], + parameters: {}, + }, + ], + connections: {}, + }, + project, + ); + + const stream = await service.exportPackage({ + user: owner, + workflowIds: [wfA.id, wfB.id], + }); + const { manifest } = await readExport(stream); + + expect(manifest.requirements).toEqual({ + nodeTypes: [ + { + type: 'n8n-nodes-base.manualTrigger', + typeVersion: 1, + usedByWorkflows: [wfA.id, wfB.id], + }, + { type: 'n8n-nodes-base.set', typeVersion: 3.4, usedByWorkflows: [wfB.id] }, + ], + }); + }); + it('disambiguates targets when two workflows share a name', async () => { const owner = await createOwner(); const project = await createTeamProject('Project A', owner); @@ -226,6 +278,13 @@ describe('workflow package export', () => { expect(manifest.requirements).toEqual({ workflows: [{ id: child.id, name: child.name, usedByWorkflows: [parent.id] }], + nodeTypes: [ + { + type: 'n8n-nodes-base.executeWorkflow', + typeVersion: 1, + usedByWorkflows: [parent.id], + }, + ], }); }); @@ -271,6 +330,13 @@ describe('workflow package export', () => { expect(manifest.requirements).toEqual({ workflows: [{ id: child.id, name: child.name, usedByWorkflows: [parent.id] }], + nodeTypes: [ + { + type: '@n8n/n8n-nodes-langchain.toolWorkflow', + typeVersion: 2.2, + usedByWorkflows: [parent.id], + }, + ], }); }); @@ -334,6 +400,13 @@ describe('workflow package export', () => { expect(manifest.requirements).toEqual({ workflows: [{ id: child.id, name: child.name, usedByWorkflows: expectedUsedByWorkflows }], + nodeTypes: [ + { + type: 'n8n-nodes-base.executeWorkflow', + typeVersion: 1, + usedByWorkflows: [parentA.id, parentB.id], + }, + ], }); }); 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 9eb159a78db..45f38d5797b 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 @@ -17,6 +17,7 @@ import { DataTableService } from '@/modules/data-table/data-table.service'; import { createFolder } from '@test-integration/db/folders'; import { createOwner } from '@test-integration/db/users'; import { LicenseMocker } from '@test-integration/license'; +import { initNodeTypes } from '@test-integration/utils'; import { N8nPackagesService } from '../n8n-packages.service'; import type { ImportPackageRequest } from '../n8n-packages.types'; @@ -42,6 +43,9 @@ const licenseMocker = new LicenseMocker(); beforeAll(async () => { await testModules.loadModules(['n8n-packages', 'data-table']); await testDb.init(); + // Register node types so the plan-phase missing-node-type check can resolve + // the node types used by the package fixtures. + await initNodeTypes(); mockDataTableSizeValidator(); licenseMocker.mockLicenseState(Container.get(LicenseState)); service = Container.get(N8nPackagesService); @@ -65,6 +69,7 @@ async function importPackage(params: ImportParams) { workflowConflictPolicy: 'fail', workflowPublishingPolicy: 'preserve-published-state', workflowIdPolicy: 'new', + missingNodeTypeMode: 'fail', folderConflictPolicy: 'merge', dataTableMatchingMode: 'by-id', dataTableMissingMode: 'create', 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 5c33fecaf98..8bba42c36fc 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 @@ -10,6 +10,7 @@ import { UnprocessableRequestError } from '@/errors/response-errors/unprocessabl import { createFolder } from '@test-integration/db/folders'; import { createOwner } from '@test-integration/db/users'; 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'; @@ -42,6 +43,7 @@ async function importFolders(params: FolderImportParams) { workflowConflictPolicy: 'new-version', workflowPublishingPolicy: 'preserve-published-state', workflowIdPolicy: 'new', + missingNodeTypeMode: 'fail', folderConflictPolicy: params.folderConflictPolicy ?? 'merge', dataTableMatchingMode: 'by-id', dataTableMissingMode: 'create', @@ -70,6 +72,9 @@ async function findWorkflow(id: string) { beforeAll(async () => { await testModules.loadModules(['n8n-packages']); await testDb.init(); + // Register node types so the plan-phase missing-node-type check can resolve + // the node types used by the package fixtures. + await initNodeTypes(); licenseMocker.mockLicenseState(Container.get(LicenseState)); licenseMocker.setDefaults({ features: ['feat:folders'] }); }); 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 ea3cf4aa738..a88dd1f0f14 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 @@ -21,6 +21,7 @@ import { Container } from '@n8n/di'; import { ActiveWorkflowManager } from '@/active-workflow-manager'; import { CredentialTypes } from '@/credential-types'; import { BadRequestError } from '@/errors/response-errors/bad-request.error'; +import { UnprocessableRequestError } from '@/errors/response-errors/unprocessable.error'; import { EventService } from '@/events/event.service'; import type { RelayEventMap } from '@/events/maps/relay.event-map'; import { @@ -40,6 +41,7 @@ import { DataTableMatchingMode, DataTableMissingMode, DataTableSchemaConflictPolicy, + MissingNodeTypeMode, WorkflowConflictPolicy, WorkflowIdPolicy, WorkflowPublishingPolicy, @@ -65,6 +67,7 @@ type ImportPackageParams = Omit< | 'workflowConflictPolicy' | 'workflowPublishingPolicy' | 'workflowIdPolicy' + | 'missingNodeTypeMode' | 'folderConflictPolicy' | 'dataTableMatchingMode' | 'dataTableMissingMode' @@ -80,6 +83,7 @@ type ImportPackageParams = Omit< | 'workflowConflictPolicy' | 'workflowPublishingPolicy' | 'workflowIdPolicy' + | 'missingNodeTypeMode' | 'folderConflictPolicy' | 'dataTableMatchingMode' | 'dataTableMissingMode' @@ -95,6 +99,7 @@ async function importPackage(params: ImportPackageParams) { workflowConflictPolicy: WorkflowConflictPolicy.Fail, workflowPublishingPolicy: WorkflowPublishingPolicy.PreservePublishedState, workflowIdPolicy: WorkflowIdPolicy.New, + missingNodeTypeMode: MissingNodeTypeMode.Fail, folderConflictPolicy: FolderConflictPolicy.Merge, dataTableMatchingMode: DataTableMatchingMode.ById, dataTableMissingMode: DataTableMissingMode.Create, @@ -2451,3 +2456,240 @@ describe('Package import workflow publishing policy', () => { expect(activeWorkflowManager.add).not.toHaveBeenCalled(); }); }); + +describe('Package import missing node type mode', () => { + const sourceId = 'missing-node-type-mode-test'; + + const scheduleTriggerNode = () => ({ + id: 'schedule-trigger', + name: 'Schedule Trigger', + type: 'n8n-nodes-base.scheduleTrigger', + typeVersion: 1, + position: [0, 0] as [number, number], + parameters: {}, + }); + + const unknownNode = () => ({ + id: 'unknown-node', + name: 'Unknown Node', + type: 'n8n-nodes-community.chatBot', + typeVersion: 1, + position: [200, 0] as [number, number], + parameters: {}, + }); + + const unresolvableCredentialNode = () => ({ + id: 'http-node', + name: 'HTTP Request', + type: 'n8n-nodes-base.httpRequest', + typeVersion: 1, + position: [400, 0] as [number, number], + parameters: {}, + credentials: { + [PACKAGE_GITHUB_CREDENTIAL_TYPE]: { id: 'missing-cred', name: 'Missing GitHub' }, + }, + }); + + it('fail (default) lists every missing pair and writes nothing', async () => { + const owner = await createOwner(); + + const importPromise = importPackage({ + user: owner, + // create-stub so the rollback assertion below proves no stub was written either. + credentialMissingMode: 'create-stub', + packageBuffer: await buildImportPackageBuffer( + [ + serializedWorkflow({ + id: 'wf-alpha', + name: 'Alpha', + nodes: [scheduleTriggerNode(), unknownNode()], + }), + serializedWorkflow({ + id: 'wf-beta', + name: 'Beta', + nodes: [scheduleTriggerNode(), unknownNode(), unresolvableCredentialNode()], + }), + serializedWorkflow({ + id: 'wf-gamma', + name: 'Gamma', + // Known node type at a version this instance does not have. + nodes: [{ ...serializedWorkflow().nodes[0], typeVersion: 9 }], + }), + ], + { sourceId }, + ), + }); + await expect(importPromise).rejects.toBeInstanceOf(UnprocessableRequestError); + await expect(importPromise).rejects.toMatchObject({ + meta: { + issues: expect.arrayContaining([ + { + type: 'missing-node-type', + nodeType: 'n8n-nodes-community.chatBot', + typeVersion: 1, + usedByWorkflows: ['wf-alpha', 'wf-beta'], + }, + { + type: 'missing-node-type', + nodeType: 'n8n-nodes-base.manualTrigger', + typeVersion: 9, + usedByWorkflows: ['wf-gamma'], + }, + ]), + }, + }); + + expect(await Container.get(WorkflowRepository).count()).toBe(0); + expect(await Container.get(CredentialsRepository).count()).toBe(0); + }); + + it.each([ + WorkflowPublishingPolicy.PublishAll, + WorkflowPublishingPolicy.MatchSource, + ])( + 'import-anyway blocks publishing only the affected workflows under "%s"', + async (workflowPublishingPolicy) => { + const owner = await createOwner(); + + const result = await importPackage({ + user: owner, + missingNodeTypeMode: 'import-anyway', + // The affected workflow also gets a stubbed credential: the reported + // reason must still be missing-node-type (it takes precedence). + credentialMissingMode: 'create-stub', + workflowPublishingPolicy, + packageBuffer: await buildImportPackageBuffer( + [ + serializedWorkflow({ + id: 'wf-ok', + name: 'Publishable', + isPublished: true, + nodes: [scheduleTriggerNode()], + }), + serializedWorkflow({ + id: 'wf-broken', + name: 'Missing node type', + isPublished: true, + nodes: [scheduleTriggerNode(), unknownNode(), unresolvableCredentialNode()], + }), + ], + { sourceId }, + ), + }); + + const ok = result.workflows.find(({ sourceWorkflowId }) => sourceWorkflowId === 'wf-ok'); + const broken = result.workflows.find( + ({ sourceWorkflowId }) => sourceWorkflowId === 'wf-broken', + ); + + expect(ok?.activeVersionId).toEqual(expect.any(String)); + expect(ok?.publishing).toEqual({ state: 'published' }); + expect(broken?.activeVersionId).toBeNull(); + expect(broken?.publishing).toEqual({ + state: 'blocked', + blockedReason: 'missing-node-type', + }); + }, + ); + + it.each(['fail', 'import-anyway'] as const)( + '"%s" imports normally and respects the publishing policy when nothing is missing', + async (missingNodeTypeMode) => { + const owner = await createOwner(); + + const result = await importPackage({ + user: owner, + missingNodeTypeMode, + workflowPublishingPolicy: WorkflowPublishingPolicy.PublishAll, + packageBuffer: await buildImportPackageBuffer( + [serializedWorkflow({ id: 'wf-fine', name: 'Fine', nodes: [scheduleTriggerNode()] })], + { sourceId }, + ), + }); + + expect(result.workflows[0]?.publishing).toEqual({ state: 'published' }); + expect(result.workflows[0]?.activeVersionId).toEqual(expect.any(String)); + }, + ); + + it('import-anyway keeps the prior published version active when an update has missing node types', async () => { + const owner = await createOwner(); + const personalProject = await Container.get(ProjectRepository).getPersonalProjectForUserOrFail( + owner.id, + ); + const active = await createActiveWorkflow({ name: 'Published workflow' }, personalProject); + await Container.get(WorkflowRepository).update(active.id, { + sourceWorkflowId: 'wf-broken-update', + }); + const originalActiveVersionId = active.activeVersionId; + expect(originalActiveVersionId).not.toBeNull(); + + const result = await importPackage({ + user: owner, + missingNodeTypeMode: 'import-anyway', + workflowConflictPolicy: 'new-version', + workflowPublishingPolicy: WorkflowPublishingPolicy.PreservePublishedState, + packageBuffer: await buildImportPackageBuffer( + [ + serializedWorkflow({ + id: 'wf-broken-update', + name: 'Published workflow updated', + isPublished: true, + nodes: [scheduleTriggerNode(), unknownNode()], + }), + ], + { sourceId }, + ), + }); + + const summary = result.workflows.find( + ({ sourceWorkflowId }) => sourceWorkflowId === 'wf-broken-update', + ); + + expect(summary?.status).toBe('updated'); + expect(summary?.activeVersionId).toBe(originalActiveVersionId); + expect(summary?.publishing).toEqual({ + state: 'unchanged', + skippedPublishReason: 'missing-node-type', + }); + + const stored = await Container.get(WorkflowRepository).findOneByOrFail({ id: active.id }); + expect(stored.activeVersionId).toBe(originalActiveVersionId); + }); + + it('fail ignores missing node types in workflows the conflict policy skips', async () => { + const owner = await createOwner(); + const personalProject = await Container.get(ProjectRepository).getPersonalProjectForUserOrFail( + owner.id, + ); + const existing = await createWorkflow({ name: 'Already there' }, personalProject); + await Container.get(WorkflowRepository).update(existing.id, { + sourceWorkflowId: 'wf-skipped-broken', + }); + + const result = await importPackage({ + user: owner, + workflowConflictPolicy: WorkflowConflictPolicy.Skip, + packageBuffer: await buildImportPackageBuffer( + [ + serializedWorkflow({ + id: 'wf-skipped-broken', + name: 'Already there', + nodes: [scheduleTriggerNode(), unknownNode()], + }), + serializedWorkflow({ id: 'wf-clean', name: 'Clean', nodes: [scheduleTriggerNode()] }), + ], + { sourceId }, + ), + }); + + expect(result.workflows).toHaveLength(2); + expect( + result.workflows.find(({ sourceWorkflowId }) => sourceWorkflowId === 'wf-skipped-broken') + ?.status, + ).toBe('skipped'); + expect( + result.workflows.find(({ sourceWorkflowId }) => sourceWorkflowId === 'wf-clean')?.status, + ).toBe('created'); + }); +}); 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 0eb1c729811..c7f5d399904 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 @@ -19,6 +19,7 @@ import type { RelayEventMap } from '@/events/maps/relay.event-map'; import { createOwner } from '@test-integration/db/users'; import { createProjectVariable, createVariable } from '@test-integration/db/variables'; import { LicenseMocker } from '@test-integration/license'; +import { initNodeTypes } from '@test-integration/utils'; import { N8nPackagesService } from '../n8n-packages.service'; import type { ImportPackageRequest } from '../n8n-packages.types'; @@ -46,6 +47,7 @@ async function importProjects( workflowConflictPolicy: 'new-version', workflowPublishingPolicy: 'preserve-published-state', workflowIdPolicy: 'new', + missingNodeTypeMode: 'fail', folderConflictPolicy: 'merge', dataTableMatchingMode: 'by-id', dataTableMissingMode: 'create', @@ -86,6 +88,9 @@ async function isAdminOf(projectId: string, userId: string): Promise { beforeAll(async () => { await testModules.loadModules(['n8n-packages']); await testDb.init(); + // Register node types so the plan-phase missing-node-type check can resolve + // the node types used by the package fixtures. + await initNodeTypes(); licenseMocker.mockLicenseState(Container.get(LicenseState)); licenseMocker.setDefaults({ features: ['feat:projectRole:admin', 'feat:folders'], @@ -443,6 +448,60 @@ describe('project shell import', () => { expect(await findProject('P2')).toBeNull(); }); + it('reports missing node types per project scope and writes nothing', async () => { + const unknownNode = { + id: 'unknown-node', + name: 'Unknown Node', + type: 'n8n-nodes-community.chatBot', + typeVersion: 1, + position: [0, 0] as [number, number], + parameters: {}, + }; + const packageBuffer = await buildEntityPackageBuffer({ + projects: [ + { target: 'projects/brie', project: serializedProject({ id: 'P1', name: 'brie' }) }, + { target: 'projects/stilton', project: serializedProject({ id: 'P2', name: 'stilton' }) }, + ], + workflows: [ + { + target: 'projects/brie/workflows/wfa', + workflow: serializedWorkflow({ id: 'WFA', name: 'wfa', nodes: [unknownNode] }), + }, + { + target: 'projects/stilton/workflows/wfb', + workflow: serializedWorkflow({ id: 'WFB', name: 'wfb', nodes: [unknownNode] }), + }, + ], + }); + + // Every project scope is planned before anything is written; the same missing + // pair yields one issue per scope, each with that scope's workflows. + const importPromise = importProjects(owner, packageBuffer); + await expect(importPromise).rejects.toBeInstanceOf(UnprocessableRequestError); + await expect(importPromise).rejects.toMatchObject({ + meta: { + issues: [ + { + type: 'missing-node-type', + nodeType: 'n8n-nodes-community.chatBot', + typeVersion: 1, + usedByWorkflows: ['WFA'], + }, + { + type: 'missing-node-type', + nodeType: 'n8n-nodes-community.chatBot', + typeVersion: 1, + usedByWorkflows: ['WFB'], + }, + ], + }, + }); + + expect(await findProject('P1')).toBeNull(); + expect(await findProject('P2')).toBeNull(); + expect(await Container.get(WorkflowRepository).count()).toBe(0); + }); + it('creates a new project under publish-all, planned before the project exists', async () => { const packageBuffer = await buildEntityPackageBuffer({ projects: [ 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 18fee8e86d7..9e5764b723b 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 @@ -8,6 +8,7 @@ import { VariablesService } from '@/environments.ee/variables/variables.service. import { createOwner } from '@test-integration/db/users'; import { createProjectVariable, createVariable } from '@test-integration/db/variables'; import { LicenseMocker } from '@test-integration/license'; +import { initNodeTypes } from '@test-integration/utils'; import { N8nPackagesService } from '../n8n-packages.service'; import type { ImportPackageRequest } from '../n8n-packages.types'; @@ -24,6 +25,7 @@ const licenseMocker = new LicenseMocker(); beforeAll(async () => { await testModules.loadModules(['n8n-packages']); await testDb.init(); + await initNodeTypes(); licenseMocker.mockLicenseState(Container.get(LicenseState)); service = Container.get(N8nPackagesService); variablesRepository = Container.get(VariablesRepository); @@ -62,6 +64,7 @@ async function importPackage(params: ImportParams) { dataTableMissingMode: 'create', dataTableSchemaConflictPolicy: 'keep-existing', variableMissingMode: 'do-nothing', + missingNodeTypeMode: 'fail', ...params, }); } diff --git a/packages/cli/src/modules/n8n-packages/engine/__tests__/import-telemetry.test.ts b/packages/cli/src/modules/n8n-packages/engine/__tests__/import-telemetry.test.ts index 714bfe3d0e0..65393b43e5a 100644 --- a/packages/cli/src/modules/n8n-packages/engine/__tests__/import-telemetry.test.ts +++ b/packages/cli/src/modules/n8n-packages/engine/__tests__/import-telemetry.test.ts @@ -87,6 +87,7 @@ const request = mock({ credentialMatchingMode: 'id-only', credentialMissingMode: 'create-stub', workflowPublishingPolicy: 'preserve-published-state', + missingNodeTypeMode: 'fail', }); const manifest = mock({ sourceId: 'src-1', packageFormatVersion: '1' }); diff --git a/packages/cli/src/modules/n8n-packages/engine/import-orchestrator.ts b/packages/cli/src/modules/n8n-packages/engine/import-orchestrator.ts index 286b2e5f5d7..b166c324a55 100644 --- a/packages/cli/src/modules/n8n-packages/engine/import-orchestrator.ts +++ b/packages/cli/src/modules/n8n-packages/engine/import-orchestrator.ts @@ -1,5 +1,7 @@ import { Service } from '@n8n/di'; +import { NodeTypes } from '@/node-types'; + import { toImportBlockedError } from './import-blocked.error'; import { CredentialImporter } from '../entities/credential/credential-importer'; import { workflowsBlockedFromPublish } from '../entities/credential/credential-missing-mode'; @@ -25,6 +27,12 @@ import type { VariableImportPlan, VariableImportRequest, } from '../entities/variable/variable.types'; +import { + collectMissingNodeTypes, + missingNodeTypeBlockingFailures, + workflowsWithMissingNodeTypes, + type MissingNodeTypeRequirement, +} from '../entities/workflow/missing-node-type-mode'; import type { PreparedWorkflow, WorkflowImportOutcome, @@ -32,6 +40,7 @@ import type { } from '../entities/workflow/workflow-import.types'; import { WorkflowImporter } from '../entities/workflow/workflow-importer'; import { WorkflowPublisher } from '../entities/workflow/workflow-publisher'; +import type { WorkflowPublishingBlockedReason } from '../entities/workflow/workflow-publishing-policy.types'; import { createBindings } from '../n8n-packages.types'; import type { BlockingIssue, @@ -39,6 +48,7 @@ import type { ImportedFolderSummary, ImportFolderProperties, ImportWorkflowProperties, + MissingNodeTypeMode, PackageImportBindings, } from '../n8n-packages.types'; @@ -71,6 +81,7 @@ export interface ImportPlan { folderPlan: FolderImportPlan; dataTablePlan: DataTableImportPlan; variablePlan: VariableImportPlan; + missingNodeTypes: MissingNodeTypeRequirement[]; blockingIssues: BlockingIssue[]; } @@ -87,6 +98,7 @@ export class ImportOrchestrator { private readonly folderImporter: FolderImporter, private readonly workflowImporter: WorkflowImporter, private readonly workflowPublisher: WorkflowPublisher, + private readonly nodeTypes: NodeTypes, ) {} async import(input: ImportOrchestrationInput): Promise { @@ -122,6 +134,12 @@ export class ImportOrchestrator { const folderContext = { ...context, folderConflictPolicy: options.folderConflictPolicy }; const folderPlan = await this.folderImporter.plan(folderContext, folders); + // Skipped workflows are never written, so their node types don't gate the import. + const missingNodeTypes = collectMissingNodeTypes( + workflowPlan.items.filter((item) => item.action !== 'skip'), + (nodeType) => this.nodeTypes.getSupportedVersions(nodeType), + ); + const blockingIssues = this.collectBlockingIssues({ workflowPlan, credentialPlan, @@ -130,6 +148,8 @@ export class ImportOrchestrator { dataTablePlan, variableRequest, variablePlan, + missingNodeTypes, + missingNodeTypeMode: options.missingNodeTypeMode, }); return { @@ -140,6 +160,7 @@ export class ImportOrchestrator { folderPlan, dataTablePlan, variablePlan, + missingNodeTypes, blockingIssues, }; } @@ -165,16 +186,23 @@ export class ImportOrchestrator { ); await this.dataTableImporter.apply(context, dataTablePlan); - const publishBlockedSourceWorkflowIds = workflowsBlockedFromPublish( + const publishBlocked = new Map(); + for (const sourceWorkflowId of workflowsBlockedFromPublish( credentialRequest.requirements, new Set(credentialResult.stubbed), - ); + )) { + publishBlocked.set(sourceWorkflowId, 'stub-credential'); + } + // A workflow blocked for both reasons reports missing-node-type: it physically can't run. + for (const sourceWorkflowId of workflowsWithMissingNodeTypes(plan.missingNodeTypes)) { + publishBlocked.set(sourceWorkflowId, 'missing-node-type'); + } const { outcomes, bindings } = await this.workflowImporter.apply( { ...context, publishingPolicy: options.workflowPublishingPolicy, - publishBlockedSourceWorkflowIds, + publishBlocked, }, workflowPlan, createBindings({ credentials: credentialResult.bindings }), @@ -198,6 +226,8 @@ export class ImportOrchestrator { dataTablePlan, variableRequest, variablePlan, + missingNodeTypes, + missingNodeTypeMode, }: { workflowPlan: WorkflowImportPlan; credentialPlan: CredentialResolution; @@ -206,6 +236,8 @@ export class ImportOrchestrator { dataTablePlan: DataTableImportPlan; variableRequest: VariableImportRequest; variablePlan: VariableImportPlan; + missingNodeTypes: MissingNodeTypeRequirement[]; + missingNodeTypeMode: MissingNodeTypeMode; }): BlockingIssue[] { return [ ...workflowPlan.conflicts.map( @@ -229,6 +261,14 @@ export class ImportOrchestrator { ...this.variableImporter .blockingFailures(variableRequest, variablePlan) .map((failure): BlockingIssue => ({ type: 'variable-unresolved', ...failure })), + ...missingNodeTypeBlockingFailures(missingNodeTypeMode, missingNodeTypes).map( + ({ type, typeVersion, usedByWorkflows }): BlockingIssue => ({ + type: 'missing-node-type', + nodeType: type, + typeVersion, + usedByWorkflows, + }), + ), ]; } } diff --git a/packages/cli/src/modules/n8n-packages/engine/import-result.ts b/packages/cli/src/modules/n8n-packages/engine/import-result.ts index 7ad4652248e..2ee7fe0187d 100644 --- a/packages/cli/src/modules/n8n-packages/engine/import-result.ts +++ b/packages/cli/src/modules/n8n-packages/engine/import-result.ts @@ -79,7 +79,9 @@ export function assertPackageImportApiKeyScopes( } } -/** Keeps only the requirements used by the imported workflows, trimming `usedByWorkflows` to match. */ +/** + * Keeps only the requirements used by the imported workflows, trimming `usedByWorkflows` to match. + */ export function identifyRequirements( requirements: T[] | undefined, workflows: PreparedWorkflow[], diff --git a/packages/cli/src/modules/n8n-packages/engine/import-telemetry.ts b/packages/cli/src/modules/n8n-packages/engine/import-telemetry.ts index e4ad1a8285c..2f44c8aa508 100644 --- a/packages/cli/src/modules/n8n-packages/engine/import-telemetry.ts +++ b/packages/cli/src/modules/n8n-packages/engine/import-telemetry.ts @@ -75,6 +75,7 @@ export function emitPackageImportedEvent( credentialMatchingMode: request.credentialMatchingMode, credentialMissingMode: request.credentialMissingMode, workflowPublishingPolicy: request.workflowPublishingPolicy, + missingNodeTypeMode: request.missingNodeTypeMode, dataTableMatchingMode: request.dataTableMatchingMode, dataTableMissingMode: request.dataTableMissingMode, dataTableSchemaConflictPolicy: request.dataTableSchemaConflictPolicy, diff --git a/packages/cli/src/modules/n8n-packages/entities/__tests__/requirements.types.test.ts b/packages/cli/src/modules/n8n-packages/entities/__tests__/requirements.types.test.ts index 86e2c337f87..488d86012bb 100644 --- a/packages/cli/src/modules/n8n-packages/entities/__tests__/requirements.types.test.ts +++ b/packages/cli/src/modules/n8n-packages/entities/__tests__/requirements.types.test.ts @@ -2,6 +2,7 @@ import type { WorkflowCredentialRequirement } from '../credential/credential.typ import type { WorkflowDataTableRequirement } from '../data-table/data-table.types'; import { mergeRequirements } from '../requirements.types'; import type { WorkflowVariableRequirement } from '../variable/variable.types'; +import type { WorkflowNodeTypeSource } from '../workflow/node-type-usage'; function cred(credentialId: string, workflowId: string): WorkflowCredentialRequirement { return { @@ -20,6 +21,10 @@ function dataTable(dataTableId: string, workflowId: string): WorkflowDataTableRe return { workflowId, dataTableId }; } +function nodeTypeSource(workflowId: string): WorkflowNodeTypeSource { + return { workflowId, nodes: [] }; +} + describe('mergeRequirements', () => { it('concatenates credential requirements across parts, preserving order', () => { const merged = mergeRequirements( @@ -27,17 +32,20 @@ describe('mergeRequirements', () => { credentials: [cred('c1', 'w1')], dataTables: [dataTable('dt1', 'w1')], variables: [variable('V1', 'w1')], + nodeTypes: [nodeTypeSource('w1')], }, { credentials: [cred('c2', 'w2'), cred('c3', 'w3')], dataTables: [dataTable('dt2', 'w2')], variables: [variable('V2', 'w2')], + nodeTypes: [nodeTypeSource('w2')], }, ); expect(merged.credentials).toEqual([cred('c1', 'w1'), cred('c2', 'w2'), cred('c3', 'w3')]); expect(merged.dataTables).toEqual([dataTable('dt1', 'w1'), dataTable('dt2', 'w2')]); expect(merged.variables).toEqual([variable('V1', 'w1'), variable('V2', 'w2')]); + expect(merged.nodeTypes).toEqual([nodeTypeSource('w1'), nodeTypeSource('w2')]); }); it('skips undefined parts so optional export results can be passed directly', () => { @@ -47,6 +55,7 @@ describe('mergeRequirements', () => { credentials: [cred('c1', 'w1')], dataTables: [dataTable('dt1', 'w1')], variables: [variable('V1', 'w1')], + nodeTypes: [nodeTypeSource('w1')], }, undefined, ); @@ -54,9 +63,15 @@ describe('mergeRequirements', () => { expect(merged.credentials).toEqual([cred('c1', 'w1')]); expect(merged.dataTables).toEqual([dataTable('dt1', 'w1')]); expect(merged.variables).toEqual([variable('V1', 'w1')]); + expect(merged.nodeTypes).toEqual([nodeTypeSource('w1')]); }); it('returns empty requirement lists when given no parts', () => { - expect(mergeRequirements()).toEqual({ credentials: [], dataTables: [], variables: [] }); + expect(mergeRequirements()).toEqual({ + credentials: [], + dataTables: [], + variables: [], + nodeTypes: [], + }); }); }); diff --git a/packages/cli/src/modules/n8n-packages/entities/folder/__tests__/folder.exporter.test.ts b/packages/cli/src/modules/n8n-packages/entities/folder/__tests__/folder.exporter.test.ts index 82ffda57b1a..3a63acb2677 100644 --- a/packages/cli/src/modules/n8n-packages/entities/folder/__tests__/folder.exporter.test.ts +++ b/packages/cli/src/modules/n8n-packages/entities/folder/__tests__/folder.exporter.test.ts @@ -71,6 +71,7 @@ describe('FolderExporter', () => { ], dataTables: [], variables: [], + nodeTypes: [], }, }); diff --git a/packages/cli/src/modules/n8n-packages/entities/requirements.types.ts b/packages/cli/src/modules/n8n-packages/entities/requirements.types.ts index 3a1a978d98e..b7dafdcc183 100644 --- a/packages/cli/src/modules/n8n-packages/entities/requirements.types.ts +++ b/packages/cli/src/modules/n8n-packages/entities/requirements.types.ts @@ -1,11 +1,14 @@ import type { WorkflowCredentialRequirement } from './credential/credential.types'; import type { WorkflowDataTableRequirement } from './data-table/data-table.types'; import type { WorkflowVariableRequirement } from './variable/variable.types'; +import type { WorkflowNodeTypeSource } from './workflow/node-type-usage'; export interface WorkflowExportRequirements { credentials: WorkflowCredentialRequirement[]; dataTables: WorkflowDataTableRequirement[]; variables: WorkflowVariableRequirement[]; + /** Per-workflow node lists; folded into unique pairs at manifest-assembly time. */ + nodeTypes: WorkflowNodeTypeSource[]; } export const mergeRequirements = ( @@ -14,4 +17,5 @@ export const mergeRequirements = ( credentials: parts.flatMap((part) => part?.credentials ?? []), dataTables: parts.flatMap((part) => part?.dataTables ?? []), variables: parts.flatMap((part) => part?.variables ?? []), + nodeTypes: parts.flatMap((part) => part?.nodeTypes ?? []), }); diff --git a/packages/cli/src/modules/n8n-packages/entities/workflow/__tests__/missing-node-type-mode.test.ts b/packages/cli/src/modules/n8n-packages/entities/workflow/__tests__/missing-node-type-mode.test.ts new file mode 100644 index 00000000000..ad09eb4f5a1 --- /dev/null +++ b/packages/cli/src/modules/n8n-packages/entities/workflow/__tests__/missing-node-type-mode.test.ts @@ -0,0 +1,123 @@ +import type { WorkflowEntity } from '@n8n/db'; +import type { INode } from 'n8n-workflow'; + +import { + collectMissingNodeTypes, + missingNodeTypeBlockingFailures, + workflowsWithMissingNodeTypes, +} from '../missing-node-type-mode'; +import type { PreparedWorkflow } from '../workflow-import.types'; + +const SUPPORTED_VERSIONS: Record = { + 'n8n-nodes-base.manualTrigger': [1], + 'n8n-nodes-base.set': [1, 2, 3], +}; + +const getSupportedVersions = (nodeType: string) => SUPPORTED_VERSIONS[nodeType]; + +function prepared( + sourceWorkflowId: string, + nodes: Array>, +): PreparedWorkflow { + return { + entity: { + nodes: nodes.map((node, index) => ({ + id: `n${index}`, + name: `Node ${index}`, + position: [0, 0], + parameters: {}, + ...node, + })), + } as WorkflowEntity, + sourceWorkflowId, + sourcePublished: false, + parentFolderId: null, + }; +} + +describe('collectMissingNodeTypes', () => { + it('returns nothing for an empty workflows array', () => { + expect(collectMissingNodeTypes([], getSupportedVersions)).toEqual([]); + }); + + it('returns nothing when every node type and version is supported', () => { + const workflows = [ + prepared('wf-1', [ + { type: 'n8n-nodes-base.manualTrigger', typeVersion: 1 }, + { type: 'n8n-nodes-base.set', typeVersion: 3 }, + ]), + ]; + + expect(collectMissingNodeTypes(workflows, getSupportedVersions)).toEqual([]); + }); + + it('reports an unknown node type as missing', () => { + const workflows = [prepared('wf-1', [{ type: 'n8n-nodes-base.unknown', typeVersion: 2 }])]; + + expect(collectMissingNodeTypes(workflows, getSupportedVersions)).toEqual([ + { type: 'n8n-nodes-base.unknown', typeVersion: 2, usedByWorkflows: ['wf-1'] }, + ]); + }); + + it('reports a known node type at an unsupported version as missing', () => { + const workflows = [prepared('wf-1', [{ type: 'n8n-nodes-base.set', typeVersion: 4 }])]; + + expect(collectMissingNodeTypes(workflows, getSupportedVersions)).toEqual([ + { type: 'n8n-nodes-base.set', typeVersion: 4, usedByWorkflows: ['wf-1'] }, + ]); + }); + + it('folds duplicate pairs into one requirement with merged workflow ids', () => { + const workflows = [ + prepared('wf-1', [ + { type: 'n8n-nodes-base.unknown', typeVersion: 1 }, + // The same pair twice within one workflow must not duplicate the id. + { type: 'n8n-nodes-base.unknown', typeVersion: 1 }, + ]), + prepared('wf-2', [ + { type: 'n8n-nodes-base.unknown', typeVersion: 1 }, + { type: 'n8n-nodes-base.unknown', typeVersion: 2 }, + ]), + ]; + + expect(collectMissingNodeTypes(workflows, getSupportedVersions)).toEqual([ + { type: 'n8n-nodes-base.unknown', typeVersion: 1, usedByWorkflows: ['wf-1', 'wf-2'] }, + { type: 'n8n-nodes-base.unknown', typeVersion: 2, usedByWorkflows: ['wf-2'] }, + ]); + }); + + it('dedupes a workflow id that reappears non-adjacently in the input', () => { + const workflows = [ + prepared('wf-1', [{ type: 'n8n-nodes-base.unknown', typeVersion: 1 }]), + prepared('wf-2', [{ type: 'n8n-nodes-base.unknown', typeVersion: 1 }]), + prepared('wf-1', [{ type: 'n8n-nodes-base.unknown', typeVersion: 1 }]), + ]; + + expect(collectMissingNodeTypes(workflows, getSupportedVersions)).toEqual([ + { type: 'n8n-nodes-base.unknown', typeVersion: 1, usedByWorkflows: ['wf-1', 'wf-2'] }, + ]); + }); +}); + +describe('missingNodeTypeBlockingFailures', () => { + const missing = [{ type: 'n8n-nodes-base.unknown', typeVersion: 1, usedByWorkflows: ['wf-1'] }]; + + it('fail treats every missing pair as blocking', () => { + expect(missingNodeTypeBlockingFailures('fail', missing)).toEqual(missing); + }); + + it('import-anyway treats nothing as blocking', () => { + expect(missingNodeTypeBlockingFailures('import-anyway', missing)).toEqual([]); + }); +}); + +describe('workflowsWithMissingNodeTypes', () => { + it('unions the workflow ids across every missing pair', () => { + const blocked = workflowsWithMissingNodeTypes([ + { type: 'n8n-nodes-base.unknown', typeVersion: 1, usedByWorkflows: ['wf-1', 'wf-2'] }, + { type: 'n8n-nodes-base.other', typeVersion: 2, usedByWorkflows: ['wf-2', 'wf-3'] }, + ]); + + expect(blocked).toEqual(new Set(['wf-1', 'wf-2', 'wf-3'])); + }); +}); diff --git a/packages/cli/src/modules/n8n-packages/entities/workflow/__tests__/workflow-publisher.test.ts b/packages/cli/src/modules/n8n-packages/entities/workflow/__tests__/workflow-publisher.test.ts index a0900e07084..77911d5f136 100644 --- a/packages/cli/src/modules/n8n-packages/entities/workflow/__tests__/workflow-publisher.test.ts +++ b/packages/cli/src/modules/n8n-packages/entities/workflow/__tests__/workflow-publisher.test.ts @@ -102,6 +102,7 @@ describe('WorkflowPublisher', () => { createItem(false), workflow, WorkflowPublishingPolicy.PreservePublishedState, + new Map(), ); expect(result.workflow).toBe(workflow); @@ -125,6 +126,7 @@ describe('WorkflowPublisher', () => { createItem(true), workflow, WorkflowPublishingPolicy.PublishAll, + new Map(), ); expect(workflowService.activateWorkflow).toHaveBeenCalledWith(user, 'wf-1', { @@ -149,6 +151,7 @@ describe('WorkflowPublisher', () => { createItem(true), workflow, WorkflowPublishingPolicy.PublishAll, + new Map(), ); expect(result.workflow).toBe(workflow); @@ -186,7 +189,7 @@ describe('WorkflowPublisher', () => { updateItem, workflow, WorkflowPublishingPolicy.MatchSource, - new Set(['wf-stubbed']), + new Map([['wf-stubbed', 'stub-credential']]), ); expect(workflowService.activateWorkflow).not.toHaveBeenCalled(); @@ -210,7 +213,7 @@ describe('WorkflowPublisher', () => { createItem(true), workflow, WorkflowPublishingPolicy.PublishAll, - new Set(['wf-1']), + new Map([['wf-1', 'stub-credential']]), ); expect(workflowService.activateWorkflow).not.toHaveBeenCalled(); @@ -243,7 +246,7 @@ describe('WorkflowPublisher', () => { updateItem, workflow, WorkflowPublishingPolicy.PreservePublishedState, - new Set(['wf-stubbed']), + new Map([['wf-stubbed', 'stub-credential']]), ); expect(workflowService.activateWorkflow).not.toHaveBeenCalled(); @@ -253,5 +256,40 @@ describe('WorkflowPublisher', () => { skippedPublishReason: 'stub-credential', }); }); + + it('still unpublishes a blocked workflow under unpublish-all', async () => { + const workflow = mock({ + id: 'wf-1', + versionId: 'v2', + activeVersionId: 'v1', + isArchived: false, + }); + const unpublished = mock({ id: 'wf-1', activeVersionId: null }); + workflowService.deactivateWorkflow.mockResolvedValue(unpublished); + + const updateItem: PersistedWorkflowPlanItem = { + action: 'update', + sourceWorkflowId: 'wf-broken', + sourcePublished: true, + parentFolderId: null, + entity: mock(), + existing: mock({ id: 'wf-1' }), + }; + + const result = await publisher.apply( + user, + updateItem, + workflow, + WorkflowPublishingPolicy.UnpublishAll, + new Map([['wf-broken', 'missing-node-type']]), + ); + + expect(workflowService.activateWorkflow).not.toHaveBeenCalled(); + expect(workflowService.deactivateWorkflow).toHaveBeenCalledWith(user, 'wf-1', { + source: 'import', + }); + expect(result.workflow).toBe(unpublished); + expect(result.publishing).toEqual({ state: 'unpublished' }); + }); }); }); diff --git a/packages/cli/src/modules/n8n-packages/entities/workflow/__tests__/workflow.exporter.test.ts b/packages/cli/src/modules/n8n-packages/entities/workflow/__tests__/workflow.exporter.test.ts index 4871ad9defa..a0e3fee43d2 100644 --- a/packages/cli/src/modules/n8n-packages/entities/workflow/__tests__/workflow.exporter.test.ts +++ b/packages/cli/src/modules/n8n-packages/entities/workflow/__tests__/workflow.exporter.test.ts @@ -1,5 +1,6 @@ import type { User, WorkflowEntity } from '@n8n/db'; import { jsonParse } from 'n8n-workflow'; +import type { INode } from 'n8n-workflow'; import { mock } from 'vitest-mock-extended'; import type { WorkflowFinderService } from '@/workflows/workflow-finder.service'; @@ -311,4 +312,30 @@ describe('WorkflowExporter', () => { { workflowId: 'wf-b', variableName: 'VAR_FROM_wf-b' }, ]); }); + + it('collects each workflow node list into requirements.nodeTypes', async () => { + const nodeA: INode = { + id: 'n1', + name: 'HTTP', + type: 'n8n-nodes-base.httpRequest', + typeVersion: 1, + position: [0, 0], + parameters: {}, + }; + const a = makeWorkflow({ id: 'wf-a', nodes: [nodeA] }); + const b = makeWorkflow({ id: 'wf-b' }); + const { exporter } = makeExporter([a, b]); + const writer = new CapturingWriter(); + + const { requirements } = await exporter.export({ + user, + workflowIds: [a.id, b.id], + writer, + }); + + expect(requirements.nodeTypes).toEqual([ + { workflowId: 'wf-a', nodes: [nodeA] }, + { workflowId: 'wf-b', nodes: [] }, + ]); + }); }); diff --git a/packages/cli/src/modules/n8n-packages/entities/workflow/missing-node-type-mode.ts b/packages/cli/src/modules/n8n-packages/entities/workflow/missing-node-type-mode.ts new file mode 100644 index 00000000000..108b2b2e72a --- /dev/null +++ b/packages/cli/src/modules/n8n-packages/entities/workflow/missing-node-type-mode.ts @@ -0,0 +1,52 @@ +import { collectNodeTypeUsage, type NodeTypeUsage } from './node-type-usage'; +import type { PreparedWorkflow } from './workflow-import.types'; +import type { MissingNodeTypeMode } from '../../n8n-packages.types'; + +/** A `(type, typeVersion)` pair the target instance cannot resolve. Field names align with the manifest requirements shape. */ +export type MissingNodeTypeRequirement = NodeTypeUsage; + +/** + * Folds the packaged workflows' nodes into unique `(type, typeVersion)` pairs + * and returns the ones the instance cannot resolve. Read-only. + */ +export function collectMissingNodeTypes( + workflows: PreparedWorkflow[], + getSupportedVersions: (nodeType: string) => number[] | undefined, +): MissingNodeTypeRequirement[] { + const usage = collectNodeTypeUsage( + workflows.map(({ entity, sourceWorkflowId }) => ({ + workflowId: sourceWorkflowId, + nodes: entity.nodes, + })), + ); + + const supportedByType = new Map(); + return usage.filter(({ type, typeVersion }) => { + if (!supportedByType.has(type)) { + supportedByType.set(type, getSupportedVersions(type)); + } + return !supportedByType.get(type)?.includes(typeVersion); + }); +} + +/* eslint-disable @typescript-eslint/naming-convention -- API missing node type mode keys */ +const BLOCKING_FAILURES: Record< + MissingNodeTypeMode, + (missing: MissingNodeTypeRequirement[]) => MissingNodeTypeRequirement[] +> = { + fail: (missing) => missing, + 'import-anyway': () => [], +}; +/* eslint-enable @typescript-eslint/naming-convention */ + +export function missingNodeTypeBlockingFailures( + mode: MissingNodeTypeMode, + missing: MissingNodeTypeRequirement[], +): MissingNodeTypeRequirement[] { + return BLOCKING_FAILURES[mode](missing); +} + +/** Package workflow ids that should not be published because they use missing node types. */ +export function workflowsWithMissingNodeTypes(missing: MissingNodeTypeRequirement[]): Set { + return new Set(missing.flatMap(({ usedByWorkflows }) => usedByWorkflows)); +} diff --git a/packages/cli/src/modules/n8n-packages/entities/workflow/node-type-usage.ts b/packages/cli/src/modules/n8n-packages/entities/workflow/node-type-usage.ts new file mode 100644 index 00000000000..cfd30117e59 --- /dev/null +++ b/packages/cli/src/modules/n8n-packages/entities/workflow/node-type-usage.ts @@ -0,0 +1,46 @@ +import type { INode } from 'n8n-workflow'; + +/** One workflow's node list, keyed by the id the usage entries should reference. */ +export interface WorkflowNodeTypeSource { + workflowId: string; + nodes: INode[]; +} + +/** A unique `(type, typeVersion)` pair and the workflows that use it. Matches the manifest requirements shape. */ +export interface NodeTypeUsage { + type: string; + typeVersion: number; + usedByWorkflows: string[]; +} + +/** + * Folds every node of the given workflows into unique `(type, typeVersion)` + * pairs with deduped `usedByWorkflows`. Disabled nodes still count — they + * render on canvas and are part of the content. Pure — no registry lookups. + */ +export function collectNodeTypeUsage(workflows: WorkflowNodeTypeSource[]): NodeTypeUsage[] { + const usage = new Map }>(); + + for (const { workflowId, nodes } of workflows) { + for (const node of nodes) { + const key = `${node.type}@${node.typeVersion}`; + const entry = usage.get(key); + if (entry) { + entry.workflowIds.add(workflowId); + continue; + } + + usage.set(key, { + type: node.type, + typeVersion: node.typeVersion, + workflowIds: new Set([workflowId]), + }); + } + } + + return [...usage.values()].map(({ type, typeVersion, workflowIds }) => ({ + type, + typeVersion, + usedByWorkflows: [...workflowIds], + })); +} diff --git a/packages/cli/src/modules/n8n-packages/entities/workflow/workflow-import.types.ts b/packages/cli/src/modules/n8n-packages/entities/workflow/workflow-import.types.ts index 38d78ad7c63..59fe8c97297 100644 --- a/packages/cli/src/modules/n8n-packages/entities/workflow/workflow-import.types.ts +++ b/packages/cli/src/modules/n8n-packages/entities/workflow/workflow-import.types.ts @@ -2,6 +2,7 @@ import type { WorkflowEntity } from '@n8n/db'; import type { WorkflowIdConflict } from './workflow-import-match.service'; import type { + WorkflowPublishingBlockedReason, WorkflowPublishingOutcome, WorkflowPublishingPolicy, } from './workflow-publishing-policy.types'; @@ -10,8 +11,8 @@ import type { ImportContext } from '../../n8n-packages.types'; /** Apply-time context for the workflow importer: the resolved import target plus apply-only inputs. */ export interface WorkflowImportContext extends ImportContext { publishingPolicy: WorkflowPublishingPolicy; - /** Package workflow ids that must stay inactive because they use stubbed credentials. */ - publishBlockedSourceWorkflowIds: ReadonlySet; + /** Package workflow ids that must stay inactive, mapped to the reason why. */ + publishBlocked: ReadonlyMap; } export interface PreparedWorkflow { diff --git a/packages/cli/src/modules/n8n-packages/entities/workflow/workflow-importer.ts b/packages/cli/src/modules/n8n-packages/entities/workflow/workflow-importer.ts index f7dcbc70036..2d823080458 100644 --- a/packages/cli/src/modules/n8n-packages/entities/workflow/workflow-importer.ts +++ b/packages/cli/src/modules/n8n-packages/entities/workflow/workflow-importer.ts @@ -171,7 +171,7 @@ export class WorkflowImporter { item, savedWorkflow, context.publishingPolicy, - context.publishBlockedSourceWorkflowIds, + context.publishBlocked, ); // Publish reloads the workflow without parentFolder; restore it for the import summary. diff --git a/packages/cli/src/modules/n8n-packages/entities/workflow/workflow-publisher.ts b/packages/cli/src/modules/n8n-packages/entities/workflow/workflow-publisher.ts index 9aaa296ed57..da6e1931c42 100644 --- a/packages/cli/src/modules/n8n-packages/entities/workflow/workflow-publisher.ts +++ b/packages/cli/src/modules/n8n-packages/entities/workflow/workflow-publisher.ts @@ -13,6 +13,7 @@ import type { PersistedWorkflowPlanItem } from './workflow-import.types'; import { decideWorkflowPublishingAction } from './workflow-publishing-policy'; import { WorkflowPublishingPolicy, + type WorkflowPublishingBlockedReason, type WorkflowPublishingContext, type WorkflowPublishingOutcome, } from './workflow-publishing-policy.types'; @@ -81,7 +82,7 @@ export class WorkflowPublisher { item: PersistedWorkflowPlanItem, workflow: WorkflowEntity, policy: WorkflowPublishingPolicy, - publishBlockedSourceWorkflowIds?: ReadonlySet, + publishBlocked: ReadonlyMap, ): Promise { const action = decideWorkflowPublishingAction(policy, toPublishingContext(item, workflow)); @@ -89,7 +90,8 @@ export class WorkflowPublisher { return { workflow, publishing: { state: 'unchanged' } }; } - if (action === 'publish' && publishBlockedSourceWorkflowIds?.has(item.sourceWorkflowId)) { + const blockedReason = publishBlocked.get(item.sourceWorkflowId); + if (action === 'publish' && blockedReason) { // A prior published version may still be active after an update; report // that the live publish state is unchanged rather than "blocked". if (workflow.activeVersionId) { @@ -97,14 +99,14 @@ export class WorkflowPublisher { workflow, publishing: { state: 'unchanged', - skippedPublishReason: 'stub-credential', + skippedPublishReason: blockedReason, }, }; } return { workflow, - publishing: { state: 'blocked', blockedReason: 'stub-credential' }, + publishing: { state: 'blocked', blockedReason }, }; } diff --git a/packages/cli/src/modules/n8n-packages/entities/workflow/workflow-publishing-policy.types.ts b/packages/cli/src/modules/n8n-packages/entities/workflow/workflow-publishing-policy.types.ts index 2cc113bdbf9..d74afce7556 100644 --- a/packages/cli/src/modules/n8n-packages/entities/workflow/workflow-publishing-policy.types.ts +++ b/packages/cli/src/modules/n8n-packages/entities/workflow/workflow-publishing-policy.types.ts @@ -23,7 +23,7 @@ export type WorkflowPublishingOutcomeState = | 'blocked' | 'failed'; -export type WorkflowPublishingBlockedReason = 'stub-credential'; +export type WorkflowPublishingBlockedReason = 'stub-credential' | 'missing-node-type'; /** Result of applying a publishing policy to one imported workflow. */ export interface WorkflowPublishingOutcome { diff --git a/packages/cli/src/modules/n8n-packages/entities/workflow/workflow.exporter.ts b/packages/cli/src/modules/n8n-packages/entities/workflow/workflow.exporter.ts index e37a6f98d64..c49b2c837f9 100644 --- a/packages/cli/src/modules/n8n-packages/entities/workflow/workflow.exporter.ts +++ b/packages/cli/src/modules/n8n-packages/entities/workflow/workflow.exporter.ts @@ -11,6 +11,7 @@ import { CredentialRequirementsExtractor } from '../credential/credential-requir import type { WorkflowCredentialRequirement } from '../credential/credential.types'; import { DataTableRequirementsExtractor } from '../data-table/data-table-requirements.extractor'; import type { WorkflowDataTableRequirement } from '../data-table/data-table.types'; +import type { WorkflowNodeTypeSource } from './node-type-usage'; import { assertEveryRequestedEntityAccessible } from '../package-export.errors'; import type { WorkflowExportRequirements } from '../requirements.types'; import { VariableRequirementsExtractor } from '../variable/variable-requirements.extractor'; @@ -60,6 +61,7 @@ export class WorkflowExporter { const credentials: WorkflowCredentialRequirement[] = []; const dataTables: WorkflowDataTableRequirement[] = []; const variables: WorkflowVariableRequirement[] = []; + const nodeTypes: WorkflowNodeTypeSource[] = []; const fileNames = new UniqueFilenameAllocator( request.basePrefix ? `${request.basePrefix}/workflows` : 'workflows', 'workflow', @@ -81,9 +83,10 @@ export class WorkflowExporter { credentials.push(...this.credentialRequirementsExtractor.extract(workflow)); dataTables.push(...this.dataTableRequirementsExtractor.extract(workflow)); variables.push(...this.variableRequirementsExtractor.extract(workflow)); + nodeTypes.push({ workflowId: workflow.id, nodes: workflow.nodes ?? [] }); } - return { entries, requirements: { credentials, dataTables, variables } }; + return { entries, requirements: { credentials, dataTables, variables, nodeTypes } }; } private orderWorkflowsByRequest( diff --git a/packages/cli/src/modules/n8n-packages/n8n-packages.service.ts b/packages/cli/src/modules/n8n-packages/n8n-packages.service.ts index 856bb2da6c8..444650ab8c7 100644 --- a/packages/cli/src/modules/n8n-packages/n8n-packages.service.ts +++ b/packages/cli/src/modules/n8n-packages/n8n-packages.service.ts @@ -16,6 +16,7 @@ import { PackageExportBlockedError } from './entities/package-export.errors'; import { ProjectExporter } from './entities/project/project.exporter'; import { mergeRequirements } from './entities/requirements.types'; import { VariableExporter } from './entities/variable/variable.exporter'; +import { collectNodeTypeUsage } from './entities/workflow/node-type-usage'; import { assertStaticSubWorkflowsIncluded } from './entities/workflow/static-sub-workflow-requirements'; import { WorkflowDependencyResolver } from './entities/workflow/workflow-dependency-resolver'; import { WorkflowRequirementExporter } from './entities/workflow/workflow-requirement.exporter'; @@ -176,6 +177,7 @@ export class N8nPackagesService { dataTables: dataTableExportResult.requirements, workflows: workflowRequirementExportResult.requirements, variables: variableExportResult.requirements, + nodeTypes: collectNodeTypeUsage(requirements.nodeTypes), }); const manifest = packageManifestSchema.parse({ @@ -244,14 +246,16 @@ export class N8nPackagesService { dataTables: PackageRequirements['dataTables']; workflows: PackageRequirements['workflows']; variables: PackageRequirements['variables']; + nodeTypes: PackageRequirements['nodeTypes']; }): PackageRequirements | undefined { - const { credentials, dataTables, workflows, variables } = input; + const { credentials, dataTables, workflows, variables, nodeTypes } = input; const requirements: PackageRequirements = { ...(credentials?.length ? { credentials } : {}), ...(dataTables?.length ? { dataTables } : {}), ...(workflows?.length ? { workflows } : {}), ...(variables?.length ? { variables } : {}), + ...(nodeTypes?.length ? { nodeTypes } : {}), }; return Object.keys(requirements).length > 0 ? requirements : undefined; } 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 d53ac5c0a54..3de9c197ad7 100644 --- a/packages/cli/src/modules/n8n-packages/n8n-packages.types.ts +++ b/packages/cli/src/modules/n8n-packages/n8n-packages.types.ts @@ -45,6 +45,13 @@ export const FolderConflictPolicy = { Fail: 'fail', } as const; +export const MissingNodeTypeMode = { + /** Fails the import when any workflow uses a node type or version this instance does not have. */ + Fail: 'fail', + /** Imports anyway; workflows containing missing node types are never published. */ + ImportAnyway: 'import-anyway', +} as const; + export const MissingWorkflowDependencyPolicy = { /** Fails the export when a workflow dependency is not included. */ Fail: 'fail', @@ -90,6 +97,8 @@ export type WorkflowIdPolicy = (typeof WorkflowIdPolicy)[keyof typeof WorkflowId export type FolderConflictPolicy = (typeof FolderConflictPolicy)[keyof typeof FolderConflictPolicy]; +export type MissingNodeTypeMode = (typeof MissingNodeTypeMode)[keyof typeof MissingNodeTypeMode]; + export type MissingWorkflowDependencyPolicy = (typeof MissingWorkflowDependencyPolicy)[keyof typeof MissingWorkflowDependencyPolicy]; @@ -135,6 +144,7 @@ export type ImportWorkflowProperties = { workflowConflictPolicy: WorkflowConflictPolicy; workflowPublishingPolicy: WorkflowPublishingPolicy; workflowIdPolicy: WorkflowIdPolicy; + missingNodeTypeMode: MissingNodeTypeMode; }; export type ImportFolderProperties = { @@ -260,7 +270,14 @@ export type BlockingIssue = } | ({ type: 'folder-conflict' } & FolderConflict) | ({ type: 'data-table-unresolved' } & DataTableResolutionFailure) - | ({ type: 'variable-unresolved' } & VariableResolutionFailure); + | ({ type: 'variable-unresolved' } & VariableResolutionFailure) + | { + type: 'missing-node-type'; + /** Node type this instance cannot resolve (at least not at `typeVersion`). */ + nodeType: string; + typeVersion: number; + usedByWorkflows: string[]; + }; export interface FolderConflict { kind: 'parent-mismatch' | 'id-in-other-project' | 'fail-policy'; diff --git a/packages/cli/src/modules/n8n-packages/spec/__tests__/requirements.schema.test.ts b/packages/cli/src/modules/n8n-packages/spec/__tests__/requirements.schema.test.ts index 8f2a7dc44bb..5e017ab980b 100644 --- a/packages/cli/src/modules/n8n-packages/spec/__tests__/requirements.schema.test.ts +++ b/packages/cli/src/modules/n8n-packages/spec/__tests__/requirements.schema.test.ts @@ -8,11 +8,17 @@ describe('packageRequirementsSchema', () => { usedByWorkflows: ['wf-1'], }); const variable = (name: string) => ({ name, usedByWorkflows: ['wf-1'] }); + const nodeType = (type: string, typeVersion: number) => ({ + type, + typeVersion, + usedByWorkflows: ['wf-1'], + }); it('accepts distinct entries per section', () => { const requirements = { credentials: [credential('cred-1'), credential('cred-2')], variables: [variable('API_URL'), variable('REGION')], + nodeTypes: [nodeType('n8n-nodes-base.set', 3), nodeType('n8n-nodes-base.set', 3.4)], }; expect(() => packageRequirementsSchema.parse(requirements)).not.toThrow(); @@ -33,4 +39,20 @@ describe('packageRequirementsSchema', () => { /Duplicate variable name: API_URL/, ); }); + + it('rejects duplicate node type pairs', () => { + const requirements = { + nodeTypes: [nodeType('n8n-nodes-base.set', 3), nodeType('n8n-nodes-base.set', 3)], + }; + + expect(() => packageRequirementsSchema.parse(requirements)).toThrow( + /Duplicate node type: n8n-nodes-base.set@3/, + ); + }); + + it('rejects a non-finite node type version', () => { + const requirements = { nodeTypes: [nodeType('n8n-nodes-base.set', Infinity)] }; + + expect(() => packageRequirementsSchema.parse(requirements)).toThrow(); + }); }); diff --git a/packages/cli/src/modules/n8n-packages/spec/requirements.schema.ts b/packages/cli/src/modules/n8n-packages/spec/requirements.schema.ts index 1d34e20b97e..03cf5a0de66 100644 --- a/packages/cli/src/modules/n8n-packages/spec/requirements.schema.ts +++ b/packages/cli/src/modules/n8n-packages/spec/requirements.schema.ts @@ -19,6 +19,16 @@ export const packageWorkflowRequirementSchema = z.object({ usedByWorkflows: z.array(z.string().min(1)).min(1), }); +// Node types used by the packaged workflows, folded into unique +// `(type, typeVersion)` pairs. Informational/derived only: import re-derives +// node type usage from workflow content and never trusts this section. +export const packageNodeTypeRequirementSchema = z.object({ + type: z.string().min(1), + // `finite()`: JSON like `1e999` parses to Infinity (mirrors the workflow node schema). + typeVersion: z.number().finite(), + usedByWorkflows: z.array(z.string().min(1)).min(1), +}); + // Variables are keyed by name, not id: a `$vars.` reference resolves // project-scope-first then global at runtime, so one requirement may be // satisfied by different rows on different instances — no single portable id @@ -74,10 +84,22 @@ export const packageRequirementsSchema = z.object({ .superRefine((variables, ctx) => assertNoDuplicateKey(variables, ({ name }) => name, 'variable name', ctx), ), + nodeTypes: z + .array(packageNodeTypeRequirementSchema) + .optional() + .superRefine((nodeTypes, ctx) => + assertNoDuplicateKey( + nodeTypes, + ({ type, typeVersion }) => `${type}@${typeVersion}`, + 'node type', + ctx, + ), + ), }); export type PackageCredentialRequirement = z.infer; export type PackageDataTableRequirement = z.infer; export type PackageWorkflowRequirement = z.infer; export type PackageVariableRequirement = z.infer; +export type PackageNodeTypeRequirement = z.infer; export type PackageRequirements = z.infer; diff --git a/packages/cli/src/modules/n8n-packages/spec/serialized/__tests__/workflow.schema.test.ts b/packages/cli/src/modules/n8n-packages/spec/serialized/__tests__/workflow.schema.test.ts new file mode 100644 index 00000000000..c8eaefd406a --- /dev/null +++ b/packages/cli/src/modules/n8n-packages/spec/serialized/__tests__/workflow.schema.test.ts @@ -0,0 +1,31 @@ +import { serializedWorkflowSchema } from '../workflow.schema'; + +describe('serializedWorkflowSchema', () => { + const workflow = (typeVersion: number) => ({ + id: 'wf-1', + name: 'Workflow', + nodes: [ + { + id: 'n1', + name: 'Manual Trigger', + type: 'n8n-nodes-base.manualTrigger', + typeVersion, + position: [0, 0], + parameters: {}, + }, + ], + connections: {}, + versionId: 'v1', + parentFolderId: null, + isPublished: false, + isArchived: false, + }); + + it('accepts a finite node typeVersion', () => { + expect(() => serializedWorkflowSchema.parse(workflow(1.2))).not.toThrow(); + }); + + it('rejects a non-finite node typeVersion (JSON `1e999` parses to Infinity)', () => { + expect(() => serializedWorkflowSchema.parse(workflow(Infinity))).toThrow(); + }); +}); diff --git a/packages/cli/src/modules/n8n-packages/spec/serialized/workflow.schema.ts b/packages/cli/src/modules/n8n-packages/spec/serialized/workflow.schema.ts index ca33d332946..bb699c90f0b 100644 --- a/packages/cli/src/modules/n8n-packages/spec/serialized/workflow.schema.ts +++ b/packages/cli/src/modules/n8n-packages/spec/serialized/workflow.schema.ts @@ -13,7 +13,9 @@ const nodeSchema = z.object({ id: z.string().min(1), name: z.string().min(1), type: z.string().min(1), - typeVersion: z.number(), + // `finite()`: JSON like `1e999` parses to Infinity, which would serialize to + // `null` in a missing-node-type issue and break the OpenAPI number contract. + typeVersion: z.number().finite(), position: z.tuple([z.number(), z.number()]), parameters: z.record(z.unknown()), credentials: z.record(credentialReferenceSchema).optional(), diff --git a/packages/cli/src/node-types.ts b/packages/cli/src/node-types.ts index badb80dc538..3cf9945abe0 100644 --- a/packages/cli/src/node-types.ts +++ b/packages/cli/src/node-types.ts @@ -1,20 +1,42 @@ +import { Logger } from '@n8n/backend-common'; import { Service } from '@n8n/di'; import type { NeededNodeType } from '@n8n/task-runner'; +import { ensureError } from '@n8n/utils/errors/ensure-error'; import type { Dirent } from 'fs'; import { readdir, readFile } from 'fs/promises'; -import { RoutingNode } from 'n8n-core'; +import { RoutingNode, UnrecognizedNodeTypeError } from 'n8n-core'; import type { ExecuteContext } from 'n8n-core'; import type { INodeType, INodeTypeDescription, INodeTypes, IVersionedNodeType } from 'n8n-workflow'; -import { deepCopy, NodeHelpers, UnexpectedError, UserError } from 'n8n-workflow'; +import { deepCopy, isHitlToolType, NodeHelpers, UnexpectedError, UserError } from 'n8n-workflow'; import { join, dirname } from 'path'; import { LoadNodesAndCredentials } from './load-nodes-and-credentials'; import { convertNodeToAiTool, convertNodeToHitlTool } from './tool-generation'; -import { shouldAssignExecuteMethod, stripToolSuffix } from './utils'; +import { satisfiesToolCapability, shouldAssignExecuteMethod, stripToolSuffix } from './utils'; @Service() export class NodeTypes implements INodeTypes { - constructor(private readonly loadNodesAndCredentials: LoadNodesAndCredentials) {} + constructor( + private readonly logger: Logger, + private readonly loadNodesAndCredentials: LoadNodesAndCredentials, + ) {} + + /** + * Resolves a tool-variant name (`…Tool`/`…HitlTool`) to the base node type it + * is synthesized from, unless a real node with that name exists on disk. + * + * A "synthetic tool" has no implementation of its own: workflows persist + * names like `gmailTool`, and the registry fabricates that node on demand by + * converting the `gmail` base node into an agent tool. + */ + private resolveBaseName(nodeTypeName: string): { baseName: string; isSyntheticTool: boolean } { + const isSyntheticTool = + nodeTypeName.endsWith('Tool') && !this.loadNodesAndCredentials.recognizesNode(nodeTypeName); + return { + baseName: isSyntheticTool ? stripToolSuffix(nodeTypeName) : nodeTypeName, + isSyntheticTool, + }; + } /** * Variant of `getByNameAndVersion` that includes the node's source path, used to locate a node's translations. @@ -65,23 +87,20 @@ export class NodeTypes implements INodeTypes { getByNameAndVersion(nodeType: string, version?: number): INodeType { const origType = nodeType; - const toolRequested = nodeType.endsWith('Tool'); + const { baseName, isSyntheticTool } = this.resolveBaseName(nodeType); // If an existing node name ends in `Tool`, then return that node, instead of creating a fake Tool node - if (toolRequested && this.loadNodesAndCredentials.recognizesNode(nodeType)) { + if (nodeType.endsWith('Tool') && !isSyntheticTool) { const node = this.loadNodesAndCredentials.getNode(nodeType); return NodeHelpers.getVersionedNodeType(node.type, version); } - // Make sure the nodeType to actually get from disk is the un-wrapped type - if (toolRequested) { - nodeType = stripToolSuffix(nodeType); - } - - const node = this.loadNodesAndCredentials.getNode(nodeType); + const node = this.loadNodesAndCredentials.getNode(baseName); const versionedNodeType = NodeHelpers.getVersionedNodeType(node.type, version); - if (toolRequested && typeof versionedNodeType.supplyData === 'function') { - throw new UnexpectedError('Node already has a `supplyData` method', { extra: { nodeType } }); + if (isSyntheticTool && typeof versionedNodeType.supplyData === 'function') { + throw new UnexpectedError('Node already has a `supplyData` method', { + extra: { nodeType: baseName }, + }); } if (shouldAssignExecuteMethod(versionedNodeType)) { @@ -92,18 +111,17 @@ export class NodeTypes implements INodeTypes { }; } - if (!toolRequested) return versionedNodeType; + if (!isSyntheticTool) return versionedNodeType; const { loadedNodes } = this.loadNodesAndCredentials; if (origType in loadedNodes) { return loadedNodes[origType].type as INodeType; } - // Check if this is an HITL tool (ends with 'HitlTool') - const isHitlTool = origType.endsWith('HitlTool'); + const isHitlTool = isHitlToolType(origType); - if (!isHitlTool && !versionedNodeType.description.usableAsTool) { - throw new UserError('Node cannot be used as a tool', { extra: { nodeType } }); + if (!satisfiesToolCapability(origType, versionedNodeType)) { + throw new UserError('Node cannot be used as a tool', { extra: { nodeType: baseName } }); } // Instead of modifying the existing type, we extend it into a new type object @@ -128,6 +146,49 @@ export class NodeTypes implements INodeTypes { return this.loadNodesAndCredentials.knownNodes; } + /** + * The `typeVersion`s this instance can resolve for a node type. Returns + * `undefined` when the type is unknown or fails to load (a warning is logged + * for anything but a plain unrecognized type), and `[]` when the type exists + * but no version satisfies the request — e.g. a synthetic tool name (`…Tool`) + * whose base node cannot be used as a tool (`…HitlTool` has no such + * requirement). Unlike `getByNameAndVersion`, never registers synthetic tools + * or mutates the registry, though resolving a known type may trigger its lazy + * module load. + */ + getSupportedVersions(nodeTypeName: string): number[] | undefined { + // The whole resolution runs fail-closed: name lookups use `in`, so a + // hostile name (e.g. `n8n-nodes-base.constructor`) can surface + // prototype-chain values that throw at any of the reads below. Fold any + // such failure into "unknown type" instead of failing the caller. + try { + const { baseName, isSyntheticTool } = this.resolveBaseName(nodeTypeName); + + const { type } = this.loadNodesAndCredentials.getNode(baseName); + + if ('nodeVersions' in type) { + return Object.entries(type.nodeVersions) + .filter( + ([, versioned]) => !isSyntheticTool || satisfiesToolCapability(nodeTypeName, versioned), + ) + .map(([version]) => Number(version)); + } + + if (isSyntheticTool && !satisfiesToolCapability(nodeTypeName, type)) return []; + + const { version } = type.description; + return Array.isArray(version) ? [...version] : [version]; + } catch (error) { + if (!(error instanceof UnrecognizedNodeTypeError)) { + this.logger.warn('Failed to resolve node type while listing supported versions', { + nodeType: nodeTypeName, + error: ensureError(error).message, + }); + } + return undefined; + } + } + async getNodeTranslationPath({ nodeSourcePath, longNodeType, @@ -172,9 +233,7 @@ export class NodeTypes implements INodeTypes { getNodeTypeDescriptions(nodeTypes: NeededNodeType[]): INodeTypeDescription[] { return nodeTypes.map(({ name: nodeTypeName, version: nodeTypeVersion }) => { - const isSyntheticTool = - nodeTypeName.endsWith('Tool') && !this.loadNodesAndCredentials.recognizesNode(nodeTypeName); - const baseName = isSyntheticTool ? stripToolSuffix(nodeTypeName) : nodeTypeName; + const { baseName, isSyntheticTool } = this.resolveBaseName(nodeTypeName); const nodeType = this.loadNodesAndCredentials.getNode(baseName); const { description } = NodeHelpers.getVersionedNodeType(nodeType.type, nodeTypeVersion); @@ -201,7 +260,7 @@ export class NodeTypes implements INodeTypes { nodeTypeName: string, description: INodeTypeDescription, ): INodeTypeDescription { - if (nodeTypeName.endsWith('HitlTool')) { + if (isHitlToolType(nodeTypeName)) { return convertNodeToHitlTool({ description: deepCopy(description) }).description; } 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 e1e561337e2..f64da34f54e 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 @@ -193,6 +193,7 @@ const n8nPackagesHandlers: N8nPackagesHandlers = { workflowConflictPolicy: payload.data.workflowConflictPolicy, workflowPublishingPolicy: payload.data.workflowPublishingPolicy, workflowIdPolicy: payload.data.workflowIdPolicy, + missingNodeTypeMode: payload.data.missingNodeTypeMode, 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 46b189a6d82..473dc16b854 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 @@ -111,6 +111,20 @@ post: existing workflow in the target project (status `updated` or `skipped`) always keep that workflow's current id, regardless of policy. + missingNodeTypeMode: + type: string + enum: + - fail + - import-anyway + default: fail + description: > + What to do when a workflow in the package uses a node type — or a + version of a node type — this instance does not have. `fail` + (default) rejects the import before anything is written, listing + every missing `(nodeType, typeVersion)` pair and the workflows + that use it. `import-anyway` imports the package; workflows + containing missing node types are never published by this import, + regardless of `workflowPublishingPolicy`. workflowPublishingPolicy: type: string enum: @@ -285,7 +299,8 @@ post: description: > `blocked` means the imported version could not be published and no version is active (for example because the workflow - uses a stubbed credential). When a prior published version + uses a stubbed credential, or uses a node type this instance + does not have). When a prior published version remains active, `state` is `unchanged` with `skippedPublishReason` instead. `failed` means publish or unpublish was attempted but did not succeed. @@ -298,6 +313,7 @@ post: type: string enum: - stub-credential + - missing-node-type description: > Present when `state` is `blocked`: the imported version could not be published and no version is active. @@ -305,6 +321,7 @@ post: type: string enum: - stub-credential + - missing-node-type description: > Present when `state` is `unchanged` but the policy wanted to publish the imported version: a prior published version @@ -467,7 +484,8 @@ post: '422': description: > Import blocked by non-conflict issues only (e.g. unresolved credentials or - variables). + variables, or node types this instance does not have under + `missingNodeTypeMode=fail`). content: application/json: schema: 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 4e951049d83..00ac8172c10 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 @@ -124,6 +124,32 @@ oneOf: type: string nullable: true description: 'For `id-in-other-project`: the project that already owns the id.' + - type: object + description: > + A node type — or a version of a node type — used by a package workflow that + this instance does not have, under `missingNodeTypeMode=fail`. One issue is + reported per missing `(nodeType, typeVersion)` pair. + required: + - type + - nodeType + - typeVersion + - usedByWorkflows + properties: + type: + type: string + enum: + - missing-node-type + nodeType: + type: string + description: Full node type name as used by the package's workflows. + typeVersion: + type: number + description: Node type version the package's workflows use. + usedByWorkflows: + type: array + items: + type: string + description: Package workflow ids that use this node type and version. - type: object description: A credential reference that could not be resolved in the target project. required: diff --git a/packages/cli/src/utils.ts b/packages/cli/src/utils.ts index 68a02ce1b86..a8c50f3be15 100644 --- a/packages/cli/src/utils.ts +++ b/packages/cli/src/utils.ts @@ -1,4 +1,4 @@ -import { CliWorkflowOperationError, SubworkflowOperationError } from 'n8n-workflow'; +import { CliWorkflowOperationError, isHitlToolType, SubworkflowOperationError } from 'n8n-workflow'; import type { INode, INodeType, Workflow } from 'n8n-workflow'; import { STARTING_NODES } from '@/constants'; @@ -20,6 +20,15 @@ export function stripToolSuffix(nodeType: string): string { return nodeType.replace(/HitlTool$/, '').replace(/Tool$/, ''); } +/** + * Whether the given resolved node (version) can back the synthetic tool name. + * HITL tools have no capability requirement; regular tools need the base node + * version to declare `usableAsTool`. + */ +export function satisfiesToolCapability(syntheticToolName: string, nodeType: INodeType): boolean { + return isHitlToolType(syntheticToolName) || !!nodeType.description.usableAsTool; +} + function findWorkflowStart(executionMode: 'integrated' | 'cli') { return function (nodes: INode[]) { const executeWorkflowTriggerNode = nodes.find( diff --git a/packages/cli/test/integration/public-api/n8n-packages-import.test.ts b/packages/cli/test/integration/public-api/n8n-packages-import.test.ts index c8cd1e15c01..52b0cf03b63 100644 --- a/packages/cli/test/integration/public-api/n8n-packages-import.test.ts +++ b/packages/cli/test/integration/public-api/n8n-packages-import.test.ts @@ -8,6 +8,7 @@ import { CredentialTypes } from '@/credential-types'; import { EventService } from '@/events/event.service'; import { buildImportPackageBuffer, + serializedWorkflow, serializedWorkflowWithCredential, } from '@/modules/n8n-packages/__tests__/fixtures/package-fixtures'; import { TarPackageWriter } from '@/modules/n8n-packages/io/tar/tar-package-writer'; @@ -29,6 +30,9 @@ beforeAll(async () => { const credentialTypesMock = mockInstance(CredentialTypes); credentialTypesMock.recognizes.mockReturnValue(true); + // Register node types so imports pass the default fail-on-missing-node-type check. + await utils.initNodeTypes(); + owner = await createOwnerWithApiKey(); Container.get(InstanceSettings).markAsLeader(); ownerPersonalProject = await Container.get(ProjectRepository).getPersonalProjectForUserOrFail( @@ -249,6 +253,7 @@ describe('POST /n8n-packages/import', () => { .field('bindings', '{}') .field('workflowConflictPolicy', 'fail') .field('workflowIdPolicy', 'new') + .field('missingNodeTypeMode', 'fail') .field('dataTableMatchingMode', 'by-id') .field('dataTableMissingMode', 'must-preexist') .field('dataTableSchemaConflictPolicy', 'fail') @@ -342,6 +347,71 @@ describe('POST /n8n-packages/import', () => { }); }); + const unknownNodeTypePackage = async (sourceId: string) => + await buildImportPackageBuffer( + [ + serializedWorkflow({ + id: 'wf-unknown-node', + name: 'Unknown Node Type', + // Published in the source, so a publish-intent policy would publish it. + isPublished: true, + nodes: [ + { + id: 'unknown-node', + name: 'Unknown Node', + type: 'n8n-nodes-community.chatBot', + typeVersion: 1, + position: [0, 0], + parameters: {}, + }, + ], + }), + ], + { sourceId }, + ); + + test('returns 422 by default when a workflow uses an unknown node type', async () => { + const tarBuffer = await unknownNodeTypePackage('http-integration-missing-node-type-fail'); + + const response = await authOwnerAgent + .post('/n8n-packages/import') + .field('workflowConflictPolicy', 'fail') + .attach('package', tarBuffer, 'import.n8np'); + + expect(response.statusCode).toBe(422); + expect(response.body).toMatchObject({ + message: expect.stringContaining('Import blocked'), + issues: [ + { + type: 'missing-node-type', + nodeType: 'n8n-nodes-community.chatBot', + typeVersion: 1, + usedByWorkflows: ['wf-unknown-node'], + }, + ], + }); + }); + + test('honors missingNodeTypeMode=import-anyway for a package with an unknown node type', async () => { + const tarBuffer = await unknownNodeTypePackage('http-integration-missing-node-type-anyway'); + + const response = await authOwnerAgent + .post('/n8n-packages/import') + .field('workflowConflictPolicy', 'fail') + .field('missingNodeTypeMode', 'import-anyway') + .field('workflowPublishingPolicy', 'match-source') + .attach('package', tarBuffer, 'import.n8np'); + + expect(response.statusCode).toBe(200); + expect(response.body.workflows).toHaveLength(1); + // match-source wanted to publish it, but the missing node type blocks that. + expect(response.body.workflows[0].publishing).toEqual({ + state: 'blocked', + blockedReason: 'missing-node-type', + }); + expect(response.body.workflows[0].activeVersionId).toBeNull(); + }); + test('creates stub credentials by default when references are missing', async () => { const tarBuffer = await buildImportPackageBuffer( [ diff --git a/packages/cli/test/integration/shared/utils/index.ts b/packages/cli/test/integration/shared/utils/index.ts index 58ea68f38c4..090ee7afa37 100644 --- a/packages/cli/test/integration/shared/utils/index.ts +++ b/packages/cli/test/integration/shared/utils/index.ts @@ -110,6 +110,20 @@ function buildDefaultNodes(): INodeTypeData { type: mock({ description: new WebhookNode().description } as never) as INodeType, sourcePath: '', }, + // Minimal mocks for node types the package-import fixtures reference at + // typeVersion 1; import validation only needs name + version resolution. + 'n8n-nodes-base.httpRequest': { + type: mock({ + description: { name: 'n8n-nodes-base.httpRequest', version: 1 }, + } as never) as INodeType, + sourcePath: '', + }, + 'n8n-nodes-base.dataTable': { + type: mock({ + description: { name: 'n8n-nodes-base.dataTable', version: 1 }, + } as never) as INodeType, + sourcePath: '', + }, }; }