From f54fc2ebfb407229fc222f34a8df109e855115ab Mon Sep 17 00:00:00 2001 From: Robin Braumann <50590409+bjorger@users.noreply.github.com> Date: Wed, 21 Oct 2026 10:44:19 +0000 Subject: [PATCH] feat(core): Resolve Agent package import identities (no-changelog) (#40479) --- .../src/modules/agents/agent-task.service.ts | 4 + .../cli/src/modules/agents/agents.service.ts | 12 + .../repositories/agent-task.repository.ts | 10 +- .../agents/repositories/agent.repository.ts | 26 +++ ...gent-import-identities.integration.test.ts | 208 ++++++++++++++++++ .../export-agent-package.integration.test.ts | 6 +- .../__tests__/import-id-policy.test.ts | 23 ++ .../agent-import-match.service.test.ts | 68 ++++++ .../agent/__tests__/agent.exporter.test.ts | 2 +- .../agent/agent-import-match.service.ts | 153 +++++++++++++ .../entities/agent/agent-import.types.ts | 45 ++++ .../n8n-packages/entities/import-id-policy.ts | 13 ++ .../n8n-packages/entities/match-source-ids.ts | 36 +++ .../__tests__/workflow-id-policy.test.ts | 23 -- .../workflow-import-match.service.test.ts | 4 +- .../entities/workflow/workflow-id-policy.ts | 17 -- .../workflow/workflow-import-match.service.ts | 44 ++-- .../entities/workflow/workflow-importer.ts | 6 +- 18 files changed, 623 insertions(+), 77 deletions(-) create mode 100644 packages/cli/src/modules/n8n-packages/__tests__/agent-import-identities.integration.test.ts create mode 100644 packages/cli/src/modules/n8n-packages/entities/__tests__/import-id-policy.test.ts create mode 100644 packages/cli/src/modules/n8n-packages/entities/agent/__tests__/agent-import-match.service.test.ts create mode 100644 packages/cli/src/modules/n8n-packages/entities/agent/agent-import-match.service.ts create mode 100644 packages/cli/src/modules/n8n-packages/entities/import-id-policy.ts create mode 100644 packages/cli/src/modules/n8n-packages/entities/match-source-ids.ts delete mode 100644 packages/cli/src/modules/n8n-packages/entities/workflow/__tests__/workflow-id-policy.test.ts delete mode 100644 packages/cli/src/modules/n8n-packages/entities/workflow/workflow-id-policy.ts diff --git a/packages/cli/src/modules/agents/agent-task.service.ts b/packages/cli/src/modules/agents/agent-task.service.ts index 70a0b88cec7..bcc4ca49bb6 100644 --- a/packages/cli/src/modules/agents/agent-task.service.ts +++ b/packages/cli/src/modules/agents/agent-task.service.ts @@ -91,6 +91,10 @@ export class AgentTaskService { return tasks.map((task) => this.toDto(task)); } + async findImportIdOwners(taskIds: string[]): Promise>> { + return await this.taskRepository.findImportIdOwners(taskIds); + } + /** * Create a task body and attach a `{ type:'task', id, enabled }` ref to the * agent's draft config in one transaction. Does not register a cron job — diff --git a/packages/cli/src/modules/agents/agents.service.ts b/packages/cli/src/modules/agents/agents.service.ts index fbb6d4376c9..60b5c03baba 100644 --- a/packages/cli/src/modules/agents/agents.service.ts +++ b/packages/cli/src/modules/agents/agents.service.ts @@ -66,6 +66,7 @@ import { toAgentRef } from './utils/agent-ref'; import { AgentTaskRepository } from './repositories/agent-task.repository'; import { AgentRepository, + type AgentImportIdentity, type AgentSummary, type AgentListResult, type AgentSummaryFilters, @@ -347,6 +348,17 @@ export class AgentsService { return await this.agentRepository.findByIdAndProjectId(agentId, projectId); } + async findImportCandidates( + projectId: string, + sourceAgentIds: string[], + ): Promise { + return await this.agentRepository.findImportCandidates(projectId, sourceAgentIds); + } + + async findImportIdOwners(agentIds: string[]): Promise>> { + return await this.agentRepository.findImportIdOwners(agentIds); + } + async isN8nChatPublished(agentId: string, projectId: string): Promise { return await this.agentRepository.isN8nChatPublished(agentId, projectId); } diff --git a/packages/cli/src/modules/agents/repositories/agent-task.repository.ts b/packages/cli/src/modules/agents/repositories/agent-task.repository.ts index b0214180e53..0e2f157f200 100644 --- a/packages/cli/src/modules/agents/repositories/agent-task.repository.ts +++ b/packages/cli/src/modules/agents/repositories/agent-task.repository.ts @@ -1,5 +1,5 @@ import type { AgentTaskConfig } from '@n8n/api-types'; -import { BaseRepository, TransactionRunner, type OperationContext } from '@n8n/db'; +import { BaseRepository, chunkIds, TransactionRunner, type OperationContext } from '@n8n/db'; import { Service } from '@n8n/di'; import { DataSource, In } from '@n8n/typeorm'; import isEqual from 'lodash/isEqual'; @@ -24,6 +24,14 @@ export class AgentTaskRepository extends BaseRepository { return await this.findOne({ where: { id, agentId } }); } + async findImportIdOwners(taskIds: string[]): Promise>> { + const tasks: Array> = []; + for (const ids of chunkIds([...new Set(taskIds)])) { + tasks.push(...(await this.find({ select: ['id', 'agentId'], where: { id: In(ids) } }))); + } + return tasks; + } + async saveDefinitions(tasks: AgentTask[], ctx: OperationContext): Promise { return await this.managerFor(ctx).save(AgentTask, tasks); } diff --git a/packages/cli/src/modules/agents/repositories/agent.repository.ts b/packages/cli/src/modules/agents/repositories/agent.repository.ts index 89e1038ff8d..692c3a8e37b 100644 --- a/packages/cli/src/modules/agents/repositories/agent.repository.ts +++ b/packages/cli/src/modules/agents/repositories/agent.repository.ts @@ -39,6 +39,8 @@ const projectScopeUpdateSql = "scope = 'project'"; /** Integration and publication state for channel runtime decisions. */ export type AgentIntegrationState = Pick; +export type AgentImportIdentity = Pick; + export type AgentSummaryFilters = { query?: string; publishedOnly?: boolean; @@ -119,6 +121,30 @@ export class AgentRepository extends BaseRepository { return ids; } + async findImportCandidates( + projectId: string, + sourceAgentIds: string[], + ): Promise { + const agents: AgentImportIdentity[] = []; + for (const ids of chunkIds([...new Set(sourceAgentIds)])) { + agents.push( + ...(await this.find({ + select: ['id', 'name', 'projectId'], + where: { projectId, id: In(ids) }, + })), + ); + } + return agents; + } + + async findImportIdOwners(agentIds: string[]): Promise>> { + const agents: Array> = []; + for (const ids of chunkIds([...new Set(agentIds)])) { + agents.push(...(await this.find({ select: ['id', 'projectId'], where: { id: In(ids) } }))); + } + return agents; + } + /** * Lean listing for search surfaces: selects only summary columns, skipping * the JSON config columns and the activeVersion join, and pushes all diff --git a/packages/cli/src/modules/n8n-packages/__tests__/agent-import-identities.integration.test.ts b/packages/cli/src/modules/n8n-packages/__tests__/agent-import-identities.integration.test.ts new file mode 100644 index 00000000000..349455f1783 --- /dev/null +++ b/packages/cli/src/modules/n8n-packages/__tests__/agent-import-identities.integration.test.ts @@ -0,0 +1,208 @@ +import { createTeamProject, linkUserToProject, testDb, testModules } from '@n8n/backend-test-utils'; +import type { Project, User } from '@n8n/db'; +import { Container } from '@n8n/di'; +import { ForbiddenError } from '@n8n/errors'; + +import { AgentTaskRepository } from '@/modules/agents/repositories/agent-task.repository'; +import { AgentRepository } from '@/modules/agents/repositories/agent.repository'; +import { createMember, createOwner } from '@test-integration/db/users'; + +import { looseAgentsFixture } from './fixtures/agent-package-fixtures'; +import { AgentImportMatchService } from '../entities/agent/agent-import-match.service'; +import type { PreparedAgent } from '../entities/agent/agent-import.types'; +import { serializedAgentSchema } from '../spec/serialized/agent.schema'; + +let service: AgentImportMatchService; +let agents: AgentRepository; +let tasks: AgentTaskRepository; +let user: User; +let project: Project; +let otherProject: Project; +const fixture = looseAgentsFixture(); +const sources: PreparedAgent[] = fixture.manifest.agents.map(({ target }) => { + const { id, ...content } = serializedAgentSchema.parse(fixture.files[`${target}/agent.json`]); + return { ...content, sourceAgentId: id, metadata: { versionId: null, publishedVersionId: null } }; +}); +const taskBody = sources[0].tasks.support_source_task; + +beforeAll(async () => { + await testModules.loadModules(['agents']); + await testDb.init(); + agents = Container.get(AgentRepository); + tasks = Container.get(AgentTaskRepository); + service = Container.get(AgentImportMatchService); + user = await createOwner(); + project = await createTeamProject('Destination', user); + otherProject = await createTeamProject('Other destination', user); +}); +beforeEach(async () => await agents.delete({})); +afterEach(() => vi.restoreAllMocks()); +afterAll(async () => await testDb.terminate()); + +async function seedAgent(id: string, projectId = project.id) { + return await agents.save({ id, name: id, projectId }); +} + +async function records() { + return await Promise.all([ + agents.find({ order: { id: 'ASC' } }), + tasks.find({ order: { id: 'ASC' } }), + ]); +} + +async function resolve(projectId = project.id) { + return await service.allocateAgentIds( + await service.findBySourceAgentIds( + { user, projectId }, + sources.map(({ sourceAgentId }) => sourceAgentId), + ), + ); +} + +it('retains persisted Agent and task identities on re-import', async () => { + const sourceBefore = structuredClone(sources); + const identities = await resolve(); + const mapping = await service.mapTaskIds(sources, identities); + expect(identities.idConflicts).toEqual([]); + expect(mapping.idConflicts).toEqual([]); + for (const source of sources) { + const { targetAgentId } = identities.identities.get(source.sourceAgentId)!; + await seedAgent(targetAgentId); + expect(targetAgentId).toBe(source.sourceAgentId); + for (const [sourceTaskId, body] of Object.entries(source.tasks)) { + const id = mapping.taskIdsBySourceAgentId.get(source.sourceAgentId)!.get(sourceTaskId)!; + await tasks.save({ ...body, id, agentId: targetAgentId }); + expect(id).toBe(sourceTaskId); + } + } + const before = await records(); + const repeated = await resolve(); + expect([...repeated.identities.values()].map(({ targetAgentId }) => targetAgentId)).toEqual( + [...identities.identities.values()].map(({ targetAgentId }) => targetAgentId), + ); + expect((await service.mapTaskIds(sources, repeated)).taskIdsBySourceAgentId).toEqual( + mapping.taskIdsBySourceAgentId, + ); + expect(await records()).toEqual(before); + expect(sources).toEqual(sourceBefore); +}); + +it('reports an occupied Agent ID in another project under source policy', async () => { + await seedAgent('support_source', otherProject.id); + const before = await records(); + const result = await resolve(); + expect(result.identities.has('support_source')).toBe(false); + expect(result.idConflicts).toEqual([ + { + sourceAgentId: 'support_source', + existingAgentId: 'support_source', + existingProjectId: otherProject.id, + }, + ]); + expect(await records()).toEqual(before); +}); + +it('reports an occupied task ID owned by another Agent', async () => { + await seedAgent('support_source'); + await seedAgent('other-agent', otherProject.id); + await tasks.save({ + id: 'support_source_task', + agentId: 'other-agent', + ...taskBody, + }); + const before = await records(); + const result = await service.mapTaskIds(sources, await resolve()); + expect(result.taskIdsBySourceAgentId.get('support_source')!.size).toBe(0); + expect(result.idConflicts).toEqual([ + { + sourceAgentId: 'support_source', + targetAgentId: 'support_source', + sourceTaskId: 'support_source_task', + targetTaskId: 'support_source_task', + conflictingAgentId: 'other-agent', + }, + ]); + expect(await records()).toEqual(before); +}); + +it('keeps task and Agent ID namespaces separate', async () => { + await seedAgent('support_source'); + const identities = await resolve(); + const result = await service.mapTaskIds( + [{ sourceAgentId: 'support_source', tasks: { support_source: taskBody } }], + identities, + ); + expect(result.taskIdsBySourceAgentId.get('support_source')).toEqual( + new Map([['support_source', 'support_source']]), + ); + expect(result.idConflicts).toEqual([]); +}); + +it('restricts destination matching to projects the user can read', async () => { + await seedAgent('support_source'); + const member = await createMember(); + const read = vi.spyOn(agents, 'findImportCandidates'); + await expect( + service.findBySourceAgentIds({ user: member, projectId: project.id }, ['support_source']), + ).rejects.toThrow(ForbiddenError); + expect(read).not.toHaveBeenCalled(); + await linkUserToProject(member, project, 'project:viewer'); + const matches = await service.findBySourceAgentIds({ user: member, projectId: project.id }, [ + 'support_source', + ]); + expect(matches.matches.get('support_source')?.id).toBe('support_source'); +}); + +it('keeps global ID checks for an authorized pending project', async () => { + await seedAgent('support_source'); + const member = await createMember(); + const read = vi.spyOn(agents, 'findImportCandidates'); + const matches = await service.findBySourceAgentIds( + { user: member, projectId: 'pending-project', projectPendingCreation: true }, + ['support_source'], + ); + const result = await service.allocateAgentIds(matches); + expect(result.identities.size).toBe(0); + expect(result.idConflicts).toEqual([ + { + sourceAgentId: 'support_source', + existingAgentId: 'support_source', + existingProjectId: project.id, + }, + ]); + expect(read).not.toHaveBeenCalled(); +}); + +it('bounds candidate and ownership queries for large ID lists', async () => { + await seedAgent('id-0'); + await seedAgent('id-32999'); + await seedAgent('outside-agent', otherProject.id); + await tasks.save([ + { id: 'id-0', agentId: 'id-0', ...taskBody }, + { id: 'id-32999', agentId: 'id-0', ...taskBody }, + { id: 'outside-task', agentId: 'outside-agent', ...taskBody }, + ]); + const ids = Array.from({ length: 33_000 }, (_, index) => `id-${index}`); + const candidates = await agents.findImportCandidates(project.id, ids); + expect(candidates.map(({ id }) => id).sort()).toEqual(['id-0', 'id-32999']); + expect(candidates.every((candidate) => !Object.hasOwn(candidate, 'schema'))).toBe(true); + expect(await agents.findImportIdOwners(ids)).toMatchObject([ + { id: 'id-0', projectId: project.id }, + { id: 'id-32999', projectId: project.id }, + ]); + expect(await tasks.findImportIdOwners(ids)).toMatchObject([ + { id: 'id-0', agentId: 'id-0' }, + { id: 'id-32999', agentId: 'id-0' }, + ]); +}); + +it('returns empty results without database reads', async () => { + const agentRead = vi.spyOn(agents, 'find'); + const taskRead = vi.spyOn(tasks, 'find'); + const matches = await service.findBySourceAgentIds({ user, projectId: project.id }, []); + const identities = await service.allocateAgentIds(matches); + expect((await service.mapTaskIds([], identities)).taskIdsBySourceAgentId.size).toBe(0); + expect(await agents.findImportCandidates(project.id, [])).toEqual([]); + expect(agentRead).not.toHaveBeenCalled(); + expect(taskRead).not.toHaveBeenCalled(); +}); diff --git a/packages/cli/src/modules/n8n-packages/__tests__/export-agent-package.integration.test.ts b/packages/cli/src/modules/n8n-packages/__tests__/export-agent-package.integration.test.ts index f6203a74ed6..d3f469a2b06 100644 --- a/packages/cli/src/modules/n8n-packages/__tests__/export-agent-package.integration.test.ts +++ b/packages/cli/src/modules/n8n-packages/__tests__/export-agent-package.integration.test.ts @@ -135,7 +135,11 @@ beforeAll(async () => { revision: 0, }); await taskRepository.save( - Object.entries(draft.tasks).map(([id, task]) => ({ id, agentId: agent.id, ...task })), + Object.entries(draft.tasks).map(([id, task]) => ({ + id, + agentId: agent.id, + ...task, + })), ); const history = await historyRepository.saveVersion({ versionId: `${agent.id}_published`, diff --git a/packages/cli/src/modules/n8n-packages/entities/__tests__/import-id-policy.test.ts b/packages/cli/src/modules/n8n-packages/entities/__tests__/import-id-policy.test.ts new file mode 100644 index 00000000000..499ffd453a1 --- /dev/null +++ b/packages/cli/src/modules/n8n-packages/entities/__tests__/import-id-policy.test.ts @@ -0,0 +1,23 @@ +import { decideImportId } from '../import-id-policy'; + +describe('decideImportId', () => { + describe('source', () => { + it('keeps the source id', () => { + expect(decideImportId('source', 'STILTON')).toBe('STILTON'); + }); + }); + + describe('new', () => { + it('mints a fresh id, discarding the source id', () => { + const id = decideImportId('new', 'STILTON'); + + expect(id).toEqual(expect.any(String)); + expect(id).not.toBe('STILTON'); + expect(id.length).toBeGreaterThan(0); + }); + + it('mints a different id on each call', () => { + expect(decideImportId('new', 'STILTON')).not.toBe(decideImportId('new', 'STILTON')); + }); + }); +}); diff --git a/packages/cli/src/modules/n8n-packages/entities/agent/__tests__/agent-import-match.service.test.ts b/packages/cli/src/modules/n8n-packages/entities/agent/__tests__/agent-import-match.service.test.ts new file mode 100644 index 00000000000..e364f680394 --- /dev/null +++ b/packages/cli/src/modules/n8n-packages/entities/agent/__tests__/agent-import-match.service.test.ts @@ -0,0 +1,68 @@ +import type { ProjectScopeService } from '@n8n/backend-services'; +import type { User } from '@n8n/db'; +import { ForbiddenError } from '@n8n/errors'; +import { mock } from 'vitest-mock-extended'; + +import type { AgentTaskService } from '@/modules/agents/agent-task.service'; +import type { AgentsService } from '@/modules/agents/agents.service'; + +import { looseAgentsFixture } from '../../../__tests__/fixtures/agent-package-fixtures'; +import { serializedAgentSchema } from '../../../spec/serialized/agent.schema'; +import { AgentImportMatchService } from '../agent-import-match.service'; + +function setup() { + const projects = mock(); + projects.getProjectIds.mockResolvedValue(null); + const agents = mock(); + agents.findImportCandidates.mockResolvedValue([]); + agents.findImportIdOwners.mockResolvedValue([]); + const tasks = mock(); + tasks.findImportIdOwners.mockResolvedValue([]); + return { + projects, + agents, + tasks, + context: { user: mock(), projectId: 'destination' }, + service: new AgentImportMatchService(projects, agents, tasks), + }; +} + +describe('AgentImportMatchService', () => { + it('checks project read access before reading destination Agents', async () => { + const { service, projects, agents, context } = setup(); + projects.getProjectIds.mockResolvedValue(['different-project']); + await expect(service.findBySourceAgentIds(context, ['source'])).rejects.toThrow(ForbiddenError); + expect(projects.getProjectIds).toHaveBeenCalledWith(context.user, ['agent:read']); + expect(agents.findImportCandidates).not.toHaveBeenCalled(); + }); + + it('omits both task mappings when two proposed owners use one ID', async () => { + const { service, context } = setup(); + const content = serializedAgentSchema.parse( + looseAgentsFixture().files['agents/support/agent.json'], + ); + const tasks = { shared_task: content.tasks.support_source_task }; + const sources = ['first', 'second'].map((sourceAgentId) => ({ sourceAgentId, tasks })); + const identities = await service.allocateAgentIds( + await service.findBySourceAgentIds(context, ['first', 'second']), + ); + const result = await service.mapTaskIds(sources, identities); + expect([...result.taskIdsBySourceAgentId.values()].map((map) => map.size)).toEqual([0, 0]); + expect(result.idConflicts).toEqual([ + { + sourceAgentId: 'first', + targetAgentId: 'first', + sourceTaskId: 'shared_task', + targetTaskId: 'shared_task', + conflictingAgentId: 'second', + }, + { + sourceAgentId: 'second', + targetAgentId: 'second', + sourceTaskId: 'shared_task', + targetTaskId: 'shared_task', + conflictingAgentId: 'first', + }, + ]); + }); +}); diff --git a/packages/cli/src/modules/n8n-packages/entities/agent/__tests__/agent.exporter.test.ts b/packages/cli/src/modules/n8n-packages/entities/agent/__tests__/agent.exporter.test.ts index 72a375b5a71..2bfa0187020 100644 --- a/packages/cli/src/modules/n8n-packages/entities/agent/__tests__/agent.exporter.test.ts +++ b/packages/cli/src/modules/n8n-packages/entities/agent/__tests__/agent.exporter.test.ts @@ -267,7 +267,7 @@ describe('AgentExporter', () => { expect(JSON.parse(writer.files[1].content)).toEqual(expected.metadata); }); - it('keeps authored bytes and hashes stable when only publication metadata changes', async () => { + it('keeps authored bytes and hashes stable when publication metadata changes', async () => { const { agent, exporter } = setup(); const before = (await exporter.prepare(agent))!; agent.versionId = 'new-draft-version'; diff --git a/packages/cli/src/modules/n8n-packages/entities/agent/agent-import-match.service.ts b/packages/cli/src/modules/n8n-packages/entities/agent/agent-import-match.service.ts new file mode 100644 index 00000000000..0b0910d2ad6 --- /dev/null +++ b/packages/cli/src/modules/n8n-packages/entities/agent/agent-import-match.service.ts @@ -0,0 +1,153 @@ +import { ProjectScopeService } from '@n8n/backend-services'; +import { Service } from '@n8n/di'; +import { ForbiddenError } from '@n8n/errors'; + +import { AgentTaskService } from '@/modules/agents/agent-task.service'; +import { AgentsService } from '@/modules/agents/agents.service'; + +import type { + AgentImportIdentities, + AgentImportMatchContext, + AgentImportMatches, + AgentTaskImportMappings, + PreparedAgent, +} from './agent-import.types'; + +interface TaskMapping { + sourceAgentId: string; + targetAgentId: string; + sourceTaskId: string; + targetTaskId: string; +} + +@Service() +export class AgentImportMatchService { + constructor( + private readonly projectScopeService: ProjectScopeService, + private readonly agentsService: AgentsService, + private readonly agentTaskService: AgentTaskService, + ) {} + + async findBySourceAgentIds( + context: AgentImportMatchContext, + sourceAgentIds: string[], + ): Promise { + const result: AgentImportMatches = { + projectId: context.projectId, + sourceAgentIds: [...new Set(sourceAgentIds)], + matches: new Map(), + }; + if (sourceAgentIds.length === 0 || context.projectPendingCreation) return result; + + const readableProjects = await this.projectScopeService.getProjectIds(context.user, [ + 'agent:read', + ]); + if (readableProjects !== null && !readableProjects.includes(context.projectId)) { + throw new ForbiddenError( + 'You do not have permission to read Agents in the destination project.', + ); + } + const candidates = await this.agentsService.findImportCandidates( + context.projectId, + result.sourceAgentIds, + ); + result.matches = new Map(candidates.map((agent) => [agent.id, agent])); + return result; + } + + /** Allocate only after matching. The planner decides which definitions it will apply. */ + async allocateAgentIds(matches: AgentImportMatches): Promise { + const result: AgentImportIdentities = { + projectId: matches.projectId, + identities: new Map(), + idConflicts: [], + }; + const unmatched = matches.sourceAgentIds.filter((id) => !matches.matches.has(id)); + const owners = await this.agentsService.findImportIdOwners(unmatched); + const ownersById = new Map(owners.map((owner) => [owner.id, owner])); + + for (const sourceAgentId of matches.sourceAgentIds) { + const owner = ownersById.get(sourceAgentId); + if (owner) { + result.idConflicts.push({ + sourceAgentId, + existingAgentId: owner.id, + existingProjectId: owner.projectId, + }); + continue; + } + const existing = matches.matches.get(sourceAgentId) ?? null; + result.identities.set(sourceAgentId, { + sourceAgentId, + targetAgentId: sourceAgentId, + existing, + }); + } + return result; + } + + async mapTaskIds( + agents: Array>, + identities: AgentImportIdentities, + ): Promise { + const result: AgentTaskImportMappings = { + taskIdsBySourceAgentId: new Map(), + idConflicts: [], + }; + const mappings: TaskMapping[] = []; + for (const agent of agents) { + const identity = identities.identities.get(agent.sourceAgentId); + if (!identity) continue; + result.taskIdsBySourceAgentId.set(agent.sourceAgentId, new Map()); + for (const sourceTaskId of Object.keys(agent.tasks)) { + mappings.push({ + sourceAgentId: agent.sourceAgentId, + targetAgentId: identity.targetAgentId, + sourceTaskId, + targetTaskId: sourceTaskId, + }); + } + } + const owners = await this.agentTaskService.findImportIdOwners( + mappings.map(({ targetTaskId }) => targetTaskId), + ); + const ownersById = new Map(owners.map(({ id, agentId }) => [id, agentId])); + const available: TaskMapping[] = []; + for (const mapping of mappings) { + const conflictingAgentId = ownersById.get(mapping.targetTaskId); + if (conflictingAgentId !== undefined && conflictingAgentId !== mapping.targetAgentId) { + result.idConflicts.push(toTaskIdConflict(mapping, conflictingAgentId)); + } else { + available.push(mapping); + } + } + collectTaskMappings(available, result); + return result; + } +} + +function toTaskIdConflict(mapping: TaskMapping, conflictingAgentId: string) { + const { sourceAgentId, sourceTaskId, targetAgentId, targetTaskId } = mapping; + return { sourceAgentId, sourceTaskId, targetAgentId, targetTaskId, conflictingAgentId }; +} + +function collectTaskMappings(mappings: TaskMapping[], result: AgentTaskImportMappings): void { + const byTargetId = new Map(); + for (const mapping of mappings) { + const group = byTargetId.get(mapping.targetTaskId) ?? []; + group.push(mapping); + byTargetId.set(mapping.targetTaskId, group); + } + for (const group of byTargetId.values()) { + for (const mapping of group) { + const other = group.find(({ targetAgentId }) => targetAgentId !== mapping.targetAgentId); + if (other) { + result.idConflicts.push(toTaskIdConflict(mapping, other.targetAgentId)); + continue; + } + result.taskIdsBySourceAgentId + .get(mapping.sourceAgentId) + ?.set(mapping.sourceTaskId, mapping.targetTaskId); + } + } +} diff --git a/packages/cli/src/modules/n8n-packages/entities/agent/agent-import.types.ts b/packages/cli/src/modules/n8n-packages/entities/agent/agent-import.types.ts index 6fe1e732f65..8bd2ed685ef 100644 --- a/packages/cli/src/modules/n8n-packages/entities/agent/agent-import.types.ts +++ b/packages/cli/src/modules/n8n-packages/entities/agent/agent-import.types.ts @@ -1,6 +1,51 @@ +import type { AgentImportIdentity } from '@/modules/agents/repositories/agent.repository'; + +import type { ImportContext } from '../../n8n-packages.types'; import type { SerializedAgent, SerializedAgentMetadata } from '../../spec/serialized/agent.schema'; export interface PreparedAgent extends Omit { sourceAgentId: string; metadata: SerializedAgentMetadata; } + +export interface AgentImportMatchContext extends Pick { + /** Set only after the project planner authorizes creation of this destination. */ + projectPendingCreation?: boolean; +} + +export interface AgentIdConflict { + sourceAgentId: string; + existingAgentId: string; + existingProjectId: string | null; +} + +export interface AgentImportMatches { + projectId: string; + sourceAgentIds: string[]; + matches: Map; +} + +export interface ResolvedAgentImportIdentity { + sourceAgentId: string; + targetAgentId: string; + existing: AgentImportIdentity | null; +} + +export interface AgentImportIdentities { + projectId: string; + identities: Map; + idConflicts: AgentIdConflict[]; +} + +export interface AgentTaskIdConflict { + sourceAgentId: string; + targetAgentId: string; + sourceTaskId: string; + targetTaskId: string; + conflictingAgentId: string; +} + +export interface AgentTaskImportMappings { + taskIdsBySourceAgentId: Map>; + idConflicts: AgentTaskIdConflict[]; +} diff --git a/packages/cli/src/modules/n8n-packages/entities/import-id-policy.ts b/packages/cli/src/modules/n8n-packages/entities/import-id-policy.ts new file mode 100644 index 00000000000..fe7751aad76 --- /dev/null +++ b/packages/cli/src/modules/n8n-packages/entities/import-id-policy.ts @@ -0,0 +1,13 @@ +import { generateNanoId } from '@n8n/utils/generate-nano-id'; + +import type { WorkflowIdPolicy } from '../n8n-packages.types'; + +/** Select an ID for a new import. Existing matches keep their IDs. */ +const IMPORT_ID_POLICIES: Record string> = { + new: () => generateNanoId(), + source: (sourceId) => sourceId, +}; + +export function decideImportId(policy: WorkflowIdPolicy, sourceId: string): string { + return IMPORT_ID_POLICIES[policy](sourceId); +} diff --git a/packages/cli/src/modules/n8n-packages/entities/match-source-ids.ts b/packages/cli/src/modules/n8n-packages/entities/match-source-ids.ts new file mode 100644 index 00000000000..516f855f4bd --- /dev/null +++ b/packages/cli/src/modules/n8n-packages/entities/match-source-ids.ts @@ -0,0 +1,36 @@ +/** A stored source identity excludes the local-ID fallback. Ambiguity excludes both. */ +export function matchSourceIds( + sourceIds: string[], + candidates: T[], + getSourceId: (candidate: T) => string | null, +) { + const requested = new Set(sourceIds); + const bySourceId = new Map(); + for (const candidate of candidates) { + const sourceId = getSourceId(candidate); + if (sourceId === null || !requested.has(sourceId)) continue; + const group = bySourceId.get(sourceId) ?? []; + group.push(candidate); + bySourceId.set(sourceId, group); + } + const matches = new Map(); + const ambiguous = new Map(); + for (const [sourceId, group] of bySourceId) { + if (group.length === 1) matches.set(sourceId, group[0]); + else + ambiguous.set( + sourceId, + group.toSorted((left, right) => left.id.localeCompare(right.id)), + ); + } + for (const candidate of candidates) { + if ( + getSourceId(candidate) !== null || + !requested.has(candidate.id) || + bySourceId.has(candidate.id) + ) + continue; + matches.set(candidate.id, candidate); + } + return { matches, ambiguous }; +} diff --git a/packages/cli/src/modules/n8n-packages/entities/workflow/__tests__/workflow-id-policy.test.ts b/packages/cli/src/modules/n8n-packages/entities/workflow/__tests__/workflow-id-policy.test.ts deleted file mode 100644 index 1a30866271d..00000000000 --- a/packages/cli/src/modules/n8n-packages/entities/workflow/__tests__/workflow-id-policy.test.ts +++ /dev/null @@ -1,23 +0,0 @@ -import { decideWorkflowId } from '../workflow-id-policy'; - -describe('decideWorkflowId', () => { - describe('source', () => { - it('keeps the source workflow id', () => { - expect(decideWorkflowId('source', 'STILTON')).toBe('STILTON'); - }); - }); - - describe('new', () => { - it('mints a fresh id, discarding the source id', () => { - const id = decideWorkflowId('new', 'STILTON'); - - expect(id).toEqual(expect.any(String)); - expect(id).not.toBe('STILTON'); - expect(id.length).toBeGreaterThan(0); - }); - - it('mints a different id on each call', () => { - expect(decideWorkflowId('new', 'STILTON')).not.toBe(decideWorkflowId('new', 'STILTON')); - }); - }); -}); diff --git a/packages/cli/src/modules/n8n-packages/entities/workflow/__tests__/workflow-import-match.service.test.ts b/packages/cli/src/modules/n8n-packages/entities/workflow/__tests__/workflow-import-match.service.test.ts index f93380cdda1..70c5b06c9ae 100644 --- a/packages/cli/src/modules/n8n-packages/entities/workflow/__tests__/workflow-import-match.service.test.ts +++ b/packages/cli/src/modules/n8n-packages/entities/workflow/__tests__/workflow-import-match.service.test.ts @@ -102,10 +102,12 @@ describe('WorkflowImportMatchService', () => { it('reports a conflict when two active workflows share a source id', async () => { const first = workflow({ id: 'local-1', sourceWorkflowId: 'wf-dup', name: 'First' }); const second = workflow({ id: 'local-2', sourceWorkflowId: 'wf-dup', name: 'Second' }); - const { service } = makeService([first, second]); + const authored = workflow({ id: 'wf-dup' }); + const { service } = makeService([authored, second, first]); const result = await service.findBySourceWorkflowIds('project-1', ['wf-dup']); + expect(result.matches.get('wf-dup')).toBe(first); expect(result.lineageConflicts).toEqual([ { sourceWorkflowId: 'wf-dup', diff --git a/packages/cli/src/modules/n8n-packages/entities/workflow/workflow-id-policy.ts b/packages/cli/src/modules/n8n-packages/entities/workflow/workflow-id-policy.ts deleted file mode 100644 index 8cb08c6ad76..00000000000 --- a/packages/cli/src/modules/n8n-packages/entities/workflow/workflow-id-policy.ts +++ /dev/null @@ -1,17 +0,0 @@ -import { generateNanoId } from '@n8n/db'; - -import type { WorkflowIdPolicy } from '../../n8n-packages.types'; - -/** - * Decides the id a newly-created imported workflow gets. `new` mints a fresh id - * (the source id is preserved separately as `sourceWorkflowId`); `source` reuses - * the package's own id so re-imports map back to the same workflow. - */ -const WORKFLOW_ID_POLICIES: Record string> = { - new: () => generateNanoId(), - source: (sourceWorkflowId) => sourceWorkflowId, -}; - -export function decideWorkflowId(policy: WorkflowIdPolicy, sourceWorkflowId: string): string { - return WORKFLOW_ID_POLICIES[policy](sourceWorkflowId); -} diff --git a/packages/cli/src/modules/n8n-packages/entities/workflow/workflow-import-match.service.ts b/packages/cli/src/modules/n8n-packages/entities/workflow/workflow-import-match.service.ts index bcc0f46b388..70665935984 100644 --- a/packages/cli/src/modules/n8n-packages/entities/workflow/workflow-import-match.service.ts +++ b/packages/cli/src/modules/n8n-packages/entities/workflow/workflow-import-match.service.ts @@ -3,6 +3,8 @@ import { Service } from '@n8n/di'; import { WorkflowFinderService } from '@/workflows/workflow-finder.service'; +import { matchSourceIds } from '../match-source-ids'; + export interface WorkflowIdConflict { sourceWorkflowId: string; existingWorkflowId: string; @@ -61,13 +63,11 @@ export class WorkflowImportMatchService { return { matches: new Map(), lineageConflicts: [] }; } - const packageWorkflowIds = new Set(sourceWorkflowIds); const finderOptions = { includeActiveVersion: true, includeParentFolder: true, includeArchived: true, } as const; - const matchBySourceWorkflowId = new Map(); const lineageConflicts: WorkflowLineageConflict[] = []; const workflows = await this.workflowFinderService.findOwnedWorkflowsBySourceWorkflowIds( @@ -76,32 +76,25 @@ export class WorkflowImportMatchService { finderOptions, ); - const workflowsBySourceId = new Map(); - for (const workflow of workflows) { - if (!workflow.sourceWorkflowId) continue; + const { matches, ambiguous } = matchSourceIds( + sourceWorkflowIds, + workflows, + (workflow) => workflow.sourceWorkflowId, + ); - const key = workflow.sourceWorkflowId; - if (!packageWorkflowIds.has(key)) continue; - const candidates = workflowsBySourceId.get(key) ?? []; - candidates.push(workflow); - workflowsBySourceId.set(key, candidates); - } - - for (const [sourceWorkflowId, candidates] of workflowsBySourceId) { + for (const [sourceWorkflowId, candidates] of ambiguous) { const activeCandidates = candidates.filter((workflow) => !workflow.isArchived); - if (candidates.length === 1 || activeCandidates.length === 1) { - matchBySourceWorkflowId.set(sourceWorkflowId, activeCandidates[0] ?? candidates[0]); + if (activeCandidates.length === 1) { + matches.set(sourceWorkflowId, activeCandidates[0]); continue; } - const sortedCandidates = candidates.toSorted((left, right) => - left.id.localeCompare(right.id), - ); - matchBySourceWorkflowId.set(sourceWorkflowId, sortedCandidates[0]); + // Keep a representative so the planner skips the conflicting import. + matches.set(sourceWorkflowId, candidates[0]); lineageConflicts.push({ sourceWorkflowId, projectId, - existingWorkflows: sortedCandidates.map(({ id, name, isArchived }) => ({ + existingWorkflows: candidates.map(({ id, name, isArchived }) => ({ id, name, isArchived, @@ -109,15 +102,6 @@ export class WorkflowImportMatchService { }); } - for (const workflow of workflows) { - if (workflow.sourceWorkflowId !== null) continue; - - const key = workflow.id; - if (!packageWorkflowIds.has(key) || matchBySourceWorkflowId.has(key)) continue; - - matchBySourceWorkflowId.set(key, workflow); - } - - return { matches: matchBySourceWorkflowId, lineageConflicts }; + return { matches, lineageConflicts }; } } 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 7ddedeea641..43ca737187e 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 @@ -15,7 +15,6 @@ import { type WorkflowArchiveTransition, } from './workflow-archive-transition'; import { decideWorkflowConflictAction } from './workflow-conflict-policy'; -import { decideWorkflowId } from './workflow-id-policy'; import { WorkflowImportMatchService, type WorkflowIdConflict, @@ -40,6 +39,7 @@ import type { WorkflowIdPolicy, } from '../../n8n-packages.types'; import { visitWorkflowCredentials } from '../credential/workflow-credential-references'; +import { decideImportId } from '../import-id-policy'; export interface WorkflowImportResult { outcomes: PersistedWorkflowOutcome[]; @@ -388,7 +388,7 @@ function toPlanItem( if (existing === null) { return { action: 'create', - decidedId: decideWorkflowId(idPolicy, prepared.sourceWorkflowId), + decidedId: decideImportId(idPolicy, prepared.sourceWorkflowId), ...prepared, }; } @@ -411,7 +411,7 @@ function toPlanItem( // Only `fail` reaches here with a match; it records a conflict the gate rejects first. return { action, - decidedId: decideWorkflowId(idPolicy, prepared.sourceWorkflowId), + decidedId: decideImportId(idPolicy, prepared.sourceWorkflowId), ...prepared, }; }