fix(core): Shift existing rules on create to avoid order conflict

packages/@n8n/api-types/src/dto/roles/__tests__/create-role-mapping-rule.dto.test.ts

@@ -0,0 +1,39 @@
+import { CreateRoleMappingRuleDto } from '../create-role-mapping-rule.dto';
+
+const VALID_BODY = {
+	expression: 'claims.admin === true',
+	role: 'global:member',
+	type: 'instance' as const,
+};
+
+describe('CreateRoleMappingRuleDto', () => {
+	describe('Valid requests', () => {
+		test.each([
+			{ name: 'order omitted', request: { ...VALID_BODY } },
+			{ name: 'order zero', request: { ...VALID_BODY, order: 0 } },
+			{ name: 'positive integer order', request: { ...VALID_BODY, order: 5 } },
+			{
+				name: 'project rule with projectIds',
+				request: {
+					...VALID_BODY,
+					type: 'project',
+					role: 'project:editor',
+					projectIds: ['p1'],
+				},
+			},
+		])('accepts $name', ({ request }) => {
+			expect(CreateRoleMappingRuleDto.safeParse(request).success).toBe(true);
+		});
+	});
+
+	describe('Invalid requests', () => {
+		test.each([
+			{ name: 'negative order', request: { ...VALID_BODY, order: -1 } },
+			{ name: 'non-integer order', request: { ...VALID_BODY, order: 1.5 } },
+			{ name: 'empty expression', request: { ...VALID_BODY, expression: '' } },
+			{ name: 'unknown type', request: { ...VALID_BODY, type: 'unknown' } },
+		])('rejects $name', ({ request }) => {
+			expect(CreateRoleMappingRuleDto.safeParse(request).success).toBe(false);
+		});
+	});
+});

packages/@n8n/api-types/src/dto/roles/create-role-mapping-rule.dto.ts

@@ -6,6 +6,6 @@ export class CreateRoleMappingRuleDto extends Z.class({
 	expression: z.string().min(1),
 	role: z.string().min(1).max(128),
 	type: z.enum(['instance', 'project']),
-	order: z.number().int(),
+	order: z.number().int().min(0).optional(),
 	projectIds: z.array(z.string()).optional(),
 }) {}

packages/cli/src/modules/provisioning.ee/__tests__/role-mapping-rule.service.ee.test.ts

@@ -52,12 +52,26 @@ const projectRole: Role = {
 };
 
 describe('RoleMappingRuleService', () => {
+	const defaultUpdateSpy = jest.fn().mockResolvedValue(undefined);
+	const defaultTransactionSpy = jest
+		.fn()
+		.mockImplementation(async (cb: (tx: { update: typeof defaultUpdateSpy }) => Promise<void>) => {
+			await cb({ update: defaultUpdateSpy });
+		});
+
 	beforeEach(() => {
 		jest.clearAllMocks();
 		roleMappingRuleRepository.findOne.mockResolvedValue(null);
 		// normalizeOrderForType calls find after every mutation; default to empty
 		// so existing tests hit the early-exit path and require no transaction mock.
 		roleMappingRuleRepository.find.mockResolvedValue([]);
+		// create() always calls applyOrder (which uses a transaction) to splice the
+		// newly-saved rule into the existing order. Inner describes may override.
+		(roleMappingRuleRepository as unknown as Record<string, unknown>).manager = {
+			transaction: defaultTransactionSpy,
+		};
+		defaultUpdateSpy.mockClear();
+		defaultTransactionSpy.mockClear();
 	});
 
 	describe('create', () => {
@@ -134,22 +148,6 @@ describe('RoleMappingRuleService', () => {
 			).rejects.toThrow(BadRequestError);
 		});
 
-		it('should reject when another rule already uses the same type and order', async () => {
-			roleRepository.findOne.mockResolvedValue(globalRole);
-			roleMappingRuleRepository.findOne.mockResolvedValue({
-				id: 'f47ac10b-58cc-4372-a567-0e02b2c3d479',
-			} as RoleMappingRule);
-
-			await expect(
-				service.create({
-					expression: 'true',
-					role: globalRole.slug,
-					type: 'instance',
-					order: 2,
-				}),
-			).rejects.toThrow(ConflictError);
-		});
-
 		it('should reject when some project ids do not exist', async () => {
 			roleRepository.findOne.mockResolvedValue(projectRole);
 			projectRepository.findBy.mockResolvedValue([{ id: 'p1' } as Project]);
@@ -285,6 +283,146 @@ describe('RoleMappingRuleService', () => {
 			expect(result.projectIds).toEqual([projA.id]);
 			expect(projectRepository.findBy).toHaveBeenCalledTimes(1);
 		});
+
+		it('should append when order is omitted', async () => {
+			roleRepository.findOne.mockResolvedValue(globalRole);
+			roleMappingRuleRepository.find.mockResolvedValue([
+				{ id: 'rule-a', order: 0 } as RoleMappingRule,
+				{ id: 'rule-b', order: 1 } as RoleMappingRule,
+			]);
+
+			const savedRule = {
+				id: 'rule-c',
+				expression: 'claims.c',
+				role: globalRole,
+				type: 'instance',
+				order: 2,
+				projects: [],
+				createdAt: new Date('2025-01-01T00:00:00.000Z'),
+				updatedAt: new Date('2025-01-01T00:00:00.000Z'),
+			} as unknown as RoleMappingRule;
+
+			roleMappingRuleRepository.save.mockImplementation(async (r) => ({
+				...(r as RoleMappingRule),
+				id: 'rule-c',
+			}));
+			roleMappingRuleRepository.findOneOrFail.mockResolvedValue(savedRule);
+
+			await service.create({
+				expression: 'claims.c',
+				role: globalRole.slug,
+				type: 'instance',
+			});
+
+			// applyOrder should renumber [rule-a, rule-b, rule-c] to [0, 1, 2]
+			expect(defaultUpdateSpy).toHaveBeenCalledWith(
+				expect.anything(),
+				{ id: 'rule-a' },
+				{ order: 0 },
+			);
+			expect(defaultUpdateSpy).toHaveBeenCalledWith(
+				expect.anything(),
+				{ id: 'rule-b' },
+				{ order: 1 },
+			);
+			expect(defaultUpdateSpy).toHaveBeenCalledWith(
+				expect.anything(),
+				{ id: 'rule-c' },
+				{ order: 2 },
+			);
+		});
+
+		it('should insert at index 0 and shift existing rules down', async () => {
+			roleRepository.findOne.mockResolvedValue(globalRole);
+			roleMappingRuleRepository.find.mockResolvedValue([
+				{ id: 'rule-a', order: 0 } as RoleMappingRule,
+				{ id: 'rule-b', order: 1 } as RoleMappingRule,
+			]);
+
+			const savedRule = {
+				id: 'rule-new',
+				expression: 'claims.new',
+				role: globalRole,
+				type: 'instance',
+				order: 0,
+				projects: [],
+				createdAt: new Date('2025-01-01T00:00:00.000Z'),
+				updatedAt: new Date('2025-01-01T00:00:00.000Z'),
+			} as unknown as RoleMappingRule;
+
+			roleMappingRuleRepository.save.mockImplementation(async (r) => ({
+				...(r as RoleMappingRule),
+				id: 'rule-new',
+			}));
+			roleMappingRuleRepository.findOneOrFail.mockResolvedValue(savedRule);
+
+			await service.create({
+				expression: 'claims.new',
+				role: globalRole.slug,
+				type: 'instance',
+				order: 0,
+			});
+
+			// applyOrder should renumber [rule-new, rule-a, rule-b] to [0, 1, 2]
+			expect(defaultUpdateSpy).toHaveBeenCalledWith(
+				expect.anything(),
+				{ id: 'rule-new' },
+				{ order: 0 },
+			);
+			expect(defaultUpdateSpy).toHaveBeenCalledWith(
+				expect.anything(),
+				{ id: 'rule-a' },
+				{ order: 1 },
+			);
+			expect(defaultUpdateSpy).toHaveBeenCalledWith(
+				expect.anything(),
+				{ id: 'rule-b' },
+				{ order: 2 },
+			);
+		});
+
+		it('should clamp supplied order beyond existing list length to the end', async () => {
+			roleRepository.findOne.mockResolvedValue(globalRole);
+			roleMappingRuleRepository.find.mockResolvedValue([
+				{ id: 'rule-a', order: 0 } as RoleMappingRule,
+			]);
+
+			const savedRule = {
+				id: 'rule-new',
+				expression: 'claims.new',
+				role: globalRole,
+				type: 'instance',
+				order: 1,
+				projects: [],
+				createdAt: new Date('2025-01-01T00:00:00.000Z'),
+				updatedAt: new Date('2025-01-01T00:00:00.000Z'),
+			} as unknown as RoleMappingRule;
+
+			roleMappingRuleRepository.save.mockImplementation(async (r) => ({
+				...(r as RoleMappingRule),
+				id: 'rule-new',
+			}));
+			roleMappingRuleRepository.findOneOrFail.mockResolvedValue(savedRule);
+
+			await service.create({
+				expression: 'claims.new',
+				role: globalRole.slug,
+				type: 'instance',
+				order: 999,
+			});
+
+			// Clamped to end: [rule-a, rule-new] → orders [0, 1]
+			expect(defaultUpdateSpy).toHaveBeenCalledWith(
+				expect.anything(),
+				{ id: 'rule-a' },
+				{ order: 0 },
+			);
+			expect(defaultUpdateSpy).toHaveBeenCalledWith(
+				expect.anything(),
+				{ id: 'rule-new' },
+				{ order: 1 },
+			);
+		});
 	});
 
 	describe('list', () => {
@@ -645,35 +783,6 @@ describe('RoleMappingRuleService', () => {
 			expect(updateSpy).toHaveBeenCalledWith(expect.anything(), { id: 'c' }, { order: 2 });
 		});
 
-		it('should compact a large gap after create', async () => {
-			// Simulates [0, 1, 100] after creating a rule at order 100
-			roleMappingRuleRepository.find.mockResolvedValue([
-				makeRule('a', 0),
-				makeRule('b', 1),
-				makeRule('c', 100),
-			]);
-
-			roleRepository.findOne.mockResolvedValue(globalRole);
-			roleMappingRuleRepository.save.mockResolvedValue(makeRule('c', 100));
-			roleMappingRuleRepository.findOneOrFail.mockResolvedValue({
-				...makeRule('c', 2),
-				role: globalRole,
-				projects: [],
-				createdAt: new Date('2025-01-01T00:00:00.000Z'),
-				updatedAt: new Date('2025-01-01T00:00:00.000Z'),
-			} as unknown as RoleMappingRule);
-
-			await service.create({
-				expression: 'true',
-				role: globalRole.slug,
-				type: 'instance',
-				order: 100,
-			});
-
-			expect(transactionSpy).toHaveBeenCalledTimes(1);
-			expect(updateSpy).toHaveBeenCalledWith(expect.anything(), { id: 'c' }, { order: 2 });
-		});
-
 		it('should normalize both types when type changes during patch', async () => {
 			const existingRule = {
 				id: 'rule-1',

packages/cli/src/modules/provisioning.ee/role-mapping-rule.service.ee.ts

@@ -90,8 +90,6 @@ export class RoleMappingRuleService {
 
 		assertRoleCompatibleWithMappingType(role, dto.type);
 
-		await this.assertOrderAvailable(dto.type, dto.order);
-
 		const projects =
 			uniqueProjectIds.length > 0
 				? await this.projectRepository.findBy({ id: In(uniqueProjectIds) })
@@ -101,16 +99,37 @@ export class RoleMappingRuleService {
 			throw new BadRequestError('One or more projects were not found');
 		}
 
+		const existingRules = await this.roleMappingRuleRepository.find({
+			where: { type: dto.type },
+			select: ['id', 'order'],
+			order: { order: 'ASC' },
+		});
+
+		// Clamp the requested index into the valid range. Omitted order appends.
+		const requestedOrder = dto.order ?? existingRules.length;
+		const targetIndex = Math.min(Math.max(requestedOrder, 0), existingRules.length);
+
+		// Save the new rule at a temporary order beyond any currently-used slot,
+		// so the unique (type, order) constraint cannot fire on the initial insert.
+		const maxOrder = existingRules.length > 0 ? existingRules[existingRules.length - 1].order : -1;
+		const tempOrder = Math.max(maxOrder, existingRules.length - 1) + 1;
+
 		const rule = new RoleMappingRule();
 		rule.expression = dto.expression;
 		rule.role = role;
 		rule.type = dto.type;
-		rule.order = dto.order;
+		rule.order = tempOrder;
 		rule.projects = projects;
 
 		const saved = await this.roleMappingRuleRepository.save(rule);
 
-		await this.normalizeOrderForType(dto.type);
+		// Build the final ordering: existing rules in their current order, with the
+		// newly saved rule spliced in at targetIndex. applyOrder atomically renumbers
+		// everything to [0..n-1] using its two-phase transaction.
+		const reorderedIds = existingRules.map((r) => r.id);
+		reorderedIds.splice(targetIndex, 0, saved.id);
+
+		await this.applyOrder(reorderedIds);
 
 		const loaded = await this.roleMappingRuleRepository.findOneOrFail({
 			where: { id: saved.id },