test(core): Restructure and dedupe the workflow review test suites (no-changelog) (#37243)

packages/@n8n/api-types/src/dto/workflow-reviews/__tests__/create-workflow-review-request.dto.test.ts

@@ -5,7 +5,7 @@ describe('CreateWorkflowReviewRequestDto', () => {
 	const base = { title: 'Please review', reviewerUserIds: ['reviewer-1'] };
 	const pinnedWorkflow = [{ ...workflow, workflowVersionName: 'Release candidate' }];
 
-	test('should accept a version name on the pinned workflow', () => {
+	test('accepts a version name on the pinned workflow', () => {
 		const result = CreateWorkflowReviewRequestDto.safeParse({
 			...base,
 			workflows: [{ ...workflow, workflowVersionName: 'Release candidate' }],
@@ -15,7 +15,7 @@ describe('CreateWorkflowReviewRequestDto', () => {
 		expect(result.data?.workflows[0].workflowVersionName).toBe('Release candidate');
 	});
 
-	test('should reject a version name longer than 128 characters', () => {
+	test('rejects a version name longer than 128 characters', () => {
 		const result = CreateWorkflowReviewRequestDto.safeParse({
 			...base,
 			workflows: [{ ...workflow, workflowVersionName: 'a'.repeat(129) }],
@@ -25,7 +25,7 @@ describe('CreateWorkflowReviewRequestDto', () => {
 		expect(result.error?.issues[0].path).toEqual(['workflows', 0, 'workflowVersionName']);
 	});
 
-	test('should trim the version name', () => {
+	test('trims the version name', () => {
 		const result = CreateWorkflowReviewRequestDto.safeParse({
 			...base,
 			workflows: [{ ...workflow, workflowVersionName: '  Release candidate  ' }],
@@ -39,7 +39,7 @@ describe('CreateWorkflowReviewRequestDto', () => {
 		{ name: 'a missing version name', workflowVersionName: undefined },
 		{ name: 'an empty version name', workflowVersionName: '' },
 		{ name: 'a whitespace-only version name', workflowVersionName: '   ' },
-	])('should reject $name', ({ workflowVersionName }) => {
+	])('rejects $name', ({ workflowVersionName }) => {
 		const result = CreateWorkflowReviewRequestDto.safeParse({
 			...base,
 			workflows: [{ ...workflow, workflowVersionName }],
@@ -65,7 +65,7 @@ describe('CreateWorkflowReviewRequestDto', () => {
 		expect(result.data?.workflows[0].workflowVersionDescription).toBe(workflowVersionDescription);
 	});
 
-	test('should trim the review description', () => {
+	test('trims the review description', () => {
 		const result = CreateWorkflowReviewRequestDto.safeParse({
 			...base,
 			description: '  Please take a look  ',
@@ -79,7 +79,7 @@ describe('CreateWorkflowReviewRequestDto', () => {
 	test.each([
 		{ name: 'an empty', description: '' },
 		{ name: 'a whitespace-only', description: '   ' },
-	])('should reduce $name review description to an empty string', ({ description }) => {
+	])('reduces $name review description to an empty string', ({ description }) => {
 		const result = CreateWorkflowReviewRequestDto.safeParse({
 			...base,
 			description,
@@ -90,7 +90,7 @@ describe('CreateWorkflowReviewRequestDto', () => {
 		expect(result.data?.description).toBe('');
 	});
 
-	test('should reject a review description longer than 512 characters', () => {
+	test('rejects a review description longer than 512 characters', () => {
 		const result = CreateWorkflowReviewRequestDto.safeParse({
 			...base,
 			description: 'a'.repeat(513),
@@ -101,7 +101,7 @@ describe('CreateWorkflowReviewRequestDto', () => {
 		expect(result.error?.issues[0].path).toEqual(['description']);
 	});
 
-	test('should reject a version description longer than 2048 characters', () => {
+	test('rejects a version description longer than 2048 characters', () => {
 		const result = CreateWorkflowReviewRequestDto.safeParse({
 			...base,
 			workflows: [
@@ -117,6 +117,51 @@ describe('CreateWorkflowReviewRequestDto', () => {
 		expect(result.error?.issues[0].path).toEqual(['workflows', 0, 'workflowVersionDescription']);
 	});
 
+	// The title and the single-workflow shape are enforced here rather than over
+	// HTTP, so the review suites do not re-test the schema through the whole stack.
+	describe('title', () => {
+		test('trims it', () => {
+			const result = CreateWorkflowReviewRequestDto.safeParse({
+				...base,
+				title: '  Please review  ',
+				workflows: pinnedWorkflow,
+			});
+
+			expect(result.success).toBe(true);
+			expect(result.data?.title).toBe('Please review');
+		});
+
+		test.each([
+			{ name: 'a missing title', title: undefined },
+			{ name: 'an empty title', title: '' },
+			{ name: 'a whitespace-only title', title: '   ' },
+			{ name: 'a title longer than 128 characters', title: 'a'.repeat(129) },
+		])('rejects $name', ({ title }) => {
+			const result = CreateWorkflowReviewRequestDto.safeParse({
+				reviewerUserIds: ['reviewer-1'],
+				title,
+				workflows: pinnedWorkflow,
+			});
+
+			expect(result.success).toBe(false);
+			expect(result.error?.issues[0].path).toEqual(['title']);
+		});
+	});
+
+	// A review covers exactly one workflow today; the array is there for LIGO-601.
+	describe('workflows', () => {
+		test.each([
+			{ name: 'no workflow', workflows: [] },
+			{ name: 'more than one workflow', workflows: [...pinnedWorkflow, ...pinnedWorkflow] },
+			{ name: 'a missing workflows array', workflows: undefined },
+		])('rejects $name', ({ workflows }) => {
+			const result = CreateWorkflowReviewRequestDto.safeParse({ ...base, workflows });
+
+			expect(result.success).toBe(false);
+			expect(result.error?.issues[0].path).toEqual(['workflows']);
+		});
+	});
+
 	describe('reviewerUserIds', () => {
 		test.each([
 			{ name: 'a single reviewer', reviewerUserIds: ['reviewer-1'] },
@@ -124,7 +169,7 @@ describe('CreateWorkflowReviewRequestDto', () => {
 				name: 'ten reviewers',
 				reviewerUserIds: Array.from({ length: 10 }, (_, i) => `reviewer-${i}`),
 			},
-		])('should accept $name', ({ reviewerUserIds }) => {
+		])('accepts $name', ({ reviewerUserIds }) => {
 			const result = CreateWorkflowReviewRequestDto.safeParse({
 				title: 'Please review',
 				reviewerUserIds,
@@ -143,7 +188,7 @@ describe('CreateWorkflowReviewRequestDto', () => {
 				reviewerUserIds: Array.from({ length: 11 }, (_, i) => `reviewer-${i}`),
 			},
 			{ name: 'a non-array reviewer value', reviewerUserIds: 'reviewer-1' },
-		])('should reject $name', ({ reviewerUserIds }) => {
+		])('rejects $name', ({ reviewerUserIds }) => {
 			const result = CreateWorkflowReviewRequestDto.safeParse({
 				title: 'Please review',
 				reviewerUserIds,

packages/@n8n/api-types/src/dto/workflow-reviews/__tests__/decide-workflow-review-request.dto.test.ts

@@ -1,7 +1,7 @@
 import { DecideWorkflowReviewRequestDto } from '../decide-workflow-review-request.dto';
 
 describe('DecideWorkflowReviewRequestDto', () => {
-	describe('Valid requests', () => {
+	describe('accepted', () => {
 		test.each([
 			{ name: 'approved decision', request: { decision: 'approved' } },
 			{ name: 'changes_requested decision', request: { decision: 'changes_requested' } },
@@ -13,7 +13,7 @@ describe('DecideWorkflowReviewRequestDto', () => {
 				name: 'changes_requested decision with a note',
 				request: { decision: 'changes_requested', note: 'Please rename the node' },
 			},
-		])('should validate $name', ({ request }) => {
+		])('accepts $name', ({ request }) => {
 			const result = DecideWorkflowReviewRequestDto.safeParse(request);
 			expect(result.success).toBe(true);
 			expect(result.data).toMatchObject(request);
@@ -21,7 +21,7 @@ describe('DecideWorkflowReviewRequestDto', () => {
 
 		// Its own test because the table above asserts `toMatchObject(request)`, which a row
 		// whose point is that the output differs from the input cannot satisfy.
-		test('should trim the note', () => {
+		test('trims the note', () => {
 			const result = DecideWorkflowReviewRequestDto.safeParse({
 				decision: 'approved',
 				note: '  first line\nsecond line  ',
@@ -31,7 +31,7 @@ describe('DecideWorkflowReviewRequestDto', () => {
 		});
 	});
 
-	describe('Invalid requests', () => {
+	describe('rejected', () => {
 		test.each([
 			{
 				name: 'missing decision',
@@ -68,7 +68,7 @@ describe('DecideWorkflowReviewRequestDto', () => {
 				request: { decision: 'changes_requested', note: 'oops \x00 here' },
 				expectedErrorPath: ['note'],
 			},
-		])('should fail validation for $name', ({ request, expectedErrorPath }) => {
+		])('rejects $name', ({ request, expectedErrorPath }) => {
 			const result = DecideWorkflowReviewRequestDto.safeParse(request);
 			expect(result.success).toBe(false);
 			expect(result.error?.issues[0].path).toEqual(expectedErrorPath);

packages/@n8n/api-types/src/dto/workflow-reviews/__tests__/list-workflow-review-inbox-query.dto.test.ts

@@ -1,7 +1,7 @@
 import { ListWorkflowReviewInboxQueryDto } from '../list-workflow-review-inbox.dto';
 
 describe('ListWorkflowReviewInboxQueryDto', () => {
-	describe('Valid requests', () => {
+	describe('accepted', () => {
 		test.each([
 			{
 				name: 'no filters at all',
@@ -23,20 +23,20 @@ describe('ListWorkflowReviewInboxQueryDto', () => {
 				request: { category: 'authored', state: 'closed', limit: '30', cursor: 'abc' },
 				parsedResult: { category: 'authored', state: 'closed', limit: 30, cursor: 'abc' },
 			},
-		])('should validate $name', ({ request, parsedResult }) => {
+		])('accepts $name', ({ request, parsedResult }) => {
 			const result = ListWorkflowReviewInboxQueryDto.safeParse(request);
 			expect(result.success).toBe(true);
 			expect(result.data).toMatchObject(parsedResult);
 		});
 	});
 
-	describe('Invalid requests', () => {
+	describe('rejected', () => {
 		test.each([
 			{ name: 'unknown category', request: { category: 'mine' } },
 			{ name: 'empty category', request: { category: '' } },
 			{ name: 'boolean-ish category', request: { category: 'true' } },
 			{ name: 'category as an array', request: { category: ['waiting'] } },
-		])('should fail validation for $name', ({ request }) => {
+		])('rejects $name', ({ request }) => {
 			const result = ListWorkflowReviewInboxQueryDto.safeParse(request);
 			expect(result.success).toBe(false);
 			expect(result.error?.issues[0].path).toEqual(['category']);

packages/@n8n/api-types/src/dto/workflow-reviews/__tests__/list-workflow-review-requests-query.dto.test.ts

@@ -3,7 +3,7 @@ import { ListWorkflowReviewRequestsQueryDto } from '../list-workflow-review-requ
 const DEFAULT_PAGINATION = { skip: 0, take: 10 };
 
 describe('ListWorkflowReviewRequestsQueryDto', () => {
-	describe('Valid requests', () => {
+	describe('accepted', () => {
 		test.each([
 			{
 				name: 'workflowId only',
@@ -25,7 +25,7 @@ describe('ListWorkflowReviewRequestsQueryDto', () => {
 				request: { workflowId: 'workflow-1', skip: '5', take: '1' },
 				parsedResult: { workflowId: 'workflow-1', skip: 5, take: 1 },
 			},
-		])('should validate $name', ({ request, parsedResult }) => {
+		])('accepts $name', ({ request, parsedResult }) => {
 			const result = ListWorkflowReviewRequestsQueryDto.safeParse(request);
 			expect(result.success).toBe(true);
 			if (parsedResult) {
@@ -34,7 +34,7 @@ describe('ListWorkflowReviewRequestsQueryDto', () => {
 		});
 	});
 
-	describe('Invalid requests', () => {
+	describe('rejected', () => {
 		test.each([
 			{
 				name: 'missing workflowId',
@@ -61,7 +61,7 @@ describe('ListWorkflowReviewRequestsQueryDto', () => {
 				request: { workflowId: 'workflow-1', skip: '-1' },
 				expectedErrorPath: ['skip'],
 			},
-		])('should fail validation for $name', ({ request, expectedErrorPath }) => {
+		])('rejects $name', ({ request, expectedErrorPath }) => {
 			const result = ListWorkflowReviewRequestsQueryDto.safeParse(request);
 			expect(result.success).toBe(false);
 			if (expectedErrorPath) {

packages/@n8n/api-types/src/dto/workflow-reviews/__tests__/update-workflow-review-request-version.dto.test.ts

@@ -1,7 +1,7 @@
 import { UpdateWorkflowReviewRequestVersionDto } from '../update-workflow-review-request-version.dto';
 
 describe('UpdateWorkflowReviewRequestVersionDto', () => {
-	describe('Valid requests', () => {
+	describe('accepted', () => {
 		test.each([
 			{
 				name: 'workflowId, workflowVersionId and workflowVersionName',
@@ -47,13 +47,13 @@ describe('UpdateWorkflowReviewRequestVersionDto', () => {
 					description: '',
 				},
 			},
-		])('should validate $name', ({ request }) => {
+		])('accepts $name', ({ request }) => {
 			const result = UpdateWorkflowReviewRequestVersionDto.safeParse(request);
 			expect(result.success).toBe(true);
 			expect(result.data).toMatchObject(request);
 		});
 
-		test('should trim the workflowVersionName', () => {
+		test('trims the workflowVersionName', () => {
 			const result = UpdateWorkflowReviewRequestVersionDto.safeParse({
 				workflowId: 'workflow-1',
 				workflowVersionId: 'version-1',
@@ -65,7 +65,7 @@ describe('UpdateWorkflowReviewRequestVersionDto', () => {
 		});
 	});
 
-	describe('Invalid requests', () => {
+	describe('rejected', () => {
 		test.each([
 			{
 				name: 'missing workflowId',
@@ -139,7 +139,7 @@ describe('UpdateWorkflowReviewRequestVersionDto', () => {
 				},
 				expectedErrorPath: ['description'],
 			},
-		])('should fail validation for $name', ({ request, expectedErrorPath }) => {
+		])('rejects $name', ({ request, expectedErrorPath }) => {
 			const result = UpdateWorkflowReviewRequestVersionDto.safeParse(request);
 			expect(result.success).toBe(false);
 			expect(result.error?.issues[0].path).toEqual(expectedErrorPath);

packages/cli/src/modules/workflow-reviews.ee/__tests__/support/workflow-review-test-data.ts

@@ -0,0 +1,252 @@
+import {
+	createTeamProject,
+	createWorkflow,
+	getPersonalProject,
+	linkUserToProject,
+} from '@n8n/backend-test-utils';
+import type { Project, User } from '@n8n/db';
+import {
+	WorkflowHistoryRepository,
+	WorkflowReviewRequestAuthorRepository,
+	WorkflowReviewRequestRepository,
+	WorkflowReviewRequestReviewerRepository,
+	WorkflowReviewRequestWorkflowRepository,
+} from '@n8n/db';
+import { Container } from '@n8n/di';
+import { v4 as uuid } from 'uuid';
+import type { MockProxy } from 'vitest-mock-extended';
+
+import type { WorkflowValidationService } from '@/workflows/workflow-validation.service';
+import { createMember, createOwner } from '@test-integration/db/users';
+import { createWorkflowHistoryItem } from '@test-integration/db/workflow-history';
+import type { SuperAgentTest } from '@test-integration/types';
+
+/**
+ * Shared fixtures for the workflow-review integration suites. Each suite still
+ * calls `setupTestServer` and `mockInstance` itself: those register `beforeAll`
+ * hooks at call time, so they have to run while the test file's own module is
+ * being evaluated.
+ */
+
+/**
+ * Truncation order matters: `WorkflowPublishedVersion` FKs onto `WorkflowHistory`
+ * with `onDelete RESTRICT`, so it has to go first.
+ */
+export const REVIEW_TABLES = [
+	'WorkflowReviewActivityComment',
+	'WorkflowReviewActivity',
+	'WorkflowReviewRequestAuthor',
+	'WorkflowReviewRequestReviewer',
+	'WorkflowReviewRequestWorkflow',
+	'WorkflowReviewRequest',
+	'SharedWorkflow',
+	'WorkflowPublishedVersion',
+	'WorkflowPublicationOutbox',
+	'WorkflowPublishHistory',
+	'WorkflowEntity',
+	'WorkflowHistory',
+	'ProjectRelation',
+	'Project',
+	'User',
+] as const;
+
+/** Test workflows carry no trigger nodes, so activation must not fail on that. */
+export function stubWorkflowValidation(
+	workflowValidationService: MockProxy<WorkflowValidationService>,
+): void {
+	workflowValidationService.validateForActivation.mockReturnValue({ isValid: true });
+	workflowValidationService.validateDynamicCredentials.mockResolvedValue({ isValid: true });
+	workflowValidationService.validateSubWorkflowReferences.mockResolvedValue({ isValid: true });
+	workflowValidationService.validateTriggerNodeIds.mockReturnValue({ isValid: true });
+}
+
+export interface ReviewActors {
+	owner: User;
+	/** `project:editor` on `teamProject`, so they hold `workflow:publish` there. */
+	member: User;
+	/** `project:viewer` on `teamProject`, so they hold `workflow:read` only. */
+	viewer: User;
+	ownerProject: Project;
+	teamProject: Project;
+}
+
+/** The cast of users and projects every review suite builds its scenarios from. */
+export async function seedReviewActors(authAgentFor: (user: User) => SuperAgentTest): Promise<
+	ReviewActors & {
+		ownerAgent: SuperAgentTest;
+		memberAgent: SuperAgentTest;
+		viewerAgent: SuperAgentTest;
+	}
+> {
+	const owner = await createOwner();
+	const member = await createMember();
+	const viewer = await createMember();
+	const ownerProject = await getPersonalProject(owner);
+	const teamProject = await createTeamProject('Reviews Project', owner);
+	await linkUserToProject(member, teamProject, 'project:editor');
+	await linkUserToProject(viewer, teamProject, 'project:viewer');
+
+	return {
+		owner,
+		member,
+		viewer,
+		ownerProject,
+		teamProject,
+		ownerAgent: authAgentFor(owner),
+		memberAgent: authAgentFor(member),
+		viewerAgent: authAgentFor(viewer),
+	};
+}
+
+/** A workflow with one history version, ready to be submitted for review. */
+export async function createReviewableWorkflow(
+	ownerOrProject: User | Project,
+	{
+		versionId = uuid(),
+		...attributes
+	}: { versionId?: string; isArchived?: boolean; name?: string } = {},
+) {
+	const workflow = await createWorkflow({ versionId, ...attributes }, ownerOrProject);
+	await createWorkflowHistoryItem(workflow.id, { versionId });
+	return { workflow, versionId };
+}
+
+export interface SeedReviewOptions {
+	projectId: string;
+	workflowId?: string;
+	/** `null` pins no version; omit to link no workflow at all. */
+	versionId?: string | null;
+	author: User;
+	reviewerIds?: string[];
+	state?: 'open' | 'closed';
+	decision?: 'pending' | 'changes_requested' | 'approved';
+	title?: string;
+	description?: string | null;
+	/** Who last touched the review — the decision actor on a decided review. */
+	updatedById?: string | null;
+}
+
+/**
+ * Seed a review the way the create endpoint does: the request, its workflow link,
+ * an author row for the requester, and any assigned reviewers.
+ */
+export async function seedReview({
+	projectId,
+	workflowId,
+	versionId = null,
+	author,
+	reviewerIds = [],
+	state,
+	decision,
+	title = 'Review before publishing',
+	description,
+	updatedById,
+}: SeedReviewOptions) {
+	const request = await Container.get(WorkflowReviewRequestRepository).createRequest(
+		{ projectId, title, description, createdById: author.id, state, decision, updatedById },
+		{},
+	);
+
+	if (workflowId !== undefined) {
+		await Container.get(WorkflowReviewRequestWorkflowRepository).createWorkflowRow(
+			{ workflowReviewRequestId: request.id, workflowId, workflowVersionId: versionId },
+			{},
+		);
+	}
+
+	await Container.get(WorkflowReviewRequestAuthorRepository).addAuthor(
+		{ workflowReviewRequestId: request.id, userId: author.id },
+		{},
+	);
+
+	if (reviewerIds.length > 0) {
+		await Container.get(WorkflowReviewRequestReviewerRepository).addReviewers(
+			{ workflowReviewRequestId: request.id, userIds: reviewerIds },
+			{},
+		);
+	}
+
+	return request;
+}
+
+/** The single-workflow `workflows` array every submission payload carries. */
+export function reviewPayload({
+	workflowId,
+	versionId,
+	versionName = 'Release candidate',
+	versionDescription,
+	...rest
+}: {
+	workflowId: string;
+	versionId: string;
+	versionName?: string;
+	versionDescription?: string;
+	title?: string;
+	description?: string;
+	reviewerUserIds?: string[];
+}) {
+	return {
+		title: 'Please review my workflow',
+		...rest,
+		workflows: [
+			{
+				workflowId,
+				workflowVersionId: versionId,
+				workflowVersionName: versionName,
+				...(versionDescription === undefined
+					? {}
+					: { workflowVersionDescription: versionDescription }),
+			},
+		],
+	};
+}
+
+/** The body of a version-update request. */
+export function versionUpdatePayload({
+	workflowId,
+	versionId,
+	versionName = 'Release candidate',
+	...rest
+}: {
+	workflowId: string;
+	versionId: string;
+	versionName?: string;
+	workflowVersionDescription?: string;
+	description?: string;
+}) {
+	return {
+		workflowId,
+		workflowVersionId: versionId,
+		workflowVersionName: versionName,
+		...rest,
+	};
+}
+
+export type ActivityFeedEntry = {
+	id: string;
+	type: string;
+	data: Record<string, unknown> | null;
+	createdBy: { id: string } | null;
+	messages?: Array<{ body: string | null; createdBy: unknown; deletedAt: string | null }>;
+};
+
+/** Read the feed through the endpoint, so entries are asserted as a reader sees them. */
+export async function readActivityFeed(
+	agent: SuperAgentTest,
+	requestId: string,
+	limit?: number,
+): Promise<{ data: ActivityFeedEntry[]; nextCursor: string | null; hasMore: boolean }> {
+	const response = await agent
+		.get(`/workflow-review-requests/${requestId}/activity`)
+		.query(limit === undefined ? {} : { limit })
+		.expect(200);
+	return response.body.data;
+}
+
+export async function findVersionName(workflowId: string, versionId: string) {
+	const version = await Container.get(WorkflowHistoryRepository).findOneBy({
+		workflowId,
+		versionId,
+	});
+	return version?.name;
+}

packages/cli/src/modules/workflow-reviews.ee/__tests__/workflow-review-activity.integration.test.ts

@@ -1,17 +1,10 @@
-import {
-	createTeamProject,
-	createWorkflow,
-	linkUserToProject,
-	mockInstance,
-	testDb,
-} from '@n8n/backend-test-utils';
+import { createTeamProject, createWorkflow, mockInstance, testDb } from '@n8n/backend-test-utils';
 import type { Project, User } from '@n8n/db';
 import {
 	UserRepository,
 	WorkflowRepository,
 	WorkflowReviewActivityCommentRepository,
 	WorkflowReviewActivityRepository,
-	WorkflowReviewRequestAuthorRepository,
 	WorkflowReviewRequestRepository,
 	WorkflowReviewRequestReviewerRepository,
 	WorkflowReviewRequestWorkflowRepository,
@@ -22,11 +15,19 @@ import { ActiveWorkflowManager } from '@/active-workflow-manager';
 import { EventService } from '@/events/event.service';
 import { WorkflowReviewPolicyService } from '@/services/workflow-review-policy.service';
 import { WorkflowValidationService } from '@/workflows/workflow-validation.service';
-import { createMember, createOwner } from '@test-integration/db/users';
 import { createWorkflowHistoryItem } from '@test-integration/db/workflow-history';
 import type { SuperAgentTest } from '@test-integration/types';
 import * as utils from '@test-integration/utils';
 
+import {
+	type ActivityFeedEntry,
+	readActivityFeed,
+	REVIEW_TABLES,
+	seedReview,
+	seedReviewActors,
+	stubWorkflowValidation,
+} from './support/workflow-review-test-data';
+
 mockInstance(ActiveWorkflowManager);
 const workflowValidationService = mockInstance(WorkflowValidationService);
 
@@ -46,7 +47,6 @@ let viewerAgent: SuperAgentTest;
 
 let requestRepository: WorkflowReviewRequestRepository;
 let workflowRepository: WorkflowReviewRequestWorkflowRepository;
-let authorRepository: WorkflowReviewRequestAuthorRepository;
 let activityRepository: WorkflowReviewActivityRepository;
 let activityCommentRepository: WorkflowReviewActivityCommentRepository;
 let userRepository: UserRepository;
@@ -57,7 +57,6 @@ beforeAll(async () => {
 	await utils.initNodeTypes();
 	requestRepository = Container.get(WorkflowReviewRequestRepository);
 	workflowRepository = Container.get(WorkflowReviewRequestWorkflowRepository);
-	authorRepository = Container.get(WorkflowReviewRequestAuthorRepository);
 	activityRepository = Container.get(WorkflowReviewActivityRepository);
 	activityCommentRepository = Container.get(WorkflowReviewActivityCommentRepository);
 	userRepository = Container.get(UserRepository);
@@ -67,40 +66,12 @@ beforeAll(async () => {
 
 beforeEach(async () => {
 	testServer.license.enable('feat:workflowReviews');
-
-	await testDb.truncate([
-		'WorkflowReviewActivityComment',
-		'WorkflowReviewActivity',
-		'WorkflowReviewRequestAuthor',
-		'WorkflowReviewRequestReviewer',
-		'WorkflowReviewRequestWorkflow',
-		'WorkflowReviewRequest',
-		'SharedWorkflow',
-		'WorkflowPublishedVersion',
-		'WorkflowPublicationOutbox',
-		'WorkflowPublishHistory',
-		'WorkflowEntity',
-		'WorkflowHistory',
-		'ProjectRelation',
-		'Project',
-		'User',
-	]);
-
+	await testDb.truncate([...REVIEW_TABLES]);
 	await policyService.set(true);
-	workflowValidationService.validateForActivation.mockReturnValue({ isValid: true });
-	workflowValidationService.validateDynamicCredentials.mockResolvedValue({ isValid: true });
-	workflowValidationService.validateSubWorkflowReferences.mockResolvedValue({ isValid: true });
+	stubWorkflowValidation(workflowValidationService);
 
-	owner = await createOwner();
-	member = await createMember();
-	viewer = await createMember();
-	teamProject = await createTeamProject('Reviews Project', owner);
-	await linkUserToProject(member, teamProject, 'project:editor');
-	await linkUserToProject(viewer, teamProject, 'project:viewer');
-
-	ownerAgent = testServer.authAgentFor(owner);
-	memberAgent = testServer.authAgentFor(member);
-	viewerAgent = testServer.authAgentFor(viewer);
+	({ owner, member, viewer, teamProject, ownerAgent, memberAgent, viewerAgent } =
+		await seedReviewActors(testServer.authAgentFor));
 });
 
 async function seedRequest(
@@ -109,25 +80,14 @@ async function seedRequest(
 	author: User,
 	projectId = teamProject.id,
 ) {
-	const request = await requestRepository.createRequest(
-		{
-			projectId,
-			title: 'Please review',
-			description: 'Some context',
-			createdById: author.id,
-		},
-		{},
-	);
-	await workflowRepository.createWorkflowRow(
-		{
-			workflowReviewRequestId: request.id,
-			workflowId,
-			workflowVersionId: versionId,
-		},
-		{},
-	);
-	await authorRepository.addAuthor({ workflowReviewRequestId: request.id, userId: author.id }, {});
-	return request;
+	return await seedReview({
+		projectId,
+		workflowId,
+		versionId,
+		author,
+		title: 'Please review',
+		description: 'Some context',
+	});
 }
 
 async function seedReviewInTeamProject(author: User) {
@@ -137,25 +97,7 @@ async function seedReviewInTeamProject(author: User) {
 	return { workflow, request };
 }
 
-type FeedEntry = {
-	id: string;
-	type: string;
-	data: unknown;
-	createdBy: unknown;
-	messages?: Array<{ body: string | null; createdBy: unknown; deletedAt: string | null }>;
-};
-
-async function getActivity(agent: SuperAgentTest, requestId: string, limit?: number) {
-	const response = await agent
-		.get(`/workflow-review-requests/${requestId}/activity`)
-		.query(limit === undefined ? {} : { limit })
-		.expect(200);
-	return response.body.data as {
-		data: FeedEntry[];
-		nextCursor: string | null;
-		hasMore: boolean;
-	};
-}
+const getActivity = readActivityFeed;
 
 describe('Commenting on a review', () => {
 	test('shows a comment in the feed the instant its writer posts it', async () => {
@@ -384,7 +326,7 @@ describe('Recording the review lifecycle in the feed', () => {
 		return response.body.data.id as string;
 	}
 
-	const entryTypes = (feed: { data: FeedEntry[] }) => feed.data.map((entry) => entry.type);
+	const entryTypes = (feed: { data: ActivityFeedEntry[] }) => feed.data.map((entry) => entry.type);
 
 	test('shows who opened the review and which version they submitted', async () => {
 		const workflow = await createReviewableWorkflow();
@@ -633,7 +575,7 @@ describe('Reading the activity feed', () => {
 		const pages: string[][] = [];
 		let cursor: string | null = null;
 		do {
-			const page: { data: FeedEntry[]; nextCursor: string | null } = (
+			const page: { data: ActivityFeedEntry[]; nextCursor: string | null } = (
 				await ownerAgent
 					.get(`/workflow-review-requests/${request.id}/activity`)
 					.query(cursor ? { limit: 2, cursor } : { limit: 2 })

packages/cli/src/modules/workflow-reviews.ee/__tests__/workflow-review-authorization.access.service.test.ts

@@ -35,7 +35,7 @@ function reviewRequest(overrides: Partial<WorkflowReviewRequest> = {}) {
 	});
 }
 
-describe('WorkflowReviewAuthorizationService: visibility and the read gate', () => {
+describe('WorkflowReviewAuthorizationService: who may see a review', () => {
 	const workflowFinderService = mock<WorkflowFinderService>();
 	const projectService = mock<ProjectService>();
 	const roleService = mock<RoleService>();
@@ -93,7 +93,7 @@ describe('WorkflowReviewAuthorizationService: visibility and the read gate', ()
 		projectService.getProjectIdsWithScope.mockResolvedValue(['proj-1']);
 	}
 
-	describe('who is allowed to open a review', () => {
+	describe('opening one review', () => {
 		it('reports a review that does not exist as not found', async () => {
 			requestRepository.findById.mockResolvedValue(null);
 
@@ -106,9 +106,6 @@ describe('WorkflowReviewAuthorizationService: visibility and the read gate', ()
 			mockReadableReviewProject();
 
 			// Same error as a review that does not exist: existence must not leak
-			await expect(service.findReadableRequestOrFail(member, requestId)).rejects.toThrow(
-				NotFoundError,
-			);
 			await expect(service.findReadableRequestOrFail(member, requestId)).rejects.toThrow(
 				'Could not find review request',
 			);
@@ -206,16 +203,6 @@ describe('WorkflowReviewAuthorizationService: visibility and the read gate', ()
 			expect(projectService.getProjectIdsWithScope).not.toHaveBeenCalled();
 		});
 
-		it('hides the review from anyone who can read none of the workflows it covers, the requester included', async () => {
-			mockReadableReviewProject();
-			mockChildRow();
-			workflowFinderService.findWorkflowForUser.mockResolvedValue(null);
-
-			await expect(service.findReadableRequestOrFail(requester, requestId)).rejects.toThrow(
-				NotFoundError,
-			);
-		});
-
 		it('returns the covered workflows together with the ones the caller may read', async () => {
 			mockReadableReviewProject();
 			mockChildRow();
@@ -289,7 +276,7 @@ describe('WorkflowReviewAuthorizationService: visibility and the read gate', ()
 		});
 	});
 
-	describe('resolveOpenableRequestIds', () => {
+	describe('which of many listed reviews the viewer can open', () => {
 		const requests = [
 			{ id: 'req-1', projectId: 'proj-1' },
 			{ id: 'req-2', projectId: 'proj-2' },
@@ -333,26 +320,9 @@ describe('WorkflowReviewAuthorizationService: visibility and the read gate', ()
 		it('opens nothing for an uninvolved non-admin, whatever workflow permissions they hold', async () => {
 			expect(await service.resolveOpenableRequestIds(member, requests)).toEqual(new Set());
 		});
-
-		it('answers exactly like the single-request detail gate', async () => {
-			// Same fixtures as the detail-gate tests above: assigned reviewer on req-1.
-			reviewerRepository.findRequestIdsForUser.mockResolvedValue(new Set(['req-1']));
-			reviewerRepository.isReviewer.mockImplementation(
-				async ({ workflowReviewRequestId }) => workflowReviewRequestId === 'req-1',
-			);
-			authorRepository.isAuthor.mockResolvedValue(false);
-			projectService.getProjectIdsWithScope.mockResolvedValue(['proj-1']);
-
-			const openable = await service.resolveOpenableRequestIds(member, requests);
-			expect(openable).toEqual(new Set(['req-1']));
-
-			await expect(service.findReadableRequestOrFail(member, requestId)).resolves.toMatchObject({
-				request: { id: requestId },
-			});
-		});
 	});
 
-	describe('resolveInboxVisibility', () => {
+	describe('whose reviews show up in the inbox', () => {
 		it('gives global admins and owners the whole inbox', async () => {
 			const owner = mock<User>({ role: { slug: 'global:owner', scopes: [] } });
 

packages/cli/src/modules/workflow-reviews.ee/__tests__/workflow-review-authorization.capabilities.service.test.ts

@@ -11,18 +11,25 @@ import type {
 } from '@n8n/db';
 import { mock } from 'vitest-mock-extended';
 
-import { WorkflowReviewAuthorizationService } from '../workflow-review-authorization.service';
-
 import type { ProjectService } from '@/services/project.service.ee';
 import type { RoleService } from '@/services/role.service';
 import type { WorkflowFinderService } from '@/workflows/workflow-finder.service';
 
+import { WorkflowReviewAuthorizationService } from '../workflow-review-authorization.service';
+
 const requestId = 'req-1';
 const projectId = 'proj-1';
 
 const memberUser = (id = 'user-1') => mock<User>({ id, role: { slug: 'global:member' } });
 
-describe('WorkflowReviewAuthorizationService: viewer capabilities', () => {
+/**
+ * What a viewer may do with a review they can see. The allow/deny rule itself is a
+ * pure function with its own truth table in `workflow-review-decision-policy.test.ts`;
+ * what this suite covers is the facts fed into it — who counts as an admin, and what
+ * "can read every covered workflow" resolves to — plus the commenting rule, which
+ * only exists here.
+ */
+describe('WorkflowReviewAuthorizationService.resolveViewerEligibility', () => {
 	const authorRepository = mock<WorkflowReviewRequestAuthorRepository>();
 	const reviewerRepository = mock<WorkflowReviewRequestReviewerRepository>();
 	const projectRelationRepository = mock<ProjectRelationRepository>();
@@ -62,113 +69,65 @@ describe('WorkflowReviewAuthorizationService: viewer capabilities', () => {
 		projectRelationRepository.getAccessibleProjectsByRoles.mockResolvedValue([]);
 	});
 
-	describe('who may decide', () => {
-		it('lets an assigned non-author viewer decide', async () => {
-			const eligibility = await service.resolveViewerEligibility(memberUser(), readable());
+	it('lets an assigned reviewer decide and comment', async () => {
+		const eligibility = await service.resolveViewerEligibility(memberUser(), readable());
 
-			expect(eligibility).toEqual({
-				canDecide: true,
-				decisionIneligibilityReason: null,
-				canComment: true,
-			});
+		expect(eligibility).toEqual({
+			canDecide: true,
+			decisionIneligibilityReason: null,
+			canComment: true,
 		});
+	});
 
-		it('reports a non-assigned viewer as ineligible', async () => {
-			reviewerRepository.isReviewer.mockResolvedValue(false);
+	it('refuses both to someone who is not assigned to review it', async () => {
+		reviewerRepository.isReviewer.mockResolvedValue(false);
 
-			const eligibility = await service.resolveViewerEligibility(memberUser(), readable());
+		const eligibility = await service.resolveViewerEligibility(memberUser(), readable());
 
-			expect(eligibility).toEqual({
-				canDecide: false,
-				decisionIneligibilityReason: 'missing_reviewer_permission',
-				canComment: false,
-			});
+		expect(eligibility).toEqual({
+			canDecide: false,
+			decisionIneligibilityReason: 'missing_reviewer_permission',
+			canComment: false,
 		});
+	});
 
-		it('lets an assigned reviewer decide even when they authored a version', async () => {
-			authorRepository.isAuthor.mockResolvedValue(true);
-			reviewerRepository.isReviewer.mockResolvedValue(true);
+	// The only case where the two answers differ: authors keep talking about a
+	// review they may not decide.
+	it('lets an author who cannot decide comment anyway', async () => {
+		authorRepository.isAuthor.mockResolvedValue(true);
+		reviewerRepository.isReviewer.mockResolvedValue(false);
 
-			const eligibility = await service.resolveViewerEligibility(memberUser(), readable());
+		const eligibility = await service.resolveViewerEligibility(memberUser(), readable());
 
-			expect(eligibility).toEqual({
-				canDecide: true,
-				decisionIneligibilityReason: null,
-				canComment: true,
-			});
+		expect(eligibility).toEqual({
+			canDecide: false,
+			decisionIneligibilityReason: 'author',
+			canComment: true,
 		});
+	});
 
-		it('stops a non-assigned author from approving their own review', async () => {
-			authorRepository.isAuthor.mockResolvedValue(true);
-			reviewerRepository.isReviewer.mockResolvedValue(false);
-
-			const eligibility = await service.resolveViewerEligibility(memberUser(), readable());
-
-			expect(eligibility).toEqual({
-				canDecide: false,
-				decisionIneligibilityReason: 'author',
-				canComment: true,
-			});
-		});
-
-		it.each([['global:admin'], ['global:owner']])(
-			'lets an instance %s decide on a review they authored',
-			async (slug) => {
-				authorRepository.isAuthor.mockResolvedValue(true);
-				reviewerRepository.isReviewer.mockResolvedValue(false);
-				const admin = mock<User>({ id: 'user-1', role: { slug } });
-
-				const eligibility = await service.resolveViewerEligibility(admin, readable());
-
-				expect(eligibility).toEqual({
-					canDecide: true,
-					decisionIneligibilityReason: null,
-					canComment: true,
-				});
-				expect(projectRelationRepository.getAccessibleProjectsByRoles).not.toHaveBeenCalled();
-			},
+	// The capability answers "who", not "when": a closed review still reports the
+	// viewer's own eligibility honestly, and callers gate on state separately.
+	it.each([
+		['a closed review', { state: 'closed' as const }],
+		['an approved review', { decision: 'approved' as const }],
+	])('still says who may act on %s', async (_label, overrides) => {
+		const eligibility = await service.resolveViewerEligibility(
+			memberUser(),
+			readable({
+				request: mock<WorkflowReviewRequest>({ id: requestId, projectId, ...overrides }),
+			}),
 		);
 
-		it('lets an author who is a project admin of the review project decide', async () => {
-			authorRepository.isAuthor.mockResolvedValue(true);
-			reviewerRepository.isReviewer.mockResolvedValue(false);
-			projectRelationRepository.getAccessibleProjectsByRoles.mockResolvedValue([projectId]);
-
-			const eligibility = await service.resolveViewerEligibility(memberUser(), readable());
-
-			expect(eligibility).toEqual({
-				canDecide: true,
-				decisionIneligibilityReason: null,
-				canComment: true,
-			});
+		expect(eligibility).toEqual({
+			canDecide: true,
+			decisionIneligibilityReason: null,
+			canComment: true,
 		});
+	});
 
-		it('still stops an author whose project-admin rights are in another project', async () => {
-			authorRepository.isAuthor.mockResolvedValue(true);
-			reviewerRepository.isReviewer.mockResolvedValue(false);
-			projectRelationRepository.getAccessibleProjectsByRoles.mockResolvedValue(['other-proj']);
-
-			const eligibility = await service.resolveViewerEligibility(memberUser(), readable());
-
-			expect(eligibility).toEqual({
-				canDecide: false,
-				decisionIneligibilityReason: 'author',
-				canComment: true,
-			});
-		});
-
-		it('still queries project roles for a non-admin', async () => {
-			await service.resolveViewerEligibility(memberUser(), readable());
-
-			// Only global admin/owner short-circuit; everyone else hits the project-role lookup.
-			expect(projectRelationRepository.getAccessibleProjectsByRoles).toHaveBeenCalledOnce();
-		});
-
-		it('tells an author without view access about the permission, not about their authorship', async () => {
-			// An author who cannot view the workflow would hit the endpoint's 404 first,
-			// so the surfaced reason must be the permission one, not 'author'.
-			authorRepository.isAuthor.mockResolvedValue(true);
-
+	describe('what counts as reading every covered workflow', () => {
+		it('refuses everything, without a participation lookup, when none is readable', async () => {
 			const eligibility = await service.resolveViewerEligibility(
 				memberUser(),
 				readable({ readableWorkflowRows: [] }),
@@ -179,30 +138,11 @@ describe('WorkflowReviewAuthorizationService: viewer capabilities', () => {
 				decisionIneligibilityReason: 'missing_permission',
 				canComment: false,
 			});
+			expect(reviewerRepository.isReviewer).not.toHaveBeenCalled();
 			expect(authorRepository.isAuthor).not.toHaveBeenCalled();
 		});
 
-		// The capability answers "who", not "when": a closed review still reports the
-		// viewer's own eligibility honestly, and callers gate on state separately.
-		it.each([
-			['a closed review', { state: 'closed' as const }],
-			['an approved review', { decision: 'approved' as const }],
-		])('still says who may act on %s', async (_label, overrides) => {
-			const eligibility = await service.resolveViewerEligibility(
-				memberUser(),
-				readable({
-					request: mock<WorkflowReviewRequest>({ id: requestId, projectId, ...overrides }),
-				}),
-			);
-
-			expect(eligibility).toEqual({
-				canDecide: true,
-				decisionIneligibilityReason: null,
-				canComment: true,
-			});
-		});
-
-		it('requires read access to every covered workflow, not just one of them', async () => {
+		it('requires every covered workflow, not just one of them', async () => {
 			const rows = [row('wf-1'), row('wf-2')];
 
 			const eligibility = await service.resolveViewerEligibility(
@@ -216,35 +156,10 @@ describe('WorkflowReviewAuthorizationService: viewer capabilities', () => {
 				canComment: false,
 			});
 		});
-	});
 
-	describe('who may comment', () => {
-		it('refuses commenting to a viewer who cannot open the workflow under review', async () => {
-			const eligibility = await service.resolveViewerEligibility(
-				memberUser(),
-				readable({ readableWorkflowRows: [] }),
-			);
-
-			expect(eligibility).toEqual({
-				canDecide: false,
-				decisionIneligibilityReason: 'missing_permission',
-				canComment: false,
-			});
-		});
-
-		it('refuses commenting to a non-assigned viewer even when they can open the workflow', async () => {
-			reviewerRepository.isReviewer.mockResolvedValue(false);
-
-			const eligibility = await service.resolveViewerEligibility(memberUser(), readable());
-
-			expect(eligibility).toEqual({
-				canDecide: false,
-				decisionIneligibilityReason: 'missing_reviewer_permission',
-				canComment: false,
-			});
-		});
-
-		it('refuses commenting to an author who can no longer open the workflow under review', async () => {
+		// An author who cannot view the workflow would hit the endpoint's 404 first,
+		// so the surfaced reason must be the permission one, not 'author'.
+		it('tells an author about the permission rather than their authorship', async () => {
 			authorRepository.isAuthor.mockResolvedValue(true);
 
 			const eligibility = await service.resolveViewerEligibility(
@@ -258,23 +173,52 @@ describe('WorkflowReviewAuthorizationService: viewer capabilities', () => {
 				canComment: false,
 			});
 		});
+	});
 
-		it('refuses both deciding and commenting on a review whose workflow is gone', async () => {
-			const eligibility = await service.resolveViewerEligibility(
-				memberUser(),
-				readable({ readableWorkflowRows: [] }),
+	describe('who counts as an admin of the review', () => {
+		beforeEach(() => {
+			// An admin override only matters for someone who could not decide otherwise.
+			authorRepository.isAuthor.mockResolvedValue(true);
+			reviewerRepository.isReviewer.mockResolvedValue(false);
+		});
+
+		it.each([['global:admin'], ['global:owner']])(
+			'lets an instance %s decide a review they authored, without a project lookup',
+			async (slug) => {
+				const admin = mock<User>({ id: 'user-1', role: { slug } });
+
+				const eligibility = await service.resolveViewerEligibility(admin, readable());
+
+				expect(eligibility.canDecide).toBe(true);
+				expect(projectRelationRepository.getAccessibleProjectsByRoles).not.toHaveBeenCalled();
+			},
+		);
+
+		it('lets a project admin of the review project decide', async () => {
+			projectRelationRepository.getAccessibleProjectsByRoles.mockResolvedValue([projectId]);
+
+			const eligibility = await service.resolveViewerEligibility(memberUser(), readable());
+
+			expect(eligibility.canDecide).toBe(true);
+			expect(projectRelationRepository.getAccessibleProjectsByRoles).toHaveBeenCalledWith(
+				'user-1',
+				['project:admin'],
 			);
+		});
 
-			expect(eligibility).toEqual({
+		it('does not let a project admin of some other project decide', async () => {
+			projectRelationRepository.getAccessibleProjectsByRoles.mockResolvedValue(['other-proj']);
+
+			const eligibility = await service.resolveViewerEligibility(memberUser(), readable());
+
+			expect(eligibility).toMatchObject({
 				canDecide: false,
-				decisionIneligibilityReason: 'missing_permission',
-				canComment: false,
+				decisionIneligibilityReason: 'author',
 			});
-			expect(reviewerRepository.isReviewer).not.toHaveBeenCalled();
 		});
 	});
 
-	describe('the admin rule', () => {
+	describe('isAdminForProject', () => {
 		it.each([['global:admin'], ['global:owner']])(
 			'treats a %s as admin of every project',
 			async (slug) => {

packages/cli/src/modules/workflow-reviews.ee/__tests__/workflow-review-create.controller.integration.test.ts

@@ -0,0 +1,414 @@
+import {
+	createTeamProject,
+	createWorkflow,
+	linkUserToProject,
+	mockInstance,
+	testDb,
+} from '@n8n/backend-test-utils';
+import type { Project, User } from '@n8n/db';
+import {
+	WorkflowHistoryRepository,
+	WorkflowReviewRequestAuthorRepository,
+	WorkflowReviewRequestRepository,
+	WorkflowReviewRequestReviewerRepository,
+	WorkflowReviewRequestWorkflowRepository,
+} from '@n8n/db';
+import { Container } from '@n8n/di';
+
+import { ActiveWorkflowManager } from '@/active-workflow-manager';
+import { WorkflowReviewPolicyService } from '@/services/workflow-review-policy.service';
+import { WorkflowValidationService } from '@/workflows/workflow-validation.service';
+import { createAdmin } from '@test-integration/db/users';
+import { createWorkflowHistoryItem } from '@test-integration/db/workflow-history';
+import type { SuperAgentTest } from '@test-integration/types';
+import * as utils from '@test-integration/utils';
+
+import {
+	createReviewableWorkflow,
+	findVersionName,
+	REVIEW_TABLES,
+	reviewPayload,
+	seedReview,
+	seedReviewActors,
+	stubWorkflowValidation,
+} from './support/workflow-review-test-data';
+
+mockInstance(ActiveWorkflowManager);
+const workflowValidationService = mockInstance(WorkflowValidationService);
+
+const testServer = utils.setupTestServer({
+	endpointGroups: ['workflow-reviews', 'workflows'],
+	enabledFeatures: ['feat:workflowReviews'],
+	modules: ['workflow-reviews'],
+});
+
+let owner: User;
+let member: User;
+let ownerProject: Project;
+let ownerAgent: SuperAgentTest;
+let memberAgent: SuperAgentTest;
+/** A global admin, so it is publish-capable on every project under test. */
+let reviewer: User;
+
+let requestRepository: WorkflowReviewRequestRepository;
+let workflowRepository: WorkflowReviewRequestWorkflowRepository;
+let authorRepository: WorkflowReviewRequestAuthorRepository;
+let reviewerRepository: WorkflowReviewRequestReviewerRepository;
+let workflowHistoryRepository: WorkflowHistoryRepository;
+let policyService: WorkflowReviewPolicyService;
+
+beforeAll(async () => {
+	await utils.initNodeTypes();
+	requestRepository = Container.get(WorkflowReviewRequestRepository);
+	workflowRepository = Container.get(WorkflowReviewRequestWorkflowRepository);
+	authorRepository = Container.get(WorkflowReviewRequestAuthorRepository);
+	reviewerRepository = Container.get(WorkflowReviewRequestReviewerRepository);
+	workflowHistoryRepository = Container.get(WorkflowHistoryRepository);
+	policyService = Container.get(WorkflowReviewPolicyService);
+});
+
+beforeEach(async () => {
+	testServer.license.enable('feat:workflowReviews');
+	await testDb.truncate([...REVIEW_TABLES]);
+	await policyService.set(true);
+	stubWorkflowValidation(workflowValidationService);
+
+	({ owner, member, ownerProject, ownerAgent, memberAgent } = await seedReviewActors(
+		testServer.authAgentFor,
+	));
+	reviewer = await createAdmin();
+});
+
+/**
+ * A reviewer is mandatory, so default one in. Tests asserting on reviewers pass
+ * their own `reviewerUserIds`, which overrides the default.
+ */
+const postReview = (agent: SuperAgentTest, body: object) =>
+	agent.post('/workflow-review-requests').send({ reviewerUserIds: [reviewer.id], ...body });
+
+describe('POST /workflow-review-requests', () => {
+	test('opens a review with its workflow reference and author', async () => {
+		const { workflow, versionId } = await createReviewableWorkflow(owner);
+
+		const response = await postReview(
+			ownerAgent,
+			reviewPayload({
+				workflowId: workflow.id,
+				versionId,
+				title: 'Please review my workflow',
+				description: 'It is ready',
+			}),
+		).expect(201);
+
+		expect(response.body.data.state).toBe('open');
+		expect(response.body.data.decision).toBe('pending');
+
+		const requests = await requestRepository.find();
+		expect(requests).toHaveLength(1);
+		expect(requests[0]).toMatchObject({
+			state: 'open',
+			decision: 'pending',
+			title: 'Please review my workflow',
+			description: 'It is ready',
+			projectId: ownerProject.id,
+			createdById: owner.id,
+		});
+
+		const childRows = await workflowRepository.find();
+		expect(childRows).toHaveLength(1);
+		expect(childRows[0]).toMatchObject({
+			workflowReviewRequestId: requests[0].id,
+			workflowId: workflow.id,
+			workflowVersionId: versionId,
+		});
+
+		const authorRows = await authorRepository.find();
+		expect(authorRows).toHaveLength(1);
+		expect(authorRows[0]).toMatchObject({
+			workflowReviewRequestId: requests[0].id,
+			userId: owner.id,
+		});
+	});
+
+	test('persists deduplicated reviewer rows together with the request', async () => {
+		const { workflow, versionId } = await createReviewableWorkflow(owner);
+		const secondReviewer = await createAdmin();
+
+		await postReview(ownerAgent, {
+			...reviewPayload({ workflowId: workflow.id, versionId, title: 'With a reviewer' }),
+			reviewerUserIds: [secondReviewer.id, secondReviewer.id],
+		}).expect(201);
+
+		const reviewerRows = await reviewerRepository.find();
+		expect(reviewerRows).toHaveLength(1);
+		expect(reviewerRows[0]).toMatchObject({ userId: secondReviewer.id });
+	});
+
+	test('refuses a requester who assigns themselves as reviewer', async () => {
+		const { workflow, versionId } = await createReviewableWorkflow(owner);
+
+		await postReview(ownerAgent, {
+			...reviewPayload({ workflowId: workflow.id, versionId }),
+			reviewerUserIds: [owner.id],
+		}).expect(400);
+
+		expect(await requestRepository.find()).toHaveLength(0);
+	});
+
+	test('refuses a reviewer who cannot read the workflow, and writes nothing', async () => {
+		const { workflow, versionId } = await createReviewableWorkflow(owner);
+
+		// A plain member has no publish rights on the owner's personal project
+		await postReview(ownerAgent, {
+			...reviewPayload({ workflowId: workflow.id, versionId }),
+			reviewerUserIds: [member.id],
+		}).expect(400);
+
+		expect(await requestRepository.find()).toHaveLength(0);
+		expect(await reviewerRepository.find()).toHaveLength(0);
+	});
+
+	test('refuses a version that belongs to another workflow', async () => {
+		const { workflow } = await createReviewableWorkflow(owner, { versionId: 'version-a' });
+		const other = await createWorkflow({}, owner);
+		await createWorkflowHistoryItem(other.id, { versionId: 'version-b' });
+
+		await postReview(
+			ownerAgent,
+			reviewPayload({ workflowId: workflow.id, versionId: 'version-b' }),
+		).expect(400);
+	});
+
+	test('trims the review description on create', async () => {
+		const { workflow, versionId } = await createReviewableWorkflow(owner);
+
+		await postReview(
+			ownerAgent,
+			reviewPayload({ workflowId: workflow.id, versionId, description: '  It is ready  ' }),
+		).expect(201);
+
+		const requests = await requestRepository.find();
+		expect(requests[0].description).toBe('It is ready');
+	});
+
+	test.each([
+		{ name: 'an empty', description: '' },
+		{ name: 'a whitespace-only', description: '   ' },
+	])('stores $name review description as null on create', async ({ description }) => {
+		const { workflow, versionId } = await createReviewableWorkflow(owner);
+
+		await postReview(
+			ownerAgent,
+			reviewPayload({ workflowId: workflow.id, versionId, description }),
+		).expect(201);
+
+		const requests = await requestRepository.find();
+		expect(requests[0].description).toBeNull();
+	});
+
+	test('refuses an archived workflow', async () => {
+		const { workflow, versionId } = await createReviewableWorkflow(owner, { isArchived: true });
+
+		await postReview(ownerAgent, reviewPayload({ workflowId: workflow.id, versionId })).expect(400);
+	});
+
+	test('hides a workflow that does not exist', async () => {
+		await postReview(
+			ownerAgent,
+			reviewPayload({ workflowId: 'unknown-workflow', versionId: 'version-1' }),
+		).expect(404);
+	});
+
+	test('hides a workflow the caller cannot access', async () => {
+		const { workflow, versionId } = await createReviewableWorkflow(owner);
+
+		await postReview(memberAgent, reviewPayload({ workflowId: workflow.id, versionId })).expect(
+			404,
+		);
+	});
+
+	test('hides a workflow the caller can only view', async () => {
+		const project = await createTeamProject('team', owner);
+		await linkUserToProject(member, project, 'project:viewer');
+		const { workflow, versionId } = await createReviewableWorkflow(project);
+
+		await postReview(memberAgent, reviewPayload({ workflowId: workflow.id, versionId })).expect(
+			404,
+		);
+	});
+
+	test('lets anyone who can publish the workflow ask for a review', async () => {
+		const project = await createTeamProject('team', owner);
+		await linkUserToProject(member, project, 'project:editor');
+		const { workflow, versionId } = await createReviewableWorkflow(project);
+
+		await postReview(memberAgent, reviewPayload({ workflowId: workflow.id, versionId })).expect(
+			201,
+		);
+	});
+
+	test('refuses a second review and points at the one already open', async () => {
+		const { workflow } = await createReviewableWorkflow(owner, { versionId: 'version-1' });
+		await createWorkflowHistoryItem(workflow.id, { versionId: 'version-2' });
+
+		const existing = await seedReview({
+			projectId: ownerProject.id,
+			workflowId: workflow.id,
+			versionId: 'version-1',
+			author: owner,
+			title: 'Existing',
+		});
+
+		const response = await postReview(
+			ownerAgent,
+			reviewPayload({ workflowId: workflow.id, versionId: 'version-2', title: 'New' }),
+		).expect(409);
+
+		expect(response.body.meta.workflowReviewRequestId).toBe(existing.id);
+		expect(JSON.stringify(response.body)).not.toMatch(/sync/i);
+		// No new rows written.
+		expect(await requestRepository.find()).toHaveLength(1);
+		expect(await workflowRepository.find()).toHaveLength(1);
+	});
+
+	test('refuses a second review while the approved one is still open', async () => {
+		const { workflow, versionId } = await createReviewableWorkflow(owner);
+
+		const existing = await seedReview({
+			projectId: ownerProject.id,
+			workflowId: workflow.id,
+			versionId,
+			author: owner,
+			state: 'open',
+			decision: 'approved',
+			title: 'Existing',
+		});
+
+		const response = await postReview(
+			ownerAgent,
+			reviewPayload({ workflowId: workflow.id, versionId, title: 'New' }),
+		).expect(409);
+
+		expect(response.body.meta.workflowReviewRequestId).toBe(existing.id);
+	});
+
+	test('opens a new review once the previous one is closed', async () => {
+		const { workflow, versionId } = await createReviewableWorkflow(owner);
+
+		await seedReview({
+			projectId: ownerProject.id,
+			workflowId: workflow.id,
+			versionId,
+			author: owner,
+			state: 'closed',
+			title: 'Closed',
+		});
+
+		await postReview(
+			ownerAgent,
+			reviewPayload({ workflowId: workflow.id, versionId, title: 'New' }),
+		).expect(201);
+
+		const openRequests = await requestRepository.find({ where: { state: 'open' } });
+		expect(openRequests).toHaveLength(1);
+	});
+
+	test('lets only one of two simultaneous submissions win', async () => {
+		const { workflow, versionId } = await createReviewableWorkflow(owner);
+		const body = reviewPayload({ workflowId: workflow.id, versionId, title: 'Race' });
+
+		const [first, second] = await Promise.all([
+			postReview(ownerAgent, body),
+			postReview(ownerAgent, body),
+		]);
+
+		expect([first.status, second.status].sort()).toEqual([201, 409]);
+
+		const openRequests = await requestRepository.find({ where: { state: 'open' } });
+		expect(openRequests).toHaveLength(1);
+		expect(await workflowRepository.find()).toHaveLength(1);
+	});
+
+	test('stops accepting reviews the moment an admin turns them off', async () => {
+		const { workflow, versionId } = await createReviewableWorkflow(owner);
+
+		// Enabled (from beforeEach) → allowed.
+		await postReview(
+			ownerAgent,
+			reviewPayload({ workflowId: workflow.id, versionId, title: 'First' }),
+		).expect(201);
+
+		// Disabled → rejected, even for an otherwise valid request.
+		await policyService.set(false);
+		const other = await createReviewableWorkflow(owner, { versionId: 'v-other' });
+
+		await postReview(
+			ownerAgent,
+			reviewPayload({
+				workflowId: other.workflow.id,
+				versionId: other.versionId,
+				title: 'Second',
+			}),
+		).expect(403);
+	});
+
+	test('refuses everything on an instance without a workflow reviews licence', async () => {
+		testServer.license.disable('feat:workflowReviews');
+
+		await postReview(
+			ownerAgent,
+			reviewPayload({ workflowId: 'wf-1', versionId: 'version-1' }),
+		).expect(403);
+
+		testServer.license.enable('feat:workflowReviews');
+	});
+
+	describe('pinned version naming', () => {
+		test('names the pinned version', async () => {
+			const { workflow, versionId } = await createReviewableWorkflow(owner);
+
+			await postReview(ownerAgent, reviewPayload({ workflowId: workflow.id, versionId })).expect(
+				201,
+			);
+
+			expect(await findVersionName(workflow.id, versionId)).toBe('Release candidate');
+		});
+
+		test('persists the version description alongside the name', async () => {
+			const { workflow, versionId } = await createReviewableWorkflow(owner);
+
+			await postReview(
+				ownerAgent,
+				reviewPayload({
+					workflowId: workflow.id,
+					versionId,
+					versionDescription: '  What changed in this version  ',
+				}),
+			).expect(201);
+
+			const version = await workflowHistoryRepository.findOneBy({
+				workflowId: workflow.id,
+				versionId,
+			});
+			expect(version?.name).toBe('Release candidate');
+			expect(version?.description).toBe('What changed in this version');
+		});
+
+		test('rolls the name back when the create conflicts with an open review', async () => {
+			const { workflow, versionId } = await createReviewableWorkflow(owner);
+			await seedReview({
+				projectId: ownerProject.id,
+				workflowId: workflow.id,
+				versionId,
+				author: owner,
+			});
+
+			await postReview(
+				ownerAgent,
+				reviewPayload({ workflowId: workflow.id, versionId, title: 'Second review' }),
+			).expect(409);
+
+			expect(await findVersionName(workflow.id, versionId)).toBeNull();
+		});
+	});
+});

packages/cli/src/modules/workflow-reviews.ee/__tests__/workflow-review-decision-policy.test.ts

@@ -0,0 +1,62 @@
+import {
+	resolveDecisionCapability,
+	type WorkflowReviewDecisionFacts,
+} from '../workflow-review-decision-policy';
+
+/**
+ * The one rule behind two presentations: `decide()` turns a refusal into a 403 or a
+ * hiding 404, and the detail read turns it into `viewerCanDecide` plus a reason. The
+ * table below is the whole rule, so those two suites only have to cover how they
+ * present the answer.
+ */
+describe('who may decide a review', () => {
+	const facts = (overrides: Partial<WorkflowReviewDecisionFacts> = {}) => ({
+		canReadEveryWorkflow: true,
+		isAuthor: false,
+		isAssignedReviewer: false,
+		hasAdminOverride: false,
+		...overrides,
+	});
+
+	it.each([
+		['an assigned reviewer', { isAssignedReviewer: true }],
+		['an admin', { hasAdminOverride: true }],
+		[
+			'an assigned reviewer who also authored a version',
+			{
+				isAssignedReviewer: true,
+				isAuthor: true,
+			},
+		],
+		['an admin who authored the review', { hasAdminOverride: true, isAuthor: true }],
+	])('lets %s decide', (_who, overrides) => {
+		expect(resolveDecisionCapability(facts(overrides))).toEqual({ allowed: true });
+	});
+
+	it('stops an author who is neither an assigned reviewer nor an admin, and says why', () => {
+		expect(resolveDecisionCapability(facts({ isAuthor: true }))).toEqual({
+			allowed: false,
+			reason: 'author',
+		});
+	});
+
+	it('stops an uninvolved reader, and says they are not a reviewer', () => {
+		expect(resolveDecisionCapability(facts())).toEqual({
+			allowed: false,
+			reason: 'missing_reviewer_permission',
+		});
+	});
+
+	// Read access is the floor, checked first so someone who cannot see every
+	// covered workflow hears about the permission rather than about their authorship.
+	it.each([
+		['an author', { isAuthor: true }],
+		['an assigned reviewer', { isAssignedReviewer: true }],
+		['an admin', { hasAdminOverride: true }],
+		['an uninvolved reader', {}],
+	])('tells %s who cannot read every covered workflow about the permission', (_who, overrides) => {
+		expect(resolveDecisionCapability(facts({ ...overrides, canReadEveryWorkflow: false }))).toEqual(
+			{ allowed: false, reason: 'missing_permission' },
+		);
+	});
+});

packages/cli/src/modules/workflow-reviews.ee/__tests__/workflow-review-decision.controller.integration.test.ts

@@ -0,0 +1,390 @@
+import { linkUserToProject, mockInstance, testDb } from '@n8n/backend-test-utils';
+import type { Project, User } from '@n8n/db';
+import {
+	WorkflowPublishHistoryRepository,
+	WorkflowRepository,
+	WorkflowReviewRequestRepository,
+} from '@n8n/db';
+import { Container } from '@n8n/di';
+
+import { ActiveWorkflowManager } from '@/active-workflow-manager';
+import { WorkflowReviewPolicyService } from '@/services/workflow-review-policy.service';
+import { WorkflowValidationService } from '@/workflows/workflow-validation.service';
+import { createAdmin, createUser } from '@test-integration/db/users';
+import { createWorkflowHistoryItem } from '@test-integration/db/workflow-history';
+import type { SuperAgentTest } from '@test-integration/types';
+import * as utils from '@test-integration/utils';
+
+import {
+	createReviewableWorkflow,
+	REVIEW_TABLES,
+	seedReview,
+	seedReviewActors,
+	stubWorkflowValidation,
+	versionUpdatePayload,
+} from './support/workflow-review-test-data';
+
+mockInstance(ActiveWorkflowManager);
+const workflowValidationService = mockInstance(WorkflowValidationService);
+
+const testServer = utils.setupTestServer({
+	endpointGroups: ['workflow-reviews', 'workflows'],
+	enabledFeatures: ['feat:workflowReviews'],
+	modules: ['workflow-reviews'],
+});
+
+let owner: User;
+let member: User;
+let viewer: User;
+let teamProject: Project;
+let ownerAgent: SuperAgentTest;
+let memberAgent: SuperAgentTest;
+let viewerAgent: SuperAgentTest;
+
+let requestRepository: WorkflowReviewRequestRepository;
+let publishHistoryRepository: WorkflowPublishHistoryRepository;
+let workflowEntityRepository: WorkflowRepository;
+let policyService: WorkflowReviewPolicyService;
+
+beforeAll(async () => {
+	await utils.initNodeTypes();
+	requestRepository = Container.get(WorkflowReviewRequestRepository);
+	publishHistoryRepository = Container.get(WorkflowPublishHistoryRepository);
+	workflowEntityRepository = Container.get(WorkflowRepository);
+	policyService = Container.get(WorkflowReviewPolicyService);
+});
+
+beforeEach(async () => {
+	testServer.license.enable('feat:workflowReviews');
+	await testDb.truncate([...REVIEW_TABLES]);
+	await policyService.set(true);
+	stubWorkflowValidation(workflowValidationService);
+
+	({ owner, member, viewer, teamProject, ownerAgent, memberAgent, viewerAgent } =
+		await seedReviewActors(testServer.authAgentFor));
+});
+
+/** Seed a review on a team-project workflow, authored by `author`. */
+async function seedRequest(
+	author: User,
+	overrides: {
+		state?: 'open' | 'closed';
+		decision?: 'pending' | 'changes_requested' | 'approved';
+	} = {},
+	reviewerIds: string[] = [member.id],
+) {
+	const { workflow } = await createReviewableWorkflow(teamProject, { versionId: 'version-1' });
+	const request = await seedReview({
+		projectId: teamProject.id,
+		workflowId: workflow.id,
+		versionId: 'version-1',
+		author,
+		reviewerIds,
+		title: 'Review me',
+		...overrides,
+	});
+	return { request, workflow };
+}
+
+const decide = (agent: SuperAgentTest, requestId: string, body: object) =>
+	agent.post(`/workflow-review-requests/${requestId}/decision`).send(body);
+
+const approve = (agent: SuperAgentTest, requestId: string) =>
+	decide(agent, requestId, { decision: 'approved' });
+
+const requestChanges = (agent: SuperAgentTest, requestId: string) =>
+	decide(agent, requestId, { decision: 'changes_requested', note: 'Please rename the node' });
+
+const publishedVersionOf = async (workflowId: string) =>
+	(await workflowEntityRepository.findOneByOrFail({ id: workflowId })).activeVersionId;
+
+describe('POST /workflow-review-requests/:workflowReviewRequestId/decision', () => {
+	test('closes the review on approval, recording who approved it and when', async () => {
+		const { request } = await seedRequest(owner);
+		const seededUpdatedAt = request.updatedAt.getTime();
+
+		const response = await approve(memberAgent, request.id).expect(200);
+
+		expect(response.body.data).toEqual({
+			id: request.id,
+			state: 'closed',
+			decision: 'approved',
+			workflowVersionId: 'version-1',
+			createdAt: expect.any(String),
+			updatedAt: expect.any(String),
+			autoPublish: { status: 'published' },
+		});
+
+		// the service relies on `save` (not `update`) so @BeforeUpdate bumps
+		// updatedAt — assert the timestamp actually moves.
+		expect(new Date(response.body.data.updatedAt).getTime()).toBeGreaterThan(seededUpdatedAt);
+
+		const updated = await requestRepository.findById(request.id, {});
+		expect(updated).toMatchObject({
+			state: 'closed',
+			decision: 'approved',
+			updatedById: member.id,
+			closedById: member.id,
+		});
+		expect(updated?.approvedAt).toBeInstanceOf(Date);
+	});
+
+	test('publishes the approved version as the requester, not as the reviewer', async () => {
+		const { request, workflow } = await seedRequest(owner);
+
+		await approve(memberAgent, request.id).expect(200);
+
+		expect(await publishedVersionOf(workflow.id)).toBe('version-1');
+		// Publish history must record the requester, not the approving reviewer.
+		const records = await publishHistoryRepository.findBy({ workflowId: workflow.id });
+		expect(records).toEqual([
+			expect.objectContaining({ event: 'activated', versionId: 'version-1', userId: owner.id }),
+		]);
+	});
+
+	test('closes as the system and reports the failure when the requester is gone', async () => {
+		const { request, workflow } = await seedRequest(owner);
+		const current = await requestRepository.findById(request.id, {});
+		current!.createdById = null;
+		await requestRepository.saveRequest(current!, {});
+
+		const response = await approve(memberAgent, request.id).expect(200);
+
+		expect(response.body.data).toMatchObject({
+			state: 'closed',
+			decision: 'approved',
+			autoPublish: {
+				status: 'failed',
+				message: 'The review requester is no longer available',
+			},
+		});
+		expect(await requestRepository.findById(request.id, {})).toMatchObject({
+			state: 'closed',
+			decision: 'approved',
+			closedById: null,
+			updatedById: member.id,
+		});
+		expect(await publishedVersionOf(workflow.id)).toBeNull();
+	});
+
+	test('closes as the system and reports the failure when the requester can no longer publish', async () => {
+		const demotedRequester = await createUser();
+		await linkUserToProject(demotedRequester, teamProject, 'project:editor');
+		const { request, workflow } = await seedRequest(demotedRequester);
+		// Downgrade after the review was opened — they can no longer publish.
+		await linkUserToProject(demotedRequester, teamProject, 'project:viewer');
+
+		const response = await approve(memberAgent, request.id).expect(200);
+
+		expect(response.body.data).toMatchObject({
+			state: 'closed',
+			decision: 'approved',
+			autoPublish: {
+				status: 'failed',
+				message: 'The review requester no longer has permission to publish this workflow',
+			},
+		});
+		expect(await requestRepository.findById(request.id, {})).toMatchObject({
+			state: 'closed',
+			decision: 'approved',
+			closedById: null,
+		});
+		expect(await publishedVersionOf(workflow.id)).toBeNull();
+	});
+
+	test('leaves the review open, unstamped and unpublished when a reviewer asks for changes', async () => {
+		const { request, workflow } = await seedRequest(owner);
+
+		const response = await requestChanges(memberAgent, request.id).expect(200);
+
+		expect(response.body.data).toMatchObject({
+			state: 'open',
+			decision: 'changes_requested',
+		});
+		expect(response.body.data.autoPublish).toBeUndefined();
+		expect(await publishedVersionOf(workflow.id)).toBeNull();
+
+		const updated = await requestRepository.findById(request.id, {});
+		expect(updated).toMatchObject({
+			state: 'open',
+			decision: 'changes_requested',
+			updatedById: member.id,
+			closedById: null,
+			approvedAt: null,
+		});
+	});
+
+	test('lets a reviewer approve a review that had changes requested', async () => {
+		const { request } = await seedRequest(owner, { decision: 'changes_requested' });
+
+		await approve(memberAgent, request.id).expect(200);
+
+		expect(await requestRepository.findById(request.id, {})).toMatchObject({
+			state: 'closed',
+			decision: 'approved',
+		});
+	});
+
+	test('lets a second reviewer ask for changes again', async () => {
+		const { request } = await seedRequest(owner, { decision: 'changes_requested' });
+
+		await requestChanges(memberAgent, request.id).expect(200);
+
+		expect(await requestRepository.findById(request.id, {})).toMatchObject({
+			state: 'open',
+			decision: 'changes_requested',
+			updatedById: member.id,
+		});
+	});
+
+	test('refuses an author deciding their own review', async () => {
+		const { request } = await seedRequest(member, {}, [owner.id]);
+
+		await approve(memberAgent, request.id).expect(403);
+
+		expect(await requestRepository.findById(request.id, {})).toMatchObject({
+			state: 'open',
+			decision: 'pending',
+		});
+	});
+
+	test('lets a reviewer decide after they submit a new version themselves', async () => {
+		const { request, workflow } = await seedRequest(owner);
+		await createWorkflowHistoryItem(workflow.id, { versionId: 'version-2' });
+
+		await memberAgent
+			.post(`/workflow-review-requests/${request.id}/update-version`)
+			.send(versionUpdatePayload({ workflowId: workflow.id, versionId: 'version-2' }))
+			.expect(200);
+
+		await approve(memberAgent, request.id).expect(200);
+	});
+
+	test('lets the instance owner decide their own review', async () => {
+		const { request } = await seedRequest(owner);
+
+		await approve(ownerAgent, request.id).expect(200);
+	});
+
+	test('lets an instance admin decide their own review', async () => {
+		const admin = await createAdmin();
+		const { request } = await seedRequest(admin);
+
+		await approve(testServer.authAgentFor(admin), request.id).expect(200);
+	});
+
+	test('lets a project admin decide their own review in that project', async () => {
+		const projectAdmin = await createUser();
+		await linkUserToProject(projectAdmin, teamProject, 'project:admin');
+		const { request } = await seedRequest(projectAdmin);
+
+		await approve(testServer.authAgentFor(projectAdmin), request.id).expect(200);
+	});
+
+	test('hides a review from someone who was not asked to review it', async () => {
+		const { request } = await seedRequest(owner, {}, []);
+
+		const response = await approve(memberAgent, request.id).expect(404);
+
+		// Same wording as an unknown id, so the refusal doesn't reveal the review exists.
+		expect(response.body.message).toBe('Could not find review request');
+	});
+
+	test('lets someone who can only view the workflow decide when asked to review', async () => {
+		const { request } = await seedRequest(owner, {}, [viewer.id]);
+
+		await requestChanges(viewerAgent, request.id).expect(200);
+
+		expect(await requestRepository.findById(request.id, {})).toMatchObject({
+			state: 'open',
+			decision: 'changes_requested',
+			updatedById: viewer.id,
+		});
+	});
+
+	test('lets a reviewer who can only view the workflow approve, publishing as the requester', async () => {
+		// The decider holds workflow:read only, so publishing can only work
+		// because it runs as the requester.
+		const { request, workflow } = await seedRequest(owner, {}, [viewer.id]);
+
+		const response = await approve(viewerAgent, request.id).expect(200);
+
+		expect(response.body.data).toMatchObject({
+			state: 'closed',
+			decision: 'approved',
+			autoPublish: { status: 'published' },
+		});
+		expect(await requestRepository.findById(request.id, {})).toMatchObject({
+			state: 'closed',
+			decision: 'approved',
+			updatedById: viewer.id,
+			closedById: viewer.id,
+		});
+		expect(await publishedVersionOf(workflow.id)).toBe('version-1');
+		const records = await publishHistoryRepository.findBy({ workflowId: workflow.id });
+		expect(records).toEqual([
+			expect.objectContaining({ event: 'activated', versionId: 'version-1', userId: owner.id }),
+		]);
+	});
+
+	test('hides a review that does not exist', async () => {
+		const response = await approve(memberAgent, 'unknown-request').expect(404);
+
+		expect(response.body.message).toBe('Could not find review request');
+	});
+
+	test('refuses to re-pin a closed review', async () => {
+		const { request } = await seedRequest(owner, { state: 'closed' });
+
+		await approve(memberAgent, request.id).expect(409);
+	});
+
+	test('refuses a second decision on an approved review', async () => {
+		const { request } = await seedRequest(owner, { decision: 'approved' });
+
+		await decide(memberAgent, request.id, { decision: 'changes_requested' }).expect(409);
+	});
+
+	test('refuses everything once an admin turns reviews off', async () => {
+		const { request } = await seedRequest(owner);
+		await policyService.set(false);
+
+		await approve(memberAgent, request.id).expect(403);
+	});
+
+	test('lets only one of two simultaneous approvals win', async () => {
+		const { request } = await seedRequest(owner);
+
+		const [first, second] = await Promise.all([
+			approve(memberAgent, request.id),
+			approve(memberAgent, request.id),
+		]);
+
+		expect([first.status, second.status].sort()).toEqual([200, 409]);
+		expect(await requestRepository.findById(request.id, {})).toMatchObject({
+			state: 'closed',
+			decision: 'approved',
+		});
+	});
+
+	test('never leaves a closed review undecided when a re-pin races the decision', async () => {
+		const { request, workflow } = await seedRequest(owner);
+		await createWorkflowHistoryItem(workflow.id, { versionId: 'version-2' });
+
+		const [decision, sync] = await Promise.all([
+			approve(memberAgent, request.id),
+			ownerAgent
+				.post(`/workflow-review-requests/${request.id}/update-version`)
+				.send(versionUpdatePayload({ workflowId: workflow.id, versionId: 'version-2' })),
+		]);
+
+		// Whichever wins the lock, the loser must observe the winner's write:
+		// either the sync lands first (both 200) or it conflicts on the closed request.
+		expect(decision.status).toBe(200);
+		expect([200, 409]).toContain(sync.status);
+
+		const final = await requestRepository.findById(request.id, {});
+		expect(final?.state === 'closed' && final?.decision === 'pending').toBe(false);
+		expect(final).toMatchObject({ state: 'closed', decision: 'approved' });
+	});
+});

packages/cli/src/modules/workflow-reviews.ee/__tests__/workflow-review-detail.controller.integration.test.ts

@@ -0,0 +1,501 @@
+import {
+	createTeamProject,
+	createWorkflow,
+	linkUserToProject,
+	mockInstance,
+	testDb,
+} from '@n8n/backend-test-utils';
+import type { Project, User } from '@n8n/db';
+import {
+	WorkflowRepository,
+	WorkflowReviewRequestReviewerRepository,
+	WorkflowReviewRequestWorkflowRepository,
+} from '@n8n/db';
+import { Container } from '@n8n/di';
+
+import { ActiveWorkflowManager } from '@/active-workflow-manager';
+import { WorkflowReviewPolicyService } from '@/services/workflow-review-policy.service';
+import { WorkflowValidationService } from '@/workflows/workflow-validation.service';
+import { createAdmin, createMember } from '@test-integration/db/users';
+import { createWorkflowHistoryItem } from '@test-integration/db/workflow-history';
+import type { SuperAgentTest } from '@test-integration/types';
+import * as utils from '@test-integration/utils';
+
+import {
+	REVIEW_TABLES,
+	seedReview,
+	seedReviewActors,
+	stubWorkflowValidation,
+} from './support/workflow-review-test-data';
+
+mockInstance(ActiveWorkflowManager);
+const workflowValidationService = mockInstance(WorkflowValidationService);
+
+const testServer = utils.setupTestServer({
+	endpointGroups: ['workflow-reviews', 'workflows'],
+	enabledFeatures: ['feat:workflowReviews'],
+	modules: ['workflow-reviews'],
+});
+
+let owner: User;
+let member: User;
+let viewer: User;
+let teamProject: Project;
+let ownerAgent: SuperAgentTest;
+let memberAgent: SuperAgentTest;
+let viewerAgent: SuperAgentTest;
+
+let workflowRepository: WorkflowReviewRequestWorkflowRepository;
+let reviewerRepository: WorkflowReviewRequestReviewerRepository;
+let workflowEntityRepository: WorkflowRepository;
+let policyService: WorkflowReviewPolicyService;
+
+beforeAll(async () => {
+	await utils.initNodeTypes();
+	workflowRepository = Container.get(WorkflowReviewRequestWorkflowRepository);
+	reviewerRepository = Container.get(WorkflowReviewRequestReviewerRepository);
+	workflowEntityRepository = Container.get(WorkflowRepository);
+	policyService = Container.get(WorkflowReviewPolicyService);
+});
+
+beforeEach(async () => {
+	testServer.license.enable('feat:workflowReviews');
+	await testDb.truncate([...REVIEW_TABLES]);
+	await policyService.set(true);
+	stubWorkflowValidation(workflowValidationService);
+
+	({ owner, member, viewer, teamProject, ownerAgent, memberAgent, viewerAgent } =
+		await seedReviewActors(testServer.authAgentFor));
+});
+
+/** Seed a review in `projectId` pinned to `versionId`, authored by `author`. */
+async function seedRequest(
+	workflowId: string,
+	versionId: string | null,
+	author: User,
+	projectId = teamProject.id,
+) {
+	return await seedReview({
+		projectId,
+		workflowId,
+		versionId,
+		author,
+		title: 'Please review',
+		description: 'Some context',
+	});
+}
+
+/**
+ * Seed a review in `teamProject` pinned to a workflow that has moved out of
+ * `author`'s reach, covering a second workflow that is still readable. Row ids
+ * are set explicitly because generated nanoids would leave it to chance which
+ * row the query's id ordering puts first, i.e. which one counts as pinned.
+ */
+async function seedTwoWorkflowRequest(author: User) {
+	const destinationProject = await createTeamProject('Moved Away', owner);
+	const movedWorkflow = await createWorkflow({}, destinationProject);
+	await createWorkflowHistoryItem(movedWorkflow.id, { versionId: 'version-pinned' });
+	const readableWorkflow = await createWorkflow({ name: 'Still readable' }, teamProject);
+	const request = await seedReview({
+		projectId: teamProject.id,
+		author,
+		title: 'Please review',
+	});
+	await workflowRepository.createWorkflowRow(
+		{
+			id: '1-pinned-row',
+			workflowReviewRequestId: request.id,
+			workflowId: movedWorkflow.id,
+			workflowVersionId: 'version-pinned',
+		},
+		{},
+	);
+	await workflowRepository.createWorkflowRow(
+		{ id: '2-extra-row', workflowReviewRequestId: request.id, workflowId: readableWorkflow.id },
+		{},
+	);
+	return { request, movedWorkflow, readableWorkflow };
+}
+
+const getDetail = (agent: SuperAgentTest, requestId: string) =>
+	agent.get(`/workflow-review-requests/${requestId}`);
+
+describe('GET /workflow-review-requests/:workflowReviewRequestId', () => {
+	test('returns the review, the workflows it covers, and both versions to compare', async () => {
+		const workflow = await createWorkflow({ name: 'Reviewed workflow' }, teamProject);
+		const baseline = await createWorkflowHistoryItem(workflow.id, {
+			versionId: 'version-published',
+		});
+		await createWorkflowHistoryItem(workflow.id, {
+			versionId: 'version-pinned',
+			name: 'Release candidate',
+		});
+		// The baseline resolves from the workflow row, which both publication paths maintain.
+		await workflowEntityRepository.update(workflow.id, {
+			active: true,
+			activeVersionId: baseline.versionId,
+		});
+		const reviewer = await createAdmin();
+		const request = await seedRequest(workflow.id, 'version-pinned', owner);
+		await reviewerRepository.addReviewers(
+			{ workflowReviewRequestId: request.id, userIds: [reviewer.id] },
+			{},
+		);
+
+		const response = await getDetail(ownerAgent, request.id).expect(200);
+
+		expect(response.body.data).toMatchObject({
+			id: request.id,
+			projectId: teamProject.id,
+			state: 'open',
+			decision: 'pending',
+			title: 'Please review',
+			description: 'Some context',
+			requester: { id: owner.id, email: owner.email },
+			reviewers: [{ id: reviewer.id, email: reviewer.email }],
+		});
+		// The covered workflows live only in `workflows` — the inbox card's flat
+		// summary fields are not part of the detail response.
+		expect(response.body.data).not.toHaveProperty('workflowName');
+		expect(response.body.data).not.toHaveProperty('workflowVersionId');
+
+		expect(response.body.data.workflows).toHaveLength(1);
+		const [child] = response.body.data.workflows;
+		expect(child).toMatchObject({
+			workflowId: workflow.id,
+			workflowName: 'Reviewed workflow',
+			workflowVersionId: 'version-pinned',
+		});
+		expect(child.pinnedVersion).toMatchObject({
+			versionId: 'version-pinned',
+			// The diff labels each side by name, so it travels with the snapshot
+			name: 'Release candidate',
+			connections: {},
+			nodeGroups: [],
+		});
+		expect(child.pinnedVersion.nodes).toHaveLength(1);
+		expect(child.pinnedVersion.nodes[0]).toMatchObject({ name: 'Start' });
+		expect(child.pinnedVersion).not.toHaveProperty('authors');
+		expect(child.baselineVersion).toMatchObject({
+			versionId: 'version-published',
+			name: null,
+		});
+	});
+
+	test('keeps the approval-time baseline after the published pointer moves', async () => {
+		const workflow = await createWorkflow({ name: 'Reviewed workflow' }, teamProject);
+		await createWorkflowHistoryItem(workflow.id, { versionId: 'version-published' });
+		await createWorkflowHistoryItem(workflow.id, {
+			versionId: 'version-pinned',
+			name: 'Release candidate',
+		});
+		await createWorkflowHistoryItem(workflow.id, { versionId: 'version-later' });
+		await workflowEntityRepository.update(workflow.id, {
+			active: true,
+			activeVersionId: 'version-published',
+		});
+		const request = await seedRequest(workflow.id, 'version-pinned', owner);
+		await reviewerRepository.addReviewers(
+			{ workflowReviewRequestId: request.id, userIds: [member.id] },
+			{},
+		);
+
+		await memberAgent
+			.post(`/workflow-review-requests/${request.id}/decision`)
+			.send({ decision: 'approved' })
+			.expect(200);
+
+		// Auto-publish moved the live pointer to the pinned version; advance it again
+		// so a live read would show the wrong baseline without persistence.
+		await workflowEntityRepository.update(workflow.id, { activeVersionId: 'version-later' });
+
+		const response = await getDetail(ownerAgent, request.id).expect(200);
+
+		expect(response.body.data.state).toBe('closed');
+		expect(response.body.data.workflows[0].baselineVersion).toMatchObject({
+			versionId: 'version-published',
+		});
+		expect(response.body.data.workflows[0].pinnedVersion).toMatchObject({
+			versionId: 'version-pinned',
+		});
+
+		const [child] = await workflowRepository.findByRequestId(request.id, {});
+		expect(child?.baselineVersionId).toBe('version-published');
+	});
+
+	test('keeps a null approval baseline null once auto-publish moves the pointer', async () => {
+		const workflow = await createWorkflow({ name: 'Reviewed workflow' }, teamProject);
+		await createWorkflowHistoryItem(workflow.id, { versionId: 'version-pinned' });
+		// Never published, so the approval freezes a null baseline.
+		const request = await seedRequest(workflow.id, 'version-pinned', owner);
+		await reviewerRepository.addReviewers(
+			{ workflowReviewRequestId: request.id, userIds: [member.id] },
+			{},
+		);
+
+		await memberAgent
+			.post(`/workflow-review-requests/${request.id}/decision`)
+			.send({ decision: 'approved' })
+			.expect(200);
+
+		// Auto-publish left the live pointer on the pinned version, so reading it
+		// live would diff that version against itself.
+		const response = await getDetail(ownerAgent, request.id).expect(200);
+
+		expect(response.body.data.state).toBe('closed');
+		expect(response.body.data.workflows[0].baselineVersion).toBeNull();
+	});
+
+	test('returns no baseline for a closed review that was never approved', async () => {
+		const workflow = await createWorkflow({ name: 'Reviewed workflow' }, teamProject);
+		await createWorkflowHistoryItem(workflow.id, { versionId: 'version-published' });
+		await createWorkflowHistoryItem(workflow.id, { versionId: 'version-pinned' });
+		await workflowEntityRepository.update(workflow.id, {
+			active: true,
+			activeVersionId: 'version-published',
+		});
+		const request = await seedReview({
+			projectId: teamProject.id,
+			workflowId: workflow.id,
+			versionId: 'version-pinned',
+			author: owner,
+			title: 'Please review',
+			state: 'closed',
+			decision: 'pending',
+		});
+
+		const response = await getDetail(ownerAgent, request.id).expect(200);
+
+		expect(response.body.data).toMatchObject({
+			state: 'closed',
+			decision: 'pending',
+		});
+		expect(response.body.data.workflows[0].baselineVersion).toBeNull();
+	});
+
+	test('has nothing to compare against when the workflow was never published', async () => {
+		const workflow = await createWorkflow({}, teamProject);
+		await createWorkflowHistoryItem(workflow.id, { versionId: 'version-pinned' });
+		const request = await seedRequest(workflow.id, 'version-pinned', owner);
+
+		const response = await getDetail(ownerAgent, request.id).expect(200);
+
+		expect(response.body.data.workflows[0].pinnedVersion).toMatchObject({
+			versionId: 'version-pinned',
+		});
+		expect(response.body.data.workflows[0].baselineVersion).toBeNull();
+	});
+
+	test('returns no version under review when the review does not point at one', async () => {
+		const workflow = await createWorkflow({}, teamProject);
+		const request = await seedRequest(workflow.id, null, owner);
+
+		const response = await getDetail(ownerAgent, request.id).expect(200);
+
+		expect(response.body.data.workflows[0]).toMatchObject({
+			workflowVersionId: null,
+			pinnedVersion: null,
+			baselineVersion: null,
+		});
+	});
+
+	test('still opens an open review after its workflow was hard-deleted', async () => {
+		const workflow = await createWorkflow({}, teamProject);
+		await createWorkflowHistoryItem(workflow.id, { versionId: 'version-pinned' });
+		const request = await seedRequest(workflow.id, 'version-pinned', owner);
+
+		// Bypasses the auto-close hook and the sweep: the cascade removes the link
+		// row and leaves the request open until the next delete sweeps it closed
+		await workflowEntityRepository.delete({ id: workflow.id });
+
+		const response = await getDetail(ownerAgent, request.id).expect(200);
+
+		expect(response.body.data.id).toBe(request.id);
+		expect(response.body.data.state).toBe('open');
+		expect(response.body.data.workflows).toEqual([]);
+	});
+
+	test('still opens a closed review after its workflow was deleted', async () => {
+		const workflow = await createWorkflow({}, teamProject);
+		await createWorkflowHistoryItem(workflow.id, { versionId: 'version-pinned' });
+		const request = await seedReview({
+			projectId: teamProject.id,
+			workflowId: workflow.id,
+			versionId: 'version-pinned',
+			author: owner,
+			title: 'Please review',
+			state: 'closed',
+			decision: 'approved',
+		});
+
+		// Deleting the workflow removes the review's reference, not its history
+		await workflowEntityRepository.delete({ id: workflow.id });
+
+		const response = await getDetail(ownerAgent, request.id).expect(200);
+
+		expect(response.body.data.id).toBe(request.id);
+		expect(response.body.data.workflows).toEqual([]);
+	});
+
+	test('lets an assigned reviewer in the review project open it', async () => {
+		const workflow = await createWorkflow({}, teamProject);
+		const request = await seedRequest(workflow.id, null, owner);
+		await reviewerRepository.addReviewers(
+			{ workflowReviewRequestId: request.id, userIds: [member.id] },
+			{},
+		);
+
+		const response = await getDetail(memberAgent, request.id).expect(200);
+
+		expect(response.body.data.id).toBe(request.id);
+	});
+
+	test('lets a project admin open a review in their project without involvement', async () => {
+		const projectAdmin = await createMember();
+		await linkUserToProject(projectAdmin, teamProject, 'project:admin');
+		const workflow = await createWorkflow({}, teamProject);
+		const request = await seedRequest(workflow.id, null, owner);
+
+		const response = await getDetail(testServer.authAgentFor(projectAdmin), request.id).expect(200);
+
+		expect(response.body.data.id).toBe(request.id);
+	});
+
+	test('hides the review from an uninvolved project member', async () => {
+		const workflow = await createWorkflow({}, teamProject);
+		const request = await seedRequest(workflow.id, null, owner);
+
+		await getDetail(viewerAgent, request.id).expect(404);
+	});
+
+	test('hides the review from someone outside its project', async () => {
+		const otherProject = await createTeamProject('Unrelated Project', owner);
+		const workflow = await createWorkflow({}, otherProject);
+		const request = await seedRequest(workflow.id, null, owner, otherProject.id);
+
+		await getDetail(memberAgent, request.id).expect(404);
+	});
+
+	test('hides the review once its workflow moves to a project the reviewer cannot see', async () => {
+		// The review still points at `teamProject`, where member is assigned as reviewer,
+		// while the workflow itself has moved to a project member has no access to
+		const destinationProject = await createTeamProject('Destination Project', owner);
+		const workflow = await createWorkflow({}, destinationProject);
+		await createWorkflowHistoryItem(workflow.id, { versionId: 'version-pinned' });
+		const request = await seedRequest(workflow.id, 'version-pinned', owner, teamProject.id);
+		await reviewerRepository.addReviewers(
+			{ workflowReviewRequestId: request.id, userIds: [member.id] },
+			{},
+		);
+
+		await getDetail(memberAgent, request.id).expect(404);
+	});
+
+	test('hides the review from its requester once they can read none of its workflows', async () => {
+		// Viewer asked for the review while the workflow was reachable; it has since
+		// moved to a project they have no access to. Seeing a review requires still
+		// holding read on what it reviews — requesters included.
+		const destinationProject = await createTeamProject('Moved Away', owner);
+		const workflow = await createWorkflow({}, destinationProject);
+		await createWorkflowHistoryItem(workflow.id, { versionId: 'version-pinned' });
+		const request = await seedRequest(workflow.id, 'version-pinned', viewer, teamProject.id);
+
+		await getDetail(viewerAgent, request.id).expect(404);
+	});
+
+	test('leaves out a workflow the requester can no longer see while another keeps the review open', async () => {
+		// The pinned workflow moved out of reach, but a second covered workflow is
+		// still readable — the review opens without the unreadable content.
+		const { request, readableWorkflow } = await seedTwoWorkflowRequest(viewer);
+
+		const response = await getDetail(viewerAgent, request.id).expect(200);
+
+		expect(response.body.data.id).toBe(request.id);
+		expect(response.body.data.workflows).toEqual([
+			expect.objectContaining({ workflowId: readableWorkflow.id }),
+		]);
+	});
+
+	test('shows requesters the review they asked for while they can read in its project', async () => {
+		const workflow = await createWorkflow({}, teamProject);
+		const request = await seedRequest(workflow.id, null, viewer);
+
+		const response = await getDetail(viewerAgent, request.id).expect(200);
+
+		expect(response.body.data.id).toBe(request.id);
+	});
+
+	describe('viewer decision eligibility', () => {
+		// Nobody is left who is neither admin, author, nor reviewer, so
+		// `missing_reviewer_permission` cannot happen over HTTP. The eligibility service
+		// unit tests cover that branch.
+
+		test('tells an assigned reviewer that they can decide', async () => {
+			const workflow = await createWorkflow({}, teamProject);
+			const request = await seedRequest(workflow.id, null, owner);
+			await reviewerRepository.addReviewers(
+				{ workflowReviewRequestId: request.id, userIds: [member.id] },
+				{},
+			);
+
+			const response = await getDetail(memberAgent, request.id).expect(200);
+
+			expect(response.body.data.viewerCanDecide).toBe(true);
+			expect(response.body.data.viewerDecisionIneligibilityReason).toBeNull();
+		});
+
+		test('tells a non-assigned author why they cannot decide their own review', async () => {
+			const workflow = await createWorkflow({}, teamProject);
+			const request = await seedRequest(workflow.id, null, member);
+
+			const response = await getDetail(memberAgent, request.id).expect(200);
+
+			expect(response.body.data.viewerCanDecide).toBe(false);
+			expect(response.body.data.viewerDecisionIneligibilityReason).toBe('author');
+		});
+
+		test('lets an instance admin decide a review they authored', async () => {
+			const workflow = await createWorkflow({}, teamProject);
+			const request = await seedRequest(workflow.id, null, owner);
+
+			const response = await getDetail(ownerAgent, request.id).expect(200);
+
+			expect(response.body.data.viewerCanDecide).toBe(true);
+			expect(response.body.data.viewerDecisionIneligibilityReason).toBeNull();
+		});
+
+		test('reports missing permission once the pinned workflow moved out of the requester reach', async () => {
+			// The pinned workflow moved away while a second one keeps the review
+			// readable: the requester keeps the record, but could no longer decide —
+			// and the reason says why.
+			const { request } = await seedTwoWorkflowRequest(viewer);
+
+			const response = await getDetail(viewerAgent, request.id).expect(200);
+
+			expect(response.body.data.viewerCanDecide).toBe(false);
+			expect(response.body.data.viewerDecisionIneligibilityReason).toBe('missing_permission');
+		});
+	});
+
+	test('reports a review that does not exist as not found', async () => {
+		await getDetail(ownerAgent, 'unknown-request').expect(404);
+	});
+
+	test('does not shadow the inbox, summary, and eligible-reviewers endpoints', async () => {
+		const workflow = await createWorkflow({}, teamProject);
+		await seedRequest(workflow.id, null, owner);
+
+		await ownerAgent.get('/workflow-review-requests/inbox').expect(200);
+		await ownerAgent.get('/workflow-review-requests/summary').expect(200);
+		// 400 (missing workflowId), not 404 — proves it still reaches its own handler
+		await ownerAgent.get('/workflow-review-requests/eligible-reviewers').expect(400);
+	});
+
+	test('refuses to open a review when an admin has turned reviews off', async () => {
+		const workflow = await createWorkflow({}, teamProject);
+		const request = await seedRequest(workflow.id, null, owner);
+		await policyService.set(false);
+
+		await getDetail(ownerAgent, request.id).expect(403);
+	});
+});

packages/cli/src/modules/workflow-reviews.ee/__tests__/workflow-review-eligible-reviewers.controller.integration.test.ts

@@ -0,0 +1,141 @@
+import {
+	createTeamProject,
+	createWorkflow,
+	linkUserToProject,
+	mockInstance,
+	testDb,
+} from '@n8n/backend-test-utils';
+import type { User } from '@n8n/db';
+import { Container } from '@n8n/di';
+
+import { ActiveWorkflowManager } from '@/active-workflow-manager';
+import { WorkflowReviewPolicyService } from '@/services/workflow-review-policy.service';
+import { WorkflowValidationService } from '@/workflows/workflow-validation.service';
+import { createAdmin, createUser } from '@test-integration/db/users';
+import type { SuperAgentTest } from '@test-integration/types';
+import * as utils from '@test-integration/utils';
+
+import {
+	createReviewableWorkflow,
+	REVIEW_TABLES,
+	seedReviewActors,
+	stubWorkflowValidation,
+} from './support/workflow-review-test-data';
+
+mockInstance(ActiveWorkflowManager);
+const workflowValidationService = mockInstance(WorkflowValidationService);
+
+const testServer = utils.setupTestServer({
+	endpointGroups: ['workflow-reviews', 'workflows'],
+	enabledFeatures: ['feat:workflowReviews'],
+	modules: ['workflow-reviews'],
+});
+
+let owner: User;
+let member: User;
+let ownerAgent: SuperAgentTest;
+let memberAgent: SuperAgentTest;
+let policyService: WorkflowReviewPolicyService;
+
+beforeAll(async () => {
+	await utils.initNodeTypes();
+	policyService = Container.get(WorkflowReviewPolicyService);
+});
+
+beforeEach(async () => {
+	testServer.license.enable('feat:workflowReviews');
+	await testDb.truncate([...REVIEW_TABLES]);
+	await policyService.set(true);
+	stubWorkflowValidation(workflowValidationService);
+
+	({ owner, member, ownerAgent, memberAgent } = await seedReviewActors(testServer.authAgentFor));
+});
+
+const getEligibleReviewers = (agent: SuperAgentTest, workflowId: string) =>
+	agent.get('/workflow-review-requests/eligible-reviewers').query({ workflowId });
+
+describe('GET /workflow-review-requests/eligible-reviewers', () => {
+	test('returns project viewers, editors, and instance users, excluding everyone else', async () => {
+		const project = await createTeamProject('team', owner);
+		// The requester holds workflow:publish through project:editor
+		await linkUserToProject(member, project, 'project:editor');
+
+		const projectAdmin = await createUser();
+		await linkUserToProject(projectAdmin, project, 'project:admin');
+		const projectEditor = await createUser();
+		await linkUserToProject(projectEditor, project, 'project:editor');
+		const projectViewer = await createUser();
+		await linkUserToProject(projectViewer, project, 'project:viewer');
+		const globalAdmin = await createAdmin();
+
+		const disabledEditor = await createUser({ disabled: true });
+		await linkUserToProject(disabledEditor, project, 'project:editor');
+		const pendingEditor = await createUser({ password: null });
+		await linkUserToProject(pendingEditor, project, 'project:editor');
+		await createUser(); // unrelated member
+
+		const workflow = await createWorkflow({}, project);
+
+		const response = await getEligibleReviewers(memberAgent, workflow.id).expect(200);
+
+		expect(response.body.data.count).toBe(5);
+		const ids = response.body.data.data.map((reviewer: { id: string }) => reviewer.id);
+		expect(ids.sort()).toEqual(
+			[owner.id, projectAdmin.id, projectEditor.id, projectViewer.id, globalAdmin.id].sort(),
+		);
+	});
+
+	test('returns a user holding both a project and a global qualifying role only once', async () => {
+		const project = await createTeamProject('team', owner);
+		await linkUserToProject(member, project, 'project:editor');
+		const globalAdmin = await createAdmin();
+		await linkUserToProject(globalAdmin, project, 'project:admin');
+		const workflow = await createWorkflow({}, project);
+
+		const response = await getEligibleReviewers(memberAgent, workflow.id).expect(200);
+
+		const ids = response.body.data.data.filter(
+			(reviewer: { id: string }) => reviewer.id === globalAdmin.id,
+		);
+		expect(ids).toHaveLength(1);
+	});
+
+	test('returns only instance-level reviewers for a personal-project workflow, exposing just id, email and names', async () => {
+		const globalAdmin = await createAdmin();
+		const { workflow } = await createReviewableWorkflow(owner);
+
+		const response = await getEligibleReviewers(ownerAgent, workflow.id).expect(200);
+
+		// The requesting owner is excluded; the plain member holds no read rights on this personal project
+		expect(response.body.data.count).toBe(1);
+		expect(response.body.data.data).toEqual([
+			{
+				id: globalAdmin.id,
+				email: globalAdmin.email,
+				firstName: globalAdmin.firstName,
+				lastName: globalAdmin.lastName,
+			},
+		]);
+	});
+
+	test('hides a workflow the caller cannot access', async () => {
+		const { workflow } = await createReviewableWorkflow(owner);
+
+		await getEligibleReviewers(memberAgent, workflow.id).expect(404);
+	});
+
+	test('hides a workflow the caller can only view', async () => {
+		const project = await createTeamProject('team', owner);
+		await linkUserToProject(member, project, 'project:viewer');
+		const workflow = await createWorkflow({}, project);
+
+		await getEligibleReviewers(memberAgent, workflow.id).expect(404);
+	});
+
+	test('refuses everything once an admin turns reviews off', async () => {
+		const { workflow } = await createReviewableWorkflow(owner);
+		await policyService.set(false);
+
+		await getEligibleReviewers(ownerAgent, workflow.id).expect(403);
+	});
+});

packages/cli/src/modules/workflow-reviews.ee/__tests__/workflow-review-inbox.controller.integration.test.ts

@@ -0,0 +1,715 @@
+import {
+	createTeamProject,
+	createWorkflow,
+	linkUserToProject,
+	mockInstance,
+	shareWorkflowWithUsers,
+	testDb,
+} from '@n8n/backend-test-utils';
+import type { Project, User, WorkflowReviewRequestState } from '@n8n/db';
+import {
+	UserRepository,
+	WorkflowRepository,
+	WorkflowReviewRequestAuthorRepository,
+	WorkflowReviewRequestRepository,
+	WorkflowReviewRequestReviewerRepository,
+	WorkflowReviewRequestWorkflowRepository,
+} from '@n8n/db';
+import { Container } from '@n8n/di';
+
+import { ActiveWorkflowManager } from '@/active-workflow-manager';
+import { WorkflowReviewPolicyService } from '@/services/workflow-review-policy.service';
+import { WorkflowValidationService } from '@/workflows/workflow-validation.service';
+import { createAdmin, createMember, createUser } from '@test-integration/db/users';
+import { createWorkflowHistoryItem } from '@test-integration/db/workflow-history';
+import type { SuperAgentTest } from '@test-integration/types';
+import * as utils from '@test-integration/utils';
+
+import {
+	REVIEW_TABLES,
+	seedReview,
+	seedReviewActors,
+	stubWorkflowValidation,
+	versionUpdatePayload,
+} from './support/workflow-review-test-data';
+
+mockInstance(ActiveWorkflowManager);
+const workflowValidationService = mockInstance(WorkflowValidationService);
+
+const testServer = utils.setupTestServer({
+	endpointGroups: ['workflow-reviews', 'workflows'],
+	enabledFeatures: ['feat:workflowReviews'],
+	modules: ['workflow-reviews'],
+});
+
+let owner: User;
+let member: User;
+let viewer: User;
+let ownerProject: Project;
+let teamProject: Project;
+let ownerAgent: SuperAgentTest;
+let memberAgent: SuperAgentTest;
+let viewerAgent: SuperAgentTest;
+
+let requestRepository: WorkflowReviewRequestRepository;
+let workflowRepository: WorkflowReviewRequestWorkflowRepository;
+let authorRepository: WorkflowReviewRequestAuthorRepository;
+let reviewerRepository: WorkflowReviewRequestReviewerRepository;
+let userRepository: UserRepository;
+let workflowEntityRepository: WorkflowRepository;
+let policyService: WorkflowReviewPolicyService;
+
+beforeAll(async () => {
+	await utils.initNodeTypes();
+	requestRepository = Container.get(WorkflowReviewRequestRepository);
+	workflowRepository = Container.get(WorkflowReviewRequestWorkflowRepository);
+	authorRepository = Container.get(WorkflowReviewRequestAuthorRepository);
+	reviewerRepository = Container.get(WorkflowReviewRequestReviewerRepository);
+	userRepository = Container.get(UserRepository);
+	workflowEntityRepository = Container.get(WorkflowRepository);
+	policyService = Container.get(WorkflowReviewPolicyService);
+});
+
+beforeEach(async () => {
+	testServer.license.enable('feat:workflowReviews');
+	await testDb.truncate([...REVIEW_TABLES]);
+	await policyService.set(true);
+	stubWorkflowValidation(workflowValidationService);
+
+	({ owner, member, viewer, ownerProject, teamProject, ownerAgent, memberAgent, viewerAgent } =
+		await seedReviewActors(testServer.authAgentFor));
+});
+
+/** An open request only surfaces in the inbox while it covers a live workflow. */
+async function linkToNewWorkflow(workflowReviewRequestId: string, project = teamProject) {
+	const workflow = await createWorkflow({}, project);
+	await workflowRepository.createWorkflowRow(
+		{ workflowReviewRequestId, workflowId: workflow.id },
+		{},
+	);
+	return workflow;
+}
+
+/** Seeds one open and one closed review, both with `member` as the assigned reviewer. */
+async function seedInboxRequests() {
+	const openRequest = await requestRepository.createRequest(
+		{
+			projectId: teamProject.id,
+			title: 'Open review request',
+			createdById: owner.id,
+			state: 'open',
+		},
+		{},
+	);
+	const openWorkflow = await linkToNewWorkflow(openRequest.id);
+	// No link row: a hard-deleted workflow leaves closed requests exactly like this
+	const closedRequest = await requestRepository.createRequest(
+		{
+			projectId: teamProject.id,
+			title: 'Closed review request',
+			createdById: owner.id,
+			state: 'closed',
+		},
+		{},
+	);
+	await reviewerRepository.addReviewers(
+		{ workflowReviewRequestId: openRequest.id, userIds: [member.id] },
+		{},
+	);
+	await reviewerRepository.addReviewers(
+		{ workflowReviewRequestId: closedRequest.id, userIds: [member.id] },
+		{},
+	);
+	return { openRequest, closedRequest, openWorkflow };
+}
+
+/** A review orphaned by a hard delete: the cascade drops the link row, the request stays open. */
+async function seedOrphanedOpenReview() {
+	const orphan = await requestRepository.createRequest(
+		{
+			projectId: teamProject.id,
+			title: 'Orphaned review',
+			createdById: owner.id,
+			state: 'open',
+		},
+		{},
+	);
+	await reviewerRepository.addReviewers(
+		{ workflowReviewRequestId: orphan.id, userIds: [member.id] },
+		{},
+	);
+	const workflow = await linkToNewWorkflow(orphan.id);
+	// Bypasses the auto-close hook and the sweep: the cascade removes the link
+	// row and leaves the request open — visible until the next delete sweeps it
+	await workflowEntityRepository.delete({ id: workflow.id });
+	return orphan;
+}
+
+describe('GET /workflow-review-requests/summary', () => {
+	test('counts open and closed reviews for the instance owner', async () => {
+		await seedInboxRequests();
+
+		const response = await ownerAgent.get('/workflow-review-requests/summary').expect(200);
+
+		expect(response.body.data).toEqual({ open: 1, closed: 1 });
+	});
+
+	test('counts the reviews an assigned reviewer was asked to look at', async () => {
+		await seedInboxRequests();
+
+		const response = await memberAgent.get('/workflow-review-requests/summary').expect(200);
+
+		expect(response.body.data).toEqual({ open: 1, closed: 1 });
+	});
+
+	test('counts nothing for an uninvolved project member', async () => {
+		await seedInboxRequests();
+
+		const response = await viewerAgent.get('/workflow-review-requests/summary').expect(200);
+
+		expect(response.body.data).toEqual({ open: 0, closed: 0 });
+	});
+
+	test('counts a requester their own review even without publish scope', async () => {
+		const ownRequest = await seedReview({
+			projectId: teamProject.id,
+			author: viewer,
+			title: 'Review submitted by viewer',
+		});
+		await linkToNewWorkflow(ownRequest.id);
+
+		const response = await viewerAgent.get('/workflow-review-requests/summary').expect(200);
+
+		expect(response.body.data).toEqual({ open: 1, closed: 0 });
+	});
+
+	test('still counts an open review orphaned by a workflow hard delete until a sweep closes it', async () => {
+		await seedInboxRequests();
+		await seedOrphanedOpenReview();
+
+		// Owner exercises the whole-inbox scope, member the involvement filter
+		const ownerResponse = await ownerAgent.get('/workflow-review-requests/summary').expect(200);
+		expect(ownerResponse.body.data).toEqual({ open: 2, closed: 1 });
+
+		const memberResponse = await memberAgent.get('/workflow-review-requests/summary').expect(200);
+		expect(memberResponse.body.data).toEqual({ open: 2, closed: 1 });
+	});
+
+	test('refuses the counts once an admin turns reviews off', async () => {
+		await policyService.set(false);
+
+		await ownerAgent.get('/workflow-review-requests/summary').expect(403);
+	});
+});
+
+describe('GET /workflow-review-requests/inbox', () => {
+	test('shows the instance owner every open review', async () => {
+		const { openRequest, openWorkflow } = await seedInboxRequests();
+
+		const response = await ownerAgent
+			.get('/workflow-review-requests/inbox')
+			.query({ state: 'open', limit: 15 })
+			.expect(200);
+
+		expect(response.body.data.data).toHaveLength(1);
+		expect(response.body.data.data[0]).toMatchObject({
+			id: openRequest.id,
+			title: 'Open review request',
+			state: 'open',
+			workflowName: openWorkflow.name,
+			workflowVersionId: null,
+		});
+		expect(response.body.data.hasMore).toBe(false);
+		expect(response.body.data.nextCursor).toBeNull();
+	});
+
+	test('shows nothing to an uninvolved project member', async () => {
+		await seedInboxRequests();
+
+		const response = await viewerAgent.get('/workflow-review-requests/inbox').expect(200);
+
+		expect(response.body.data.data).toEqual([]);
+		expect(response.body.data.hasMore).toBe(false);
+	});
+
+	test('still lists an open review orphaned by a workflow hard delete until a sweep closes it', async () => {
+		const { openRequest } = await seedInboxRequests();
+		const orphan = await seedOrphanedOpenReview();
+
+		// Owner exercises the whole-inbox scope, member the involvement filter
+		const ownerResponse = await ownerAgent
+			.get('/workflow-review-requests/inbox')
+			.query({ state: 'open', limit: 15 })
+			.expect(200);
+		expect(ownerResponse.body.data.data.map((row: { id: string }) => row.id).sort()).toEqual(
+			[openRequest.id, orphan.id].sort(),
+		);
+
+		const memberResponse = await memberAgent
+			.get('/workflow-review-requests/inbox')
+			.query({ state: 'open', limit: 15 })
+			.expect(200);
+		expect(memberResponse.body.data.data.map((row: { id: string }) => row.id).sort()).toEqual(
+			[openRequest.id, orphan.id].sort(),
+		);
+	});
+
+	test('still lists a closed review whose workflow was hard-deleted', async () => {
+		const { closedRequest } = await seedInboxRequests();
+
+		const response = await ownerAgent
+			.get('/workflow-review-requests/inbox')
+			.query({ state: 'closed', limit: 15 })
+			.expect(200);
+
+		// The closed seed request has no link rows — deleted-workflow history stays visible
+		expect(response.body.data.data).toEqual([
+			expect.objectContaining({ id: closedRequest.id, state: 'closed', workflowName: null }),
+		]);
+	});
+
+	test('refuses everything once an admin turns reviews off', async () => {
+		await policyService.set(false);
+
+		await ownerAgent.get('/workflow-review-requests/inbox').expect(403);
+	});
+
+	test('pages through the inbox with a cursor', async () => {
+		await seedInboxRequests();
+		const secondRequest = await requestRepository.createRequest(
+			{
+				projectId: teamProject.id,
+				title: 'Second open review',
+				createdById: owner.id,
+				state: 'open',
+			},
+			{},
+		);
+		await linkToNewWorkflow(secondRequest.id);
+
+		const firstPage = await ownerAgent
+			.get('/workflow-review-requests/inbox')
+			.query({ state: 'open', limit: 1 })
+			.expect(200);
+
+		expect(firstPage.body.data.data).toHaveLength(1);
+		expect(firstPage.body.data.hasMore).toBe(true);
+		expect(firstPage.body.data.nextCursor).toBeTruthy();
+
+		const secondPage = await ownerAgent
+			.get('/workflow-review-requests/inbox')
+			.query({
+				state: 'open',
+				limit: 1,
+				cursor: firstPage.body.data.nextCursor,
+			})
+			.expect(200);
+
+		expect(secondPage.body.data.data).toHaveLength(1);
+		expect(secondPage.body.data.data[0].id).not.toBe(firstPage.body.data.data[0].id);
+	});
+
+	test('hides reviews from projects the member cannot read, even when assigned as reviewer', async () => {
+		const otherProject = await createTeamProject('Other Reviews Project', owner);
+		const privateRequest = await requestRepository.createRequest(
+			{
+				projectId: otherProject.id,
+				title: 'Private other-project review',
+				createdById: owner.id,
+				state: 'open',
+			},
+			{},
+		);
+		await linkToNewWorkflow(privateRequest.id, otherProject);
+		// Assignment alone must not widen visibility beyond readable projects
+		await reviewerRepository.addReviewers(
+			{ workflowReviewRequestId: privateRequest.id, userIds: [member.id] },
+			{},
+		);
+
+		const memberResponse = await memberAgent.get('/workflow-review-requests/inbox').expect(200);
+		expect(memberResponse.body.data.data).toEqual([]);
+
+		const ownerResponse = await ownerAgent.get('/workflow-review-requests/inbox').expect(200);
+		expect(ownerResponse.body.data.data).toEqual(
+			expect.arrayContaining([
+				expect.objectContaining({
+					title: 'Private other-project review',
+				}),
+			]),
+		);
+	});
+
+	test("hides a requester's own review in a project they cannot read", async () => {
+		const otherProject = await createTeamProject('Unrelated Project', owner);
+		const ownRequest = await requestRepository.createRequest(
+			{
+				projectId: otherProject.id,
+				title: 'Review I submitted',
+				createdById: member.id,
+				state: 'open',
+			},
+			{},
+		);
+		await linkToNewWorkflow(ownRequest.id, otherProject);
+
+		const response = await memberAgent.get('/workflow-review-requests/inbox').expect(200);
+
+		expect(response.body.data.data).toEqual([]);
+	});
+
+	test('shows a project admin every review in their project without involvement', async () => {
+		const projectAdmin = await createMember();
+		await linkUserToProject(projectAdmin, teamProject, 'project:admin');
+		const { openRequest } = await seedInboxRequests();
+
+		const response = await testServer
+			.authAgentFor(projectAdmin)
+			.get('/workflow-review-requests/inbox')
+			.expect(200);
+
+		expect(response.body.data.data).toEqual(
+			expect.arrayContaining([expect.objectContaining({ id: openRequest.id })]),
+		);
+	});
+
+	test('does not truncate pagination when the cursor row is deleted', async () => {
+		await seedInboxRequests();
+		const secondRequest = await requestRepository.createRequest(
+			{
+				projectId: teamProject.id,
+				title: 'Second open review',
+				createdById: owner.id,
+				state: 'open',
+			},
+			{},
+		);
+		await linkToNewWorkflow(secondRequest.id);
+
+		const firstPage = await ownerAgent
+			.get('/workflow-review-requests/inbox')
+			.query({ state: 'open', limit: 1 })
+			.expect(200);
+		const cursor = firstPage.body.data.nextCursor as string;
+		const firstId = firstPage.body.data.data[0].id as string;
+
+		// Delete the anchor row before requesting the next page.
+		await requestRepository.delete({ id: firstId });
+
+		const secondPage = await ownerAgent
+			.get('/workflow-review-requests/inbox')
+			.query({ state: 'open', limit: 1, cursor })
+			.expect(200);
+
+		expect(secondPage.body.data.data).toHaveLength(1);
+		expect(secondPage.body.data.data[0].id).not.toBe(firstId);
+	});
+
+	test('hydrates the requester and requested reviewers on list items', async () => {
+		const reviewer = await createUser();
+		const request = await seedReview({
+			projectId: teamProject.id,
+			author: owner,
+			reviewerIds: [reviewer.id],
+			title: 'Needs review',
+		});
+		await linkToNewWorkflow(request.id);
+
+		const response = await ownerAgent
+			.get('/workflow-review-requests/inbox')
+			.query({ state: 'open', limit: 15 })
+			.expect(200);
+
+		const item = response.body.data.data.find((row: { id: string }) => row.id === request.id);
+		expect(item.requester).toEqual({
+			id: owner.id,
+			email: owner.email,
+			firstName: owner.firstName,
+			lastName: owner.lastName,
+		});
+		expect(item.reviewers).toEqual([
+			{
+				id: reviewer.id,
+				email: reviewer.email,
+				firstName: reviewer.firstName,
+				lastName: reviewer.lastName,
+			},
+		]);
+	});
+
+	test('drops a requester and reviewers whose accounts were deleted, and leaves a creatorless review with no requester', async () => {
+		// No FK on these rows, so a deleted user leaves a dangling id that must resolve to null.
+		const departedCreator = await createUser();
+		const survivingReviewer = await createUser();
+		const departedReviewer = await createUser();
+		const request = await requestRepository.createRequest(
+			{
+				projectId: teamProject.id,
+				title: 'With departed users',
+				createdById: departedCreator.id,
+				state: 'open',
+			},
+			{},
+		);
+		await linkToNewWorkflow(request.id);
+		await reviewerRepository.addReviewers(
+			{
+				workflowReviewRequestId: request.id,
+				userIds: [survivingReviewer.id, departedReviewer.id],
+			},
+			{},
+		);
+
+		// A review whose `createdById` was never set at all.
+		const authorless = await requestRepository.createRequest(
+			{ projectId: teamProject.id, title: 'Authorless', createdById: null, state: 'open' },
+			{},
+		);
+		await linkToNewWorkflow(authorless.id);
+
+		await userRepository.delete({ id: departedCreator.id });
+		await userRepository.delete({ id: departedReviewer.id });
+
+		const response = await ownerAgent
+			.get('/workflow-review-requests/inbox')
+			.query({ state: 'open', limit: 15 })
+			.expect(200);
+
+		const rows = response.body.data.data as Array<{
+			id: string;
+			requester: unknown;
+			reviewers: unknown[];
+		}>;
+		const withDeparted = rows.find((row) => row.id === request.id)!;
+		expect(withDeparted.requester).toBeNull();
+		expect(withDeparted.reviewers).toEqual([
+			{
+				id: survivingReviewer.id,
+				email: survivingReviewer.email,
+				firstName: survivingReviewer.firstName,
+				lastName: survivingReviewer.lastName,
+			},
+		]);
+
+		const withoutCreator = rows.find((row) => row.id === authorless.id)!;
+		expect(withoutCreator.requester).toBeNull();
+		expect(withoutCreator.reviewers).toEqual([]);
+	});
+
+	describe('category filter', () => {
+		/** Mirrors the create endpoint: a requester always gets an author row too. */
+		async function openReviewBy(
+			createdById: string | null,
+			title = 'Review',
+			state: WorkflowReviewRequestState = 'open',
+		) {
+			const request = await requestRepository.createRequest(
+				{ projectId: teamProject.id, title, createdById, state },
+				{},
+			);
+			if (createdById) {
+				await authorRepository.addAuthor(
+					{ workflowReviewRequestId: request.id, userId: createdById },
+					{},
+				);
+			}
+			return request;
+		}
+
+		async function inbox(
+			agent: SuperAgentTest,
+			query: Record<string, unknown>,
+		): Promise<{ ids: string[]; hasMore: boolean; nextCursor: string | null }> {
+			const response = await agent
+				.get('/workflow-review-requests/inbox')
+				.query({ state: 'open', limit: 15, ...query })
+				.expect(200);
+			return {
+				ids: response.body.data.data.map((row: { id: string }) => row.id),
+				hasMore: response.body.data.hasMore,
+				nextCursor: response.body.data.nextCursor,
+			};
+		}
+
+		/** Assign `member` as reviewer so the review is visible to them at all. */
+		async function assignMember(workflowReviewRequestId: string) {
+			await reviewerRepository.addReviewers({ workflowReviewRequestId, userIds: [member.id] }, {});
+		}
+
+		test('splits the visible union by who authored each review', async () => {
+			const mine = await openReviewBy(member.id, 'Submitted by me');
+			const theirs = await openReviewBy(owner.id, 'Submitted by someone else');
+			await assignMember(theirs.id);
+
+			expect((await inbox(memberAgent, { category: 'authored' })).ids).toEqual([mine.id]);
+			expect((await inbox(memberAgent, { category: 'waiting' })).ids).toEqual([theirs.id]);
+		});
+
+		test('the two categories add up to the unfiltered list', async () => {
+			await openReviewBy(member.id, 'Mine');
+			const theirs = await openReviewBy(owner.id, 'Theirs');
+			await assignMember(theirs.id);
+			const coAuthored = await openReviewBy(owner.id, 'Co-authored');
+			await authorRepository.addAuthor(
+				{ workflowReviewRequestId: coAuthored.id, userId: member.id },
+				{},
+			);
+
+			const all = await inbox(memberAgent, {});
+			const waiting = await inbox(memberAgent, { category: 'waiting' });
+			const authored = await inbox(memberAgent, { category: 'authored' });
+
+			expect([...waiting.ids, ...authored.ids].sort()).toEqual([...all.ids].sort());
+			expect(waiting.ids.filter((id) => authored.ids.includes(id))).toEqual([]);
+		});
+
+		test('counts a co-author row as authorship, not just the creator', async () => {
+			const request = await openReviewBy(owner.id, 'Created by owner');
+			await authorRepository.addAuthor(
+				{ workflowReviewRequestId: request.id, userId: member.id },
+				{},
+			);
+
+			expect((await inbox(memberAgent, { category: 'authored' })).ids).toEqual([request.id]);
+			expect((await inbox(memberAgent, { category: 'waiting' })).ids).toEqual([]);
+		});
+
+		test('keeps a review under waiting for its reviewer even after they submit a version to it', async () => {
+			const workflow = await createWorkflow({}, teamProject);
+			await createWorkflowHistoryItem(workflow.id, { versionId: 'version-1' });
+			await createWorkflowHistoryItem(workflow.id, { versionId: 'version-2' });
+			const request = await openReviewBy(owner.id, 'Owner review');
+			await workflowRepository.createWorkflowRow(
+				{
+					workflowReviewRequestId: request.id,
+					workflowId: workflow.id,
+					workflowVersionId: 'version-1',
+				},
+				{},
+			);
+			await assignMember(request.id);
+
+			expect((await inbox(memberAgent, { category: 'waiting' })).ids).toEqual([request.id]);
+
+			await memberAgent
+				.post(`/workflow-review-requests/${request.id}/update-version`)
+				.send(versionUpdatePayload({ workflowId: workflow.id, versionId: 'version-2' }))
+				.expect(200);
+
+			// The reviewer assignment wins over the authorship the re-pin created:
+			// a decision is still expected from them, so the review must not move.
+			expect((await inbox(memberAgent, { category: 'waiting' })).ids).toEqual([request.id]);
+			expect((await inbox(memberAgent, { category: 'authored' })).ids).toEqual([]);
+		});
+
+		test('puts an admin their own review under authored only, despite global scope', async () => {
+			const admin = await createAdmin();
+			const adminAgent = testServer.authAgentFor(admin);
+			const adminReview = await openReviewBy(admin.id, 'Admin review');
+			const otherReview = await openReviewBy(owner.id, 'Someone else review');
+
+			expect((await inbox(adminAgent, { category: 'authored' })).ids).toEqual([adminReview.id]);
+			expect((await inbox(adminAgent, { category: 'waiting' })).ids).toEqual([otherReview.id]);
+		});
+
+		async function openUnreachableReview(createdById: string, title: string) {
+			const otherProject = await createTeamProject('Unreachable Project', owner);
+			const unreachableWorkflow = await createWorkflow({}, otherProject);
+			const request = await requestRepository.createRequest(
+				{ projectId: otherProject.id, title, createdById, state: 'open' },
+				{},
+			);
+			await workflowRepository.createWorkflowRow(
+				{ workflowReviewRequestId: request.id, workflowId: unreachableWorkflow.id },
+				{},
+			);
+			return request;
+		}
+
+		test("hides a creator's review once they cannot read the workflow it covers", async () => {
+			await openUnreachableReview(member.id, 'Review I submitted elsewhere');
+
+			expect((await inbox(memberAgent, { category: 'authored' })).ids).toEqual([]);
+			expect((await inbox(memberAgent, { category: 'waiting' })).ids).toEqual([]);
+			expect((await inbox(memberAgent, {})).ids).toEqual([]);
+		});
+
+		test('hides a co-authored review from both categories when its workflow is unreadable', async () => {
+			const request = await openUnreachableReview(owner.id, 'Out of reach');
+			await authorRepository.addAuthor(
+				{ workflowReviewRequestId: request.id, userId: member.id },
+				{},
+			);
+
+			expect((await inbox(memberAgent, { category: 'authored' })).ids).toEqual([]);
+			expect((await inbox(memberAgent, { category: 'waiting' })).ids).toEqual([]);
+			expect((await inbox(memberAgent, {})).ids).toEqual([]);
+		});
+
+		test('shows a requester the review for a workflow shared only with them', async () => {
+			const sharedWorkflow = await createWorkflow({}, owner);
+			await shareWorkflowWithUsers(sharedWorkflow, [member]);
+			const request = await seedReview({
+				projectId: ownerProject.id,
+				workflowId: sharedWorkflow.id,
+				author: member,
+				title: 'Shared with me',
+			});
+
+			expect((await inbox(memberAgent, { category: 'authored' })).ids).toEqual([request.id]);
+		});
+
+		// The negation has to be NULL-safe: `createdById` is nullable once the creator
+		// is deleted, and those reviews still need somewhere to show up.
+		test('keeps a review whose creator is gone under waiting', async () => {
+			const orphan = await openReviewBy(null, 'Creatorless');
+			await assignMember(orphan.id);
+
+			// The reviewer reaches it through their assignment; the owner through the
+			// NULL-safe authorship negation (they see everything, and author nobody's).
+			expect((await inbox(memberAgent, { category: 'waiting' })).ids).toEqual([orphan.id]);
+			expect((await inbox(memberAgent, { category: 'authored' })).ids).toEqual([]);
+			expect((await inbox(ownerAgent, { category: 'waiting' })).ids).toEqual([orphan.id]);
+			expect((await inbox(ownerAgent, { category: 'authored' })).ids).toEqual([]);
+		});
+
+		test('paginates each category with its own independent cursor', async () => {
+			const waitingOlder = await openReviewBy(owner.id, 'Waiting older');
+			const waitingNewer = await openReviewBy(owner.id, 'Waiting newer');
+			await assignMember(waitingOlder.id);
+			await assignMember(waitingNewer.id);
+			const authored = await openReviewBy(member.id, 'Authored only');
+
+			const firstWaitingPage = await inbox(memberAgent, { category: 'waiting', limit: 1 });
+			expect(firstWaitingPage.ids).toHaveLength(1);
+			expect(firstWaitingPage.hasMore).toBe(true);
+
+			const secondWaitingPage = await inbox(memberAgent, {
+				category: 'waiting',
+				limit: 1,
+				cursor: firstWaitingPage.nextCursor,
+			});
+			expect(secondWaitingPage.hasMore).toBe(false);
+			expect([...firstWaitingPage.ids, ...secondWaitingPage.ids].sort()).toEqual(
+				[waitingOlder.id, waitingNewer.id].sort(),
+			);
+
+			// The authored section paginates on its own, unaffected by the waiting cursor.
+			const authoredPage = await inbox(memberAgent, { category: 'authored', limit: 1 });
+			expect(authoredPage.ids).toEqual([authored.id]);
+			expect(authoredPage.hasMore).toBe(false);
+			expect(authoredPage.nextCursor).toBeNull();
+		});
+
+		// The category filter ignores `state`, even though the editor sends it only
+		// for open reviews.
+		test('filters closed reviews by category too', async () => {
+			const closedMine = await openReviewBy(member.id, 'Closed mine', 'closed');
+			await openReviewBy(owner.id, 'Closed theirs', 'closed');
+
+			expect((await inbox(memberAgent, { state: 'closed', category: 'authored' })).ids).toEqual([
+				closedMine.id,
+			]);
+		});
+	});
+});

packages/cli/src/modules/workflow-reviews.ee/__tests__/workflow-review-inbox.detail.service.test.ts

@@ -174,7 +174,6 @@ describe('WorkflowReviewInboxService.getDetail', () => {
 		// review — history of a deleted workflow — can legitimately cover none
 		it('returns a closed review with no workflows when its workflow was deleted', async () => {
 			mockGate([], reviewRequest({ state: 'closed' }));
-			workflowRepository.findLinkedWorkflowDetailsByRequestId.mockResolvedValue([]);
 
 			const detail = await service.getDetail(requester, requestId);
 
@@ -185,8 +184,6 @@ describe('WorkflowReviewInboxService.getDetail', () => {
 		// An open review can transiently cover no workflow when a delete orphaned
 		// it and the sweep hasn't closed it yet — it stays readable until then
 		it('returns an open review with no workflows when its workflow was deleted', async () => {
-			workflowRepository.findLinkedWorkflowDetailsByRequestId.mockResolvedValue([]);
-
 			const detail = await service.getDetail(requester, requestId);
 
 			expect(detail.state).toBe('open');
@@ -264,9 +261,8 @@ describe('WorkflowReviewInboxService.getDetail', () => {
 			expect(detail.viewerCanComment).toBe(false);
 		});
 
-		it('passes empty coverage when a closed review no longer covers any workflow', async () => {
+		it('passes the closed review and its empty coverage to the eligibility check', async () => {
 			mockGate([], reviewRequest({ state: 'closed' }));
-			workflowRepository.findLinkedWorkflowDetailsByRequestId.mockResolvedValue([]);
 
 			await service.getDetail(requester, requestId);
 
@@ -401,7 +397,15 @@ describe('WorkflowReviewInboxService.getDetail', () => {
 			expect(detail.workflows[0]?.baselineVersion).toMatchObject({ versionId: 'ver-frozen' });
 		});
 
-		it('returns no baseline for a closed review when none was captured', async () => {
+		/**
+		 * The row's own state decides, not the request's: the request is read before its
+		 * rows, so an approval landing in between leaves the request looking open. A
+		 * frozen null would otherwise read as "still open" and resolve the live version,
+		 * showing a diff nobody approved. Null on a closed row can mean never published,
+		 * approved while unpublished, or closed without an approval — callers tell those
+		 * apart via `state` + `decision`, which is why none of them is an input here.
+		 */
+		it('has nothing to compare against on a closed row, whatever the request says', async () => {
 			mockGate(
 				[
 					{
@@ -423,56 +427,5 @@ describe('WorkflowReviewInboxService.getDetail', () => {
 
 			expect(detail.workflows[0]?.baselineVersion).toBeNull();
 		});
-
-		it('keeps a captured null null when the request row was read before the approval', async () => {
-			// The request is fetched before its rows, so an approval landing in between
-			// leaves the request looking open. The row's own state has to win: a frozen null
-			// baseline would otherwise read as "still open" and resolve the live version.
-			mockGate(
-				[
-					{
-						workflowId,
-						workflowName: 'My workflow',
-						workflowVersionId: 'ver-pinned',
-						activeVersionId: 'ver-pinned',
-						baselineVersionId: null,
-						requestState: 'closed',
-					},
-				],
-				reviewRequest({ state: 'open', decision: 'pending' }),
-			);
-			workflowHistoryService.findVersion.mockImplementation(async (_workflowId, versionId) =>
-				historyVersion(versionId),
-			);
-
-			const detail = await service.getDetail(requester, requestId);
-
-			expect(detail.workflows[0]?.baselineVersion).toBeNull();
-		});
-
-		it('returns no baseline for a closed review that was never approved', async () => {
-			mockGate(
-				[
-					{
-						workflowId,
-						workflowName: 'My workflow',
-						workflowVersionId: 'ver-pinned',
-						activeVersionId: 'ver-live-now',
-						baselineVersionId: null,
-						requestState: 'closed',
-					},
-				],
-				reviewRequest({ state: 'closed', decision: 'pending' }),
-			);
-			workflowHistoryService.findVersion.mockImplementation(async (_workflowId, versionId) =>
-				historyVersion(versionId),
-			);
-
-			const detail = await service.getDetail(requester, requestId);
-
-			expect(detail.state).toBe('closed');
-			expect(detail.decision).toBe('pending');
-			expect(detail.workflows[0]?.baselineVersion).toBeNull();
-		});
 	});
 });

packages/cli/src/modules/workflow-reviews.ee/__tests__/workflow-review-inbox.list.service.test.ts

@@ -15,7 +15,7 @@ import type {
 import { WorkflowReviewPolicyService } from '@/services/workflow-review-policy.service';
 import type { WorkflowHistoryService } from '@/workflows/workflow-history/workflow-history.service';
 
-describe('WorkflowReviewInboxService', () => {
+describe('WorkflowReviewInboxService.listForInbox', () => {
 	const workflowReviewPolicyService = mockInstance(WorkflowReviewPolicyService);
 	const authorizationService = mock<WorkflowReviewAuthorizationService>();
 	const workflowHistoryService = mock<WorkflowHistoryService>();
@@ -61,160 +61,124 @@ describe('WorkflowReviewInboxService', () => {
 		readableWorkflowRoles: ['workflow:owner', 'workflow:editor'],
 	};
 
-	describe('listForInbox', () => {
-		function mockVisibility(visibility: InboxVisibility = involvedVisibility) {
-			authorizationService.resolveInboxVisibility.mockResolvedValueOnce(visibility);
-		}
+	function mockVisibility(visibility: InboxVisibility = involvedVisibility) {
+		authorizationService.resolveInboxVisibility.mockResolvedValueOnce(visibility);
+	}
 
-		it('returns paginated data with hasMore and nextCursor', async () => {
-			mockVisibility();
-			const rows = [
-				mock<WorkflowReviewRequest>({
-					id: 'req-2',
-					projectId: 'proj-1',
-					title: 'Second',
-					decision: 'pending',
-					state: 'open',
-					createdAt: new Date('2024-01-02T00:00:00.000Z'),
-					updatedAt: new Date('2024-01-02T00:00:00.000Z'),
-				}),
-				mock<WorkflowReviewRequest>({
-					id: 'req-1',
-					projectId: 'proj-1',
-					title: 'First',
-					decision: 'pending',
-					state: 'open',
-					createdAt: new Date('2024-01-01T00:00:00.000Z'),
-					updatedAt: new Date('2024-01-01T00:00:00.000Z'),
-				}),
-			];
-			workflowReviewInboxRepository.findRequests.mockResolvedValue(rows);
-			workflowReviewRequestWorkflowRepository.findLinkedWorkflowsByRequestIds.mockResolvedValue(
-				new Map([['req-2', { workflowName: 'Linked workflow', workflowVersionId: 'ver-2' }]]),
-			);
-
-			const result = await service.listForInbox(user, { limit: 1 });
-
-			expect(workflowReviewInboxRepository.findRequests).toHaveBeenCalledWith({
-				visibility: involvedVisibility,
+	it('returns paginated data with hasMore and nextCursor', async () => {
+		mockVisibility();
+		const rows = [
+			mock<WorkflowReviewRequest>({
+				id: 'req-2',
+				projectId: 'proj-1',
+				title: 'Second',
+				decision: 'pending',
 				state: 'open',
-				limit: 2,
-				cursor: undefined,
-			});
-			expect(result.data).toHaveLength(1);
-			expect(result.data[0]?.workflowName).toBe('Linked workflow');
-			expect(result.data[0]?.workflowVersionId).toBe('ver-2');
-			expect(result.hasMore).toBe(true);
-			// nextCursor encodes the last row's keyset boundary (createdAt + id).
-			const expectedCursor = Buffer.from('2024-01-02T00:00:00.000Z|req-2', 'utf8').toString(
-				'base64url',
-			);
-			expect(result.nextCursor).toBe(expectedCursor);
-			// Participants are only resolved for the page, never for the lookahead row.
-			expect(participantResolver.resolve).toHaveBeenCalledWith([rows[0]]);
-		});
+				createdAt: new Date('2024-01-02T00:00:00.000Z'),
+				updatedAt: new Date('2024-01-02T00:00:00.000Z'),
+			}),
+			mock<WorkflowReviewRequest>({
+				id: 'req-1',
+				projectId: 'proj-1',
+				title: 'First',
+				decision: 'pending',
+				state: 'open',
+				createdAt: new Date('2024-01-01T00:00:00.000Z'),
+				updatedAt: new Date('2024-01-01T00:00:00.000Z'),
+			}),
+		];
+		workflowReviewInboxRepository.findRequests.mockResolvedValue(rows);
+		workflowReviewRequestWorkflowRepository.findLinkedWorkflowsByRequestIds.mockResolvedValue(
+			new Map([['req-2', { workflowName: 'Linked workflow', workflowVersionId: 'ver-2' }]]),
+		);
 
-		it('decodes the incoming cursor into a keyset boundary', async () => {
-			mockVisibility();
+		const result = await service.listForInbox(user, { limit: 1 });
+
+		expect(workflowReviewInboxRepository.findRequests).toHaveBeenCalledWith({
+			visibility: involvedVisibility,
+			state: 'open',
+			limit: 2,
+			cursor: undefined,
+		});
+		expect(result.data).toHaveLength(1);
+		expect(result.data[0]?.workflowName).toBe('Linked workflow');
+		expect(result.data[0]?.workflowVersionId).toBe('ver-2');
+		expect(result.hasMore).toBe(true);
+		// nextCursor encodes the last row's keyset boundary (createdAt + id).
+		const expectedCursor = Buffer.from('2024-01-02T00:00:00.000Z|req-2', 'utf8').toString(
+			'base64url',
+		);
+		expect(result.nextCursor).toBe(expectedCursor);
+		// Participants are only resolved for the page, never for the lookahead row.
+		expect(participantResolver.resolve).toHaveBeenCalledWith([rows[0]]);
+	});
+
+	it('decodes the incoming cursor into a keyset boundary', async () => {
+		mockVisibility();
+		workflowReviewInboxRepository.findRequests.mockResolvedValue([]);
+		workflowReviewRequestWorkflowRepository.findLinkedWorkflowsByRequestIds.mockResolvedValue(
+			new Map(),
+		);
+		const cursor = Buffer.from('2024-01-02T00:00:00.000Z|req-2', 'utf8').toString('base64url');
+
+		await service.listForInbox(user, { limit: 15, cursor });
+
+		expect(workflowReviewInboxRepository.findRequests).toHaveBeenCalledWith(
+			expect.objectContaining({
+				cursor: { createdAt: new Date('2024-01-02T00:00:00.000Z'), id: 'req-2' },
+			}),
+		);
+	});
+
+	it('rejects a malformed cursor', async () => {
+		mockVisibility();
+		const cursor = Buffer.from('not-a-valid-cursor', 'utf8').toString('base64url');
+
+		await expect(service.listForInbox(user, { limit: 15, cursor })).rejects.toThrow(
+			'Invalid pagination cursor',
+		);
+	});
+
+	describe('category', () => {
+		beforeEach(() => {
 			workflowReviewInboxRepository.findRequests.mockResolvedValue([]);
 			workflowReviewRequestWorkflowRepository.findLinkedWorkflowsByRequestIds.mockResolvedValue(
 				new Map(),
 			);
-			const cursor = Buffer.from('2024-01-02T00:00:00.000Z|req-2', 'utf8').toString('base64url');
+		});
 
-			await service.listForInbox(user, { limit: 15, cursor });
+		it.each(['authored', 'waiting'] as const)(
+			'passes category %s through with the requesting user',
+			async (category) => {
+				mockVisibility();
+
+				await service.listForInbox(user, { limit: 15, category });
+
+				expect(workflowReviewInboxRepository.findRequests).toHaveBeenCalledWith(
+					expect.objectContaining({ category: { userId: 'user-1', category } }),
+				);
+			},
+		);
+
+		it('derives the user from the request, never from the query', async () => {
+			mockVisibility();
+			const otherUser = mock<User>({ id: 'user-2', role: { slug: 'global:member', scopes: [] } });
+
+			await service.listForInbox(otherUser, { limit: 15, category: 'authored' });
 
 			expect(workflowReviewInboxRepository.findRequests).toHaveBeenCalledWith(
-				expect.objectContaining({
-					cursor: { createdAt: new Date('2024-01-02T00:00:00.000Z'), id: 'req-2' },
-				}),
+				expect.objectContaining({ category: { userId: 'user-2', category: 'authored' } }),
 			);
 		});
 
-		it('rejects a malformed cursor', async () => {
+		it('leaves the query unfiltered when the category is omitted', async () => {
 			mockVisibility();
-			const cursor = Buffer.from('not-a-valid-cursor', 'utf8').toString('base64url');
 
-			await expect(service.listForInbox(user, { limit: 15, cursor })).rejects.toThrow(
-				'Invalid pagination cursor',
+			await service.listForInbox(user, { limit: 15 });
+
+			expect(workflowReviewInboxRepository.findRequests).toHaveBeenCalledWith(
+				expect.objectContaining({ category: undefined }),
 			);
 		});
-
-		describe('category', () => {
-			beforeEach(() => {
-				workflowReviewInboxRepository.findRequests.mockResolvedValue([]);
-				workflowReviewRequestWorkflowRepository.findLinkedWorkflowsByRequestIds.mockResolvedValue(
-					new Map(),
-				);
-			});
-
-			it.each(['authored', 'waiting'] as const)(
-				'passes category %s through with the requesting user',
-				async (category) => {
-					mockVisibility();
-
-					await service.listForInbox(user, { limit: 15, category });
-
-					expect(workflowReviewInboxRepository.findRequests).toHaveBeenCalledWith(
-						expect.objectContaining({ category: { userId: 'user-1', category } }),
-					);
-				},
-			);
-
-			it('derives the user from the request, never from the query', async () => {
-				mockVisibility();
-				const otherUser = mock<User>({ id: 'user-2', role: { slug: 'global:member', scopes: [] } });
-
-				await service.listForInbox(otherUser, { limit: 15, category: 'authored' });
-
-				expect(workflowReviewInboxRepository.findRequests).toHaveBeenCalledWith(
-					expect.objectContaining({ category: { userId: 'user-2', category: 'authored' } }),
-				);
-			});
-
-			it('leaves the query unfiltered when the category is omitted', async () => {
-				mockVisibility();
-
-				await service.listForInbox(user, { limit: 15 });
-
-				expect(workflowReviewInboxRepository.findRequests).toHaveBeenCalledWith(
-					expect.objectContaining({ category: undefined }),
-				);
-			});
-		});
-	});
-	describe('participants on inbox items', () => {
-		const inboxRow = mock<WorkflowReviewRequest>({
-			id: 'req-1',
-			projectId: 'proj-1',
-			title: 'First',
-			decision: 'pending',
-			state: 'open',
-			createdById: 'requester-1',
-			createdAt: new Date('2024-01-01T00:00:00.000Z'),
-			updatedAt: new Date('2024-01-01T00:00:00.000Z'),
-		});
-
-		beforeEach(() => {
-			authorizationService.resolveInboxVisibility.mockResolvedValue(involvedVisibility);
-			workflowReviewInboxRepository.findRequests.mockResolvedValue([inboxRow]);
-			workflowReviewRequestWorkflowRepository.findLinkedWorkflowsByRequestIds.mockResolvedValue(
-				new Map(),
-			);
-		});
-
-		it('carries the resolved participants onto the inbox item', async () => {
-			mockParticipants({
-				requester: mock({ id: 'requester-1' }),
-				authors: [mock({ id: 'requester-1' }), mock({ id: 'author-2' })],
-				reviewers: [mock({ id: 'reviewer-1' })],
-			});
-
-			const [item] = (await service.listForInbox(user, { limit: 15 })).data;
-
-			expect(item?.requester).toMatchObject({ id: 'requester-1' });
-			expect(item?.authors.map((author) => author.id)).toEqual(['requester-1', 'author-2']);
-			expect(item?.reviewers.map((reviewer) => reviewer.id)).toEqual(['reviewer-1']);
-		});
 	});
 });

packages/cli/src/modules/workflow-reviews.ee/__tests__/workflow-review-lifecycle.integration.test.ts

@@ -1,11 +1,6 @@
 import type { SourceControlledFile } from '@n8n/api-types';
-import {
-	createTeamProject,
-	createWorkflow,
-	getPersonalProject,
-	mockInstance,
-	testDb,
-} from '@n8n/backend-test-utils';
+import { createTeamProject, mockInstance, testDb } from '@n8n/backend-test-utils';
+import { GlobalConfig } from '@n8n/config';
 import type { Project, User } from '@n8n/db';
 import {
 	FolderRepository,
@@ -15,7 +10,6 @@ import {
 	WorkflowRepository,
 	WorkflowReviewActivityRepository,
 	WorkflowReviewLifecycleRepository,
-	WorkflowReviewRequestAuthorRepository,
 	WorkflowReviewRequestRepository,
 	WorkflowReviewRequestWorkflowRepository,
 } from '@n8n/db';
@@ -25,24 +19,29 @@ import { readFile } from 'node:fs/promises';
 import { v4 as uuid } from 'uuid';
 import { mock } from 'vitest-mock-extended';
 
-import { GlobalConfig } from '@n8n/config';
-
 import { ActiveWorkflowManager } from '@/active-workflow-manager';
 import { EventService } from '@/events/event.service';
 import { SourceControlImportService } from '@/modules/source-control.ee/source-control-import.service.ee';
-import { WorkflowPublicationNotifier } from '@/workflows/publication/workflow-publication-notifier';
-import { WorkflowValidationService } from '@/workflows/workflow-validation.service';
 import { WorkflowReviewPolicyService } from '@/services/workflow-review-policy.service';
+import { WorkflowPublicationNotifier } from '@/workflows/publication/workflow-publication-notifier';
 import { WorkflowHistoryService } from '@/workflows/workflow-history/workflow-history.service';
 import { WorkflowMutationHooksProxy } from '@/workflows/workflow-mutation-hooks-proxy.service';
-import { EnterpriseWorkflowService } from '@/workflows/workflow.service.ee';
+import { WorkflowValidationService } from '@/workflows/workflow-validation.service';
 import { WorkflowService } from '@/workflows/workflow.service';
+import { EnterpriseWorkflowService } from '@/workflows/workflow.service.ee';
 import { createFolder } from '@test-integration/db/folders';
-import { createOwner } from '@test-integration/db/users';
 import { createWorkflowHistoryItem } from '@test-integration/db/workflow-history';
 import type { SuperAgentTest } from '@test-integration/types';
 import * as utils from '@test-integration/utils';
 
+import {
+	createReviewableWorkflow,
+	REVIEW_TABLES,
+	seedReview,
+	seedReviewActors,
+	stubWorkflowValidation,
+} from './support/workflow-review-test-data';
+
 const activeWorkflowManager = mockInstance(ActiveWorkflowManager);
 const workflowValidationService = mockInstance(WorkflowValidationService);
 
@@ -68,7 +67,6 @@ let ownerAgent: SuperAgentTest;
 let requestRepository: WorkflowReviewRequestRepository;
 let lifecycleRepository: WorkflowReviewLifecycleRepository;
 let linkRepository: WorkflowReviewRequestWorkflowRepository;
-let authorRepository: WorkflowReviewRequestAuthorRepository;
 let activityRepository: WorkflowReviewActivityRepository;
 
 beforeAll(async () => {
@@ -76,43 +74,17 @@ beforeAll(async () => {
 	requestRepository = Container.get(WorkflowReviewRequestRepository);
 	lifecycleRepository = Container.get(WorkflowReviewLifecycleRepository);
 	linkRepository = Container.get(WorkflowReviewRequestWorkflowRepository);
-	authorRepository = Container.get(WorkflowReviewRequestAuthorRepository);
 	activityRepository = Container.get(WorkflowReviewActivityRepository);
 });
 
 beforeEach(async () => {
 	testServer.license.enable('feat:workflowReviews');
-
-	await testDb.truncate([
-		'WorkflowReviewActivityComment',
-		'WorkflowReviewActivity',
-		'WorkflowReviewRequestAuthor',
-		'WorkflowReviewRequestReviewer',
-		'WorkflowReviewRequestWorkflow',
-		'WorkflowReviewRequest',
-		'SharedWorkflow',
-		'WorkflowPublishedVersion',
-		'WorkflowPublicationOutbox',
-		'WorkflowPublishHistory',
-		'WorkflowEntity',
-		'WorkflowHistory',
-		'Folder',
-		'ProjectRelation',
-		'Project',
-		'User',
-	]);
-
+	// `Folder` on top of the shared list: only the source-control pull tests need it.
+	await testDb.truncate(['Folder', ...REVIEW_TABLES]);
 	await Container.get(WorkflowReviewPolicyService).set(true);
+	stubWorkflowValidation(workflowValidationService);
 
-	// Test workflows have no trigger nodes; activation must not fail on that.
-	workflowValidationService.validateForActivation.mockReturnValue({ isValid: true });
-	workflowValidationService.validateDynamicCredentials.mockResolvedValue({ isValid: true });
-	workflowValidationService.validateSubWorkflowReferences.mockResolvedValue({ isValid: true });
-	workflowValidationService.validateTriggerNodeIds.mockReturnValue({ isValid: true });
-
-	owner = await createOwner();
-	ownerProject = await getPersonalProject(owner);
-	ownerAgent = testServer.authAgentFor(owner);
+	({ owner, ownerProject, ownerAgent } = await seedReviewActors(testServer.authAgentFor));
 });
 
 /** Read through the endpoint, so the entries are asserted exactly as a reader sees them. */
@@ -132,12 +104,23 @@ const closedEntry = expect.objectContaining({
 	data: { reason: 'no-reviewable-workflows' },
 });
 
-/** Create a workflow owned by `owner` with a pinned history version. */
-async function createReviewableWorkflow(attributes: { isArchived?: boolean } = {}) {
-	const versionId = uuid();
-	const workflow = await createWorkflow({ versionId, ...attributes }, owner);
-	await createWorkflowHistoryItem(workflow.id, { versionId });
-	return { workflow, versionId };
+/**
+ * Take the activity writes down for the duration of `mutate`, then restore them.
+ *
+ * Scoping it to the one mutation matters: counting `mockRejectedValueOnce` calls
+ * instead couples the test to how many writes that mutation happens to attempt,
+ * and an unconsumed rejection then leaks into the *next* mutation — which is the
+ * sweep these tests are trying to observe working.
+ */
+async function withActivityWritesDown(mutate: () => Promise<unknown>) {
+	const spy = vi
+		.spyOn(activityRepository, 'createActivity')
+		.mockRejectedValue(new Error('write failed'));
+	try {
+		await mutate();
+	} finally {
+		spy.mockRestore();
+	}
 }
 
 async function createOpenReview(
@@ -148,27 +131,18 @@ async function createOpenReview(
 		decision?: 'pending' | 'changes_requested' | 'approved';
 	} = {},
 ) {
-	const request = await requestRepository.createRequest(
-		{
-			projectId: ownerProject.id,
-			title: 'Review before publishing',
-			createdById: owner.id,
-			state: overrides.state,
-			decision: overrides.decision,
-		},
-		{},
-	);
-	await linkRepository.createWorkflowRow(
-		{ workflowReviewRequestId: request.id, workflowId, workflowVersionId: versionId },
-		{},
-	);
-	await authorRepository.addAuthor({ workflowReviewRequestId: request.id, userId: owner.id }, {});
-	return request;
+	return await seedReview({
+		projectId: ownerProject.id,
+		workflowId,
+		versionId,
+		author: owner,
+		...overrides,
+	});
 }
 
 describe('auto-close on workflow archive', () => {
 	test('archiving records the cause and the close, leaving the decision unchanged', async () => {
-		const { workflow, versionId } = await createReviewableWorkflow();
+		const { workflow, versionId } = await createReviewableWorkflow(owner);
 		const request = await createOpenReview(workflow.id, versionId, {
 			decision: 'changes_requested',
 		});
@@ -212,7 +186,7 @@ describe('auto-close on workflow archive', () => {
 	// mutation. The reconciliation sweep that follows the hook picks the rolled-back review
 	// straight back up, closing it without the (unrecoverable) cause entry.
 	test('archives the workflow anyway when the cause cannot be recorded, and the sweep closes it', async () => {
-		const { workflow, versionId } = await createReviewableWorkflow();
+		const { workflow, versionId } = await createReviewableWorkflow(owner);
 		const request = await createOpenReview(workflow.id, versionId);
 		vi.spyOn(activityRepository, 'createActivity').mockRejectedValueOnce(new Error('write failed'));
 
@@ -225,18 +199,16 @@ describe('auto-close on workflow archive', () => {
 	// Both the targeted close and the sweep that follows it are down, so the review is stranded
 	// open on a workflow that is already archived — the state the next sweep has to repair.
 	test('a review stranded open on an archived workflow is closed by the next sweep', async () => {
-		const { workflow, versionId } = await createReviewableWorkflow();
+		const { workflow, versionId } = await createReviewableWorkflow(owner);
 		const request = await createOpenReview(workflow.id, versionId);
-		vi.spyOn(activityRepository, 'createActivity')
-			.mockRejectedValueOnce(new Error('write failed'))
-			.mockRejectedValueOnce(new Error('write failed'));
-
-		await ownerAgent.post(`/workflows/${workflow.id}/archive`).expect(200);
+		await withActivityWritesDown(
+			async () => await ownerAgent.post(`/workflows/${workflow.id}/archive`).expect(200),
+		);
 		expect((await requestRepository.findById(request.id, {}))?.state).toBe('open');
 
 		// Any later lifecycle mutation runs the sweep again — this one touches an unrelated
 		// workflow, so only the sweep can reach the stranded review.
-		const unrelated = await createReviewableWorkflow();
+		const unrelated = await createReviewableWorkflow(owner);
 		await ownerAgent.post(`/workflows/${unrelated.workflow.id}/archive`).expect(200);
 
 		const closed = await requestRepository.findById(request.id, {});
@@ -246,7 +218,7 @@ describe('auto-close on workflow archive', () => {
 	});
 
 	test('unarchiving does not reopen the review, and the workflow is no longer publish-blocked', async () => {
-		const { workflow, versionId } = await createReviewableWorkflow();
+		const { workflow, versionId } = await createReviewableWorkflow(owner);
 		const request = await createOpenReview(workflow.id, versionId);
 
 		await ownerAgent.post(`/workflows/${workflow.id}/archive`).expect(200);
@@ -258,7 +230,7 @@ describe('auto-close on workflow archive', () => {
 	});
 
 	test('an already-closed (approved) review is untouched by archiving', async () => {
-		const { workflow, versionId } = await createReviewableWorkflow();
+		const { workflow, versionId } = await createReviewableWorkflow(owner);
 		const request = await createOpenReview(workflow.id, versionId, {
 			state: 'closed',
 			decision: 'approved',
@@ -275,7 +247,7 @@ describe('auto-close on workflow archive', () => {
 
 describe('auto-close on workflow transfer', () => {
 	test('moving the workflow to another project records the cause and closes the review', async () => {
-		const { workflow, versionId } = await createReviewableWorkflow();
+		const { workflow, versionId } = await createReviewableWorkflow(owner);
 		const request = await createOpenReview(workflow.id, versionId);
 		const destination = await createTeamProject('Destination', owner);
 
@@ -303,21 +275,21 @@ describe('auto-close on workflow transfer', () => {
 	// The move stays committed when its close rolls back, leaving the review open on a workflow
 	// that now belongs to another project — the sweep still has the link row to find it by.
 	test('a review stranded open on a moved workflow is closed by the next sweep', async () => {
-		const { workflow, versionId } = await createReviewableWorkflow();
+		const { workflow, versionId } = await createReviewableWorkflow(owner);
 		const request = await createOpenReview(workflow.id, versionId);
 		const destination = await createTeamProject('Destination', owner);
-		vi.spyOn(activityRepository, 'createActivity')
-			.mockRejectedValueOnce(new Error('write failed'))
-			.mockRejectedValueOnce(new Error('write failed'));
 
-		await Container.get(EnterpriseWorkflowService).transferWorkflow(
-			owner,
-			workflow.id,
-			destination.id,
+		await withActivityWritesDown(
+			async () =>
+				await Container.get(EnterpriseWorkflowService).transferWorkflow(
+					owner,
+					workflow.id,
+					destination.id,
+				),
 		);
 		expect((await requestRepository.findById(request.id, {}))?.state).toBe('open');
 
-		const unrelated = await createReviewableWorkflow();
+		const unrelated = await createReviewableWorkflow(owner);
 		await ownerAgent.post(`/workflows/${unrelated.workflow.id}/archive`).expect(200);
 
 		const closed = await requestRepository.findById(request.id, {});
@@ -327,10 +299,10 @@ describe('auto-close on workflow transfer', () => {
 	});
 
 	test('a review whose workflow is still in its project and unarchived is left open', async () => {
-		const { workflow, versionId } = await createReviewableWorkflow();
+		const { workflow, versionId } = await createReviewableWorkflow(owner);
 		const request = await createOpenReview(workflow.id, versionId);
 
-		const unrelated = await createReviewableWorkflow();
+		const unrelated = await createReviewableWorkflow(owner);
 		await ownerAgent.post(`/workflows/${unrelated.workflow.id}/archive`).expect(200);
 
 		expect((await requestRepository.findById(request.id, {}))?.state).toBe('open');
@@ -339,7 +311,7 @@ describe('auto-close on workflow transfer', () => {
 
 describe('auto-close on workflow hard delete', () => {
 	test('force-deleting a non-archived workflow records the cause and closes the review', async () => {
-		const { workflow, versionId } = await createReviewableWorkflow();
+		const { workflow, versionId } = await createReviewableWorkflow(owner);
 		const request = await createOpenReview(workflow.id, versionId);
 
 		await Container.get(WorkflowService).delete(owner, workflow.id, true);
@@ -364,7 +336,7 @@ describe('auto-close on workflow hard delete', () => {
 	// The capture is best-effort: when it fails, the delete still goes through and the
 	// sweep closes the review — without the cause entry, which the cascade made unrecoverable.
 	test('a failed capture degrades to a sweep close without a cause entry', async () => {
-		const { workflow, versionId } = await createReviewableWorkflow({ isArchived: true });
+		const { workflow, versionId } = await createReviewableWorkflow(owner, { isArchived: true });
 		const request = await createOpenReview(workflow.id, versionId);
 		vi.spyOn(lifecycleRepository, 'findOpenRequestsAffectedByWorkflows').mockRejectedValueOnce(
 			new Error('read failed'),
@@ -381,7 +353,7 @@ describe('auto-close on workflow hard delete', () => {
 	// A review opened after the capture ran loses its link row to the cascade and
 	// can no longer be found by workflow id. The post-delete sweep is what catches it.
 	test('a review orphaned by a delete that skipped the hooks is closed by the next delete', async () => {
-		const orphaned = await createReviewableWorkflow();
+		const orphaned = await createReviewableWorkflow(owner);
 		const request = await createOpenReview(orphaned.workflow.id, orphaned.versionId);
 
 		// Delete the row straight from the repository, as a folder-hierarchy cascade does:
@@ -392,7 +364,7 @@ describe('auto-close on workflow hard delete', () => {
 
 		// `unrelated` has no review of its own, so the capture finds nothing to record
 		// and the sweep is the only thing that can explain the orphaned review.
-		const unrelated = await createReviewableWorkflow();
+		const unrelated = await createReviewableWorkflow(owner);
 		await Container.get(WorkflowService).delete(owner, unrelated.workflow.id, true);
 
 		const closed = await requestRepository.findById(request.id, {});
@@ -403,10 +375,10 @@ describe('auto-close on workflow hard delete', () => {
 	});
 
 	test('leaves a review whose workflow still exists open', async () => {
-		const live = await createReviewableWorkflow();
+		const live = await createReviewableWorkflow(owner);
 		const request = await createOpenReview(live.workflow.id, live.versionId);
 
-		const other = await createReviewableWorkflow();
+		const other = await createReviewableWorkflow(owner);
 		await Container.get(WorkflowService).delete(owner, other.workflow.id, true);
 
 		expect((await requestRepository.findById(request.id, {}))?.state).toBe('open');
@@ -415,8 +387,8 @@ describe('auto-close on workflow hard delete', () => {
 
 describe('close policy with multiple linked workflows', () => {
 	test('stays open while a reviewable workflow remains outside the affected set, then closes', async () => {
-		const first = await createReviewableWorkflow();
-		const second = await createReviewableWorkflow();
+		const first = await createReviewableWorkflow(owner);
+		const second = await createReviewableWorkflow(owner);
 		const request = await createOpenReview(first.workflow.id, first.versionId);
 		await linkRepository.createWorkflowRow(
 			{
@@ -457,8 +429,8 @@ describe('close policy with multiple linked workflows', () => {
 	// A linked workflow without an owner sharing row is a broken row, not a move; the
 	// targeted close must leave it reviewable, exactly as the reconciliation sweep does.
 	test('treats a linked workflow with no owner row as reviewable, matching the sweep', async () => {
-		const first = await createReviewableWorkflow();
-		const second = await createReviewableWorkflow();
+		const first = await createReviewableWorkflow(owner);
+		const second = await createReviewableWorkflow(owner);
 		const request = await createOpenReview(first.workflow.id, first.versionId);
 		await linkRepository.createWorkflowRow(
 			{
@@ -490,7 +462,7 @@ describe('publish recorder', () => {
 		});
 
 	test('records workflow.published into the closed (approved) review pinned to the version', async () => {
-		const { workflow, versionId } = await createReviewableWorkflow();
+		const { workflow, versionId } = await createReviewableWorkflow(owner);
 		const request = await approvedReview(workflow.id, versionId);
 
 		await ownerAgent.post(`/workflows/${workflow.id}/activate`).send({ versionId }).expect(200);
@@ -504,7 +476,7 @@ describe('publish recorder', () => {
 		vi.spyOn(Container.get(WorkflowPublicationNotifier), 'requestDrain').mockReturnValue();
 		globalConfig.workflows.useWorkflowPublicationService = true;
 		try {
-			const { workflow, versionId } = await createReviewableWorkflow();
+			const { workflow, versionId } = await createReviewableWorkflow(owner);
 			const request = await approvedReview(workflow.id, versionId);
 
 			await ownerAgent.post(`/workflows/${workflow.id}/activate`).send({ versionId }).expect(200);
@@ -518,7 +490,7 @@ describe('publish recorder', () => {
 	});
 
 	test('records into every request pinned to the published version', async () => {
-		const { workflow, versionId } = await createReviewableWorkflow();
+		const { workflow, versionId } = await createReviewableWorkflow(owner);
 		const first = await approvedReview(workflow.id, versionId);
 		const second = await approvedReview(workflow.id, versionId);
 
@@ -530,7 +502,7 @@ describe('publish recorder', () => {
 
 	// It's a timeline, matching publish history: each publication of the version is an event.
 	test('repeated publication of the same version appends repeated entries', async () => {
-		const { workflow, versionId } = await createReviewableWorkflow();
+		const { workflow, versionId } = await createReviewableWorkflow(owner);
 		const request = await approvedReview(workflow.id, versionId);
 
 		await ownerAgent.post(`/workflows/${workflow.id}/activate`).send({ versionId }).expect(200);
@@ -543,7 +515,7 @@ describe('publish recorder', () => {
 	});
 
 	test('records nothing for a review pinned to a different version', async () => {
-		const { workflow, versionId: pinnedVersionId } = await createReviewableWorkflow();
+		const { workflow, versionId: pinnedVersionId } = await createReviewableWorkflow(owner);
 		const request = await approvedReview(workflow.id, pinnedVersionId);
 
 		const otherVersionId = uuid();
@@ -557,7 +529,7 @@ describe('publish recorder', () => {
 	});
 
 	test('records nothing when activation fails before the commit boundary', async () => {
-		const { workflow, versionId } = await createReviewableWorkflow();
+		const { workflow, versionId } = await createReviewableWorkflow(owner);
 		const request = await approvedReview(workflow.id, versionId);
 		activeWorkflowManager.add.mockRejectedValueOnce(new Error('Webhook path already taken'));
 
@@ -569,7 +541,7 @@ describe('publish recorder', () => {
 	// The rename and the final re-fetch run after the boundary; their failures fail the API
 	// call, but the publication is committed and its record must survive with it.
 	test('the entry persists when post-boundary work throws', async () => {
-		const { workflow, versionId } = await createReviewableWorkflow();
+		const { workflow, versionId } = await createReviewableWorkflow(owner);
 		const request = await approvedReview(workflow.id, versionId);
 		vi.spyOn(Container.get(WorkflowHistoryService), 'updateVersion').mockRejectedValueOnce(
 			new Error('rename failed'),
@@ -591,7 +563,7 @@ describe('publish recorder', () => {
 
 	// The publication must never fail because its feed entry could not be written.
 	test('a failed recorder write does not fail the publish', async () => {
-		const { workflow, versionId } = await createReviewableWorkflow();
+		const { workflow, versionId } = await createReviewableWorkflow(owner);
 		const request = await approvedReview(workflow.id, versionId);
 		vi.spyOn(activityRepository, 'createActivity').mockRejectedValueOnce(new Error('db down'));
 
@@ -607,7 +579,7 @@ describe('publish recorder', () => {
 
 describe('auto-close with the instance policy disabled', () => {
 	test('cleanup still runs when the policy toggle is off', async () => {
-		const { workflow, versionId } = await createReviewableWorkflow();
+		const { workflow, versionId } = await createReviewableWorkflow(owner);
 		const request = await createOpenReview(workflow.id, versionId);
 
 		await Container.get(WorkflowReviewPolicyService).set(false);
@@ -698,7 +670,7 @@ describe('auto-close on source-control pull', () => {
 	});
 
 	test('a pull that archives the workflow closes the open review, decision unchanged', async () => {
-		const { workflow, versionId } = await createReviewableWorkflow();
+		const { workflow, versionId } = await createReviewableWorkflow(owner);
 		const request = await createOpenReview(workflow.id, versionId, {
 			decision: 'changes_requested',
 		});
@@ -728,7 +700,7 @@ describe('auto-close on source-control pull', () => {
 	});
 
 	test('an already-approved (closed) review is untouched by a pull-archive', async () => {
-		const { workflow, versionId } = await createReviewableWorkflow();
+		const { workflow, versionId } = await createReviewableWorkflow(owner);
 		const request = await createOpenReview(workflow.id, versionId, {
 			state: 'closed',
 			decision: 'approved',
@@ -744,7 +716,7 @@ describe('auto-close on source-control pull', () => {
 	});
 
 	test('a pull that updates the workflow without archiving leaves the review open', async () => {
-		const { workflow, versionId } = await createReviewableWorkflow();
+		const { workflow, versionId } = await createReviewableWorkflow(owner);
 		const request = await createOpenReview(workflow.id, versionId);
 
 		const candidate = putWorkflowFile(remoteWorkflow(workflow.id, { name: 'Updated by pull' }));
@@ -759,7 +731,7 @@ describe('auto-close on source-control pull', () => {
 
 	test('a pull that deletes a folder closes reviews of cascade-deleted workflows', async () => {
 		const folder = await createFolder(ownerProject, { name: 'Deleted remotely' });
-		const { workflow, versionId } = await createReviewableWorkflow();
+		const { workflow, versionId } = await createReviewableWorkflow(owner);
 		await Container.get(WorkflowRepository).save({ id: workflow.id, parentFolder: folder });
 		const request = await createOpenReview(workflow.id, versionId);
 

packages/cli/src/modules/workflow-reviews.ee/__tests__/workflow-review-lifecycle.service.test.ts

@@ -64,7 +64,7 @@ describe('WorkflowReviewLifecycleService', () => {
 		collaborationService.broadcastWorkflowReviewStateChanged.mockResolvedValue(undefined);
 	});
 
-	describe('archive', () => {
+	describe('when a workflow is archived', () => {
 		it('records the cause entry and the close entry together, in the lock transaction', async () => {
 			const request = openRequest();
 			lifecycleRepository.findOpenRequestsAffectedByWorkflows.mockResolvedValue([
@@ -201,7 +201,7 @@ describe('WorkflowReviewLifecycleService', () => {
 		});
 	});
 
-	describe('transfer', () => {
+	describe('when workflows move to another project', () => {
 		it('records workflow.moved for each open request and broadcasts once per affected workflow', async () => {
 			const first = openRequest({ id: 'req-1' });
 			const second = openRequest({ id: 'req-2' });
@@ -260,7 +260,7 @@ describe('WorkflowReviewLifecycleService', () => {
 		});
 	});
 
-	describe('delete', () => {
+	describe('when a workflow is deleted', () => {
 		it('captures before the delete without writing anything', async () => {
 			lifecycleRepository.findOpenRequestsAffectedByWorkflows.mockResolvedValue([
 				{
@@ -428,7 +428,7 @@ describe('WorkflowReviewLifecycleService', () => {
 		});
 	});
 
-	describe('publish recorder', () => {
+	describe('when a workflow is published', () => {
 		it('appends workflow.published to every request pinned to the published version', async () => {
 			requestWorkflowRepository.findRequestIdsPinnedToVersion.mockResolvedValue(['req-1', 'req-2']);
 
@@ -489,7 +489,7 @@ describe('WorkflowReviewLifecycleService', () => {
 		});
 	});
 
-	describe('reconciliation sweep', () => {
+	describe('the sweep that catches what the targeted close missed', () => {
 		it('closes the requests the mutation stranded and explains each of them', async () => {
 			lifecycleRepository.findOpenRequestsAffectedByWorkflows.mockResolvedValue([]);
 			// Reconciliation finds req-9 and req-10.
@@ -589,23 +589,4 @@ describe('WorkflowReviewLifecycleService', () => {
 			expect(lifecycleRepository.findUnreviewableOpenRequestIds).not.toHaveBeenCalled();
 		});
 	});
-
-	it('a failed broadcast is only warned about, never thrown', async () => {
-		lifecycleRepository.findOpenRequestsAffectedByWorkflows.mockResolvedValue([
-			{
-				request: openRequest(),
-				links: [{ workflowId: 'wf-1', workflowVersionId: 'wfv-1' }],
-			},
-		]);
-		collaborationService.broadcastWorkflowReviewStateChanged.mockRejectedValue(
-			new Error('push down'),
-		);
-
-		await expect(service.afterWorkflowArchived('wf-1', 'user-9')).resolves.toBeUndefined();
-
-		// Wait for the rejected notification to be logged.
-		await new Promise(process.nextTick);
-		expect(logger.warn).toHaveBeenCalled();
-		expect(logger.error).not.toHaveBeenCalled();
-	});
 });

packages/cli/src/modules/workflow-reviews.ee/__tests__/workflow-review-publish-guard.integration.test.ts

@@ -0,0 +1,343 @@
+import { createTeamProject, createWorkflow, mockInstance, testDb } from '@n8n/backend-test-utils';
+import type { Project, User } from '@n8n/db';
+import { WorkflowRepository, WorkflowReviewRequestRepository } from '@n8n/db';
+import { Container } from '@n8n/di';
+import { v4 as uuid } from 'uuid';
+
+import { ActiveWorkflowManager } from '@/active-workflow-manager';
+import { WorkflowReviewPolicyService } from '@/services/workflow-review-policy.service';
+import { WorkflowValidationService } from '@/workflows/workflow-validation.service';
+import { WorkflowService } from '@/workflows/workflow.service';
+import { EnterpriseWorkflowService } from '@/workflows/workflow.service.ee';
+import { createMember } from '@test-integration/db/users';
+import { createWorkflowHistoryItem } from '@test-integration/db/workflow-history';
+import type { SuperAgentTest } from '@test-integration/types';
+import * as utils from '@test-integration/utils';
+
+import {
+	createReviewableWorkflow,
+	REVIEW_TABLES,
+	seedReview,
+	seedReviewActors,
+	stubWorkflowValidation,
+} from './support/workflow-review-test-data';
+
+const activeWorkflowManager = mockInstance(ActiveWorkflowManager);
+const workflowValidationService = mockInstance(WorkflowValidationService);
+
+const testServer = utils.setupTestServer({
+	endpointGroups: ['workflow-reviews', 'workflows'],
+	enabledFeatures: ['feat:workflowReviews'],
+	modules: ['workflow-reviews'],
+});
+
+let owner: User;
+let member: User;
+let ownerProject: Project;
+let teamProject: Project;
+let ownerAgent: SuperAgentTest;
+
+let requestRepository: WorkflowReviewRequestRepository;
+let workflowEntityRepository: WorkflowRepository;
+let policyService: WorkflowReviewPolicyService;
+
+beforeAll(async () => {
+	await utils.initNodeTypes();
+	requestRepository = Container.get(WorkflowReviewRequestRepository);
+	workflowEntityRepository = Container.get(WorkflowRepository);
+	policyService = Container.get(WorkflowReviewPolicyService);
+});
+
+beforeEach(async () => {
+	testServer.license.enable('feat:workflowReviews');
+	await testDb.truncate([...REVIEW_TABLES]);
+	await policyService.set(true);
+	stubWorkflowValidation(workflowValidationService);
+
+	({ owner, member, ownerProject, teamProject, ownerAgent } = await seedReviewActors(
+		testServer.authAgentFor,
+	));
+});
+
+/** An open review on a workflow `owner` owns personally. */
+async function createOpenReview(
+	workflowId: string,
+	versionId: string,
+	decision: 'pending' | 'changes_requested' = 'pending',
+) {
+	return await seedReview({
+		projectId: ownerProject.id,
+		workflowId,
+		versionId,
+		author: owner,
+		decision,
+	});
+}
+
+const publishedVersionOf = async (workflowId: string) =>
+	(await workflowEntityRepository.findOneByOrFail({ id: workflowId })).activeVersionId;
+
+describe('publishing a workflow under review', () => {
+	test.each([
+		['waiting for a decision', 'pending', 'review_pending'],
+		['waiting for requested changes', 'changes_requested', 'changes_requested'],
+	] as const)(
+		'blocks publication while the review is %s',
+		async (_reviewState, decision, expectedReason) => {
+			const { workflow, versionId } = await createReviewableWorkflow(owner);
+			const request = await createOpenReview(workflow.id, versionId, decision);
+
+			const response = await ownerAgent
+				.post(`/workflows/${workflow.id}/activate`)
+				.send({ versionId })
+				.expect(409);
+
+			expect(response.body.meta).toEqual({
+				reason: expectedReason,
+				workflowReviewRequestId: request.id,
+				validationError: true,
+			});
+			expect(await publishedVersionOf(workflow.id)).toBeNull();
+		},
+	);
+
+	test('publishes the pinned version automatically when the review is approved', async () => {
+		const { workflow, versionId } = await createReviewableWorkflow(owner);
+		const request = await createOpenReview(workflow.id, versionId);
+
+		const response = await ownerAgent
+			.post(`/workflow-review-requests/${request.id}/decision`)
+			.send({ decision: 'approved' })
+			.expect(200);
+
+		expect(response.body.data.autoPublish).toEqual({ status: 'published' });
+		expect(await publishedVersionOf(workflow.id)).toBe(versionId);
+
+		// The timeline completes: the approval that closed the review, then its publication.
+		const activity = await ownerAgent
+			.get(`/workflow-review-requests/${request.id}/activity`)
+			.expect(200);
+		expect((activity.body.data.data as Array<{ type: string }>).map((entry) => entry.type)).toEqual(
+			['review.approved', 'workflow.published'],
+		);
+	});
+
+	test('keeps the approval and allows manual publish when auto-publish fails', async () => {
+		const { workflow, versionId } = await createReviewableWorkflow(owner);
+		const request = await createOpenReview(workflow.id, versionId);
+
+		// Activation fails once — after the approval has already committed.
+		workflowValidationService.validateForActivation.mockReturnValueOnce({
+			isValid: false,
+			error: 'The workflow has issues',
+		});
+
+		const response = await ownerAgent
+			.post(`/workflow-review-requests/${request.id}/decision`)
+			.send({ decision: 'approved' })
+			.expect(200);
+
+		expect(response.body.data).toMatchObject({
+			state: 'closed',
+			decision: 'approved',
+			autoPublish: { status: 'failed', message: 'The workflow has issues' },
+		});
+		expect(await publishedVersionOf(workflow.id)).toBeNull();
+
+		// Retry path: the review is closed, so the regular publish flow is unblocked.
+		await ownerAgent.post(`/workflows/${workflow.id}/activate`).send({ versionId }).expect(200);
+
+		expect(await publishedVersionOf(workflow.id)).toBe(versionId);
+	});
+
+	test('leaves an already published workflow unpublished when the approval publish fails at registration', async () => {
+		const { workflow, versionId: firstVersionId } = await createReviewableWorkflow(owner);
+
+		// Publish once, so the approval below replaces a live version.
+		await ownerAgent
+			.post(`/workflows/${workflow.id}/activate`)
+			.send({ versionId: firstVersionId })
+			.expect(200);
+
+		const secondVersionId = uuid();
+		await createWorkflowHistoryItem(workflow.id, { versionId: secondVersionId });
+		const request = await createOpenReview(workflow.id, secondVersionId);
+
+		// Fails at trigger registration — after the live version was removed, so
+		// activation rolls the row back to unpublished rather than restoring it.
+		activeWorkflowManager.add.mockRejectedValueOnce(new Error('Webhook path already taken'));
+
+		const response = await ownerAgent
+			.post(`/workflow-review-requests/${request.id}/decision`)
+			.send({ decision: 'approved' })
+			.expect(200);
+
+		expect(response.body.data).toMatchObject({
+			state: 'closed',
+			decision: 'approved',
+			autoPublish: { status: 'failed', message: 'Webhook path already taken' },
+		});
+
+		// The workflow that was live before the approval is now unpublished — which
+		// is why the copy says so and the failure is logged at error level.
+		const updated = await workflowEntityRepository.findOneByOrFail({ id: workflow.id });
+		expect(updated.activeVersionId).toBeNull();
+		expect(updated.active).toBe(false);
+	});
+
+	test.each(['policy disabled', 'license unavailable'] as const)(
+		'allows publication when workflow reviews are %s',
+		async (unavailableReason) => {
+			const { workflow, versionId } = await createReviewableWorkflow(owner);
+			await createOpenReview(workflow.id, versionId);
+
+			if (unavailableReason === 'policy disabled') {
+				await policyService.set(false);
+			} else {
+				testServer.license.disable('feat:workflowReviews');
+			}
+
+			await ownerAgent.post(`/workflows/${workflow.id}/activate`).send({ versionId }).expect(200);
+
+			expect(await publishedVersionOf(workflow.id)).toBe(versionId);
+		},
+	);
+
+	test('allows unpublishing while a review is open', async () => {
+		const { workflow, versionId } = await createReviewableWorkflow(owner);
+
+		await ownerAgent.post(`/workflows/${workflow.id}/activate`).send({ versionId }).expect(200);
+		await createOpenReview(workflow.id, versionId);
+
+		await ownerAgent.post(`/workflows/${workflow.id}/deactivate`).send({}).expect(200);
+
+		expect(await publishedVersionOf(workflow.id)).toBeNull();
+	});
+});
+
+/**
+ * The approval commits under the review lock, but auto-publish runs after it is
+ * released. Workflow mutations are deliberately not serialized behind that lock,
+ * so they can land in the gap. These pin the accepted outcomes: the approval
+ * stands and the publish that lost the race is reported as a failure.
+ */
+describe('a workflow mutation racing the auto-publish of an approval', () => {
+	/** Run `raceAction` in the gap between the committed approval and auto-publish. */
+	function raceBeforeAutoPublish(raceAction: () => Promise<unknown>) {
+		const workflowService = Container.get(WorkflowService);
+		const activate = workflowService.activateWorkflow.bind(workflowService);
+		vi.spyOn(workflowService, 'activateWorkflow').mockImplementationOnce(async (...args) => {
+			await raceAction();
+			return await activate(...args);
+		});
+	}
+
+	test('an archive that lands in the gap leaves an approved review and a failed auto-publish', async () => {
+		const { workflow, versionId } = await createReviewableWorkflow(owner);
+		const request = await createOpenReview(workflow.id, versionId);
+
+		raceBeforeAutoPublish(
+			async () => await ownerAgent.post(`/workflows/${workflow.id}/archive`).expect(200),
+		);
+
+		const response = await ownerAgent
+			.post(`/workflow-review-requests/${request.id}/decision`)
+			.send({ decision: 'approved' })
+			.expect(200);
+
+		expect(response.body.data).toMatchObject({
+			state: 'closed',
+			decision: 'approved',
+			autoPublish: { status: 'failed', message: 'Cannot activate an archived workflow.' },
+		});
+
+		// The archive found the review already closed, so it left the approval alone.
+		const closed = await requestRepository.findById(request.id, {});
+		expect(closed).toMatchObject({ state: 'closed', decision: 'approved' });
+
+		const archived = await workflowEntityRepository.findOneByOrFail({ id: workflow.id });
+		expect(archived.isArchived).toBe(true);
+		expect(archived.activeVersionId).toBeNull();
+
+		// Publishing stays blocked by the archival itself, not by the closed review.
+		await ownerAgent.post(`/workflows/${workflow.id}/activate`).send({ versionId }).expect(400);
+	});
+
+	test('a transfer that lands in the gap leaves an approved review and a failed auto-publish', async () => {
+		const versionId = uuid();
+		const workflow = await createWorkflow({}, teamProject);
+		await createWorkflowHistoryItem(workflow.id, { versionId });
+
+		// The requester publishes on approval, so it must be someone who loses access
+		// when the workflow moves — the deciding owner never does.
+		const request = await seedReview({
+			projectId: teamProject.id,
+			workflowId: workflow.id,
+			versionId,
+			author: member,
+		});
+
+		const destination = await createTeamProject('Elsewhere', await createMember());
+		raceBeforeAutoPublish(
+			async () =>
+				await Container.get(EnterpriseWorkflowService).transferWorkflow(
+					owner,
+					workflow.id,
+					destination.id,
+				),
+		);
+
+		const response = await ownerAgent
+			.post(`/workflow-review-requests/${request.id}/decision`)
+			.send({ decision: 'approved' })
+			.expect(200);
+
+		expect(response.body.data).toMatchObject({
+			state: 'closed',
+			decision: 'approved',
+			autoPublish: {
+				status: 'failed',
+				message:
+					'You do not have permission to activate this workflow. Ask the owner to share it with you.',
+			},
+		});
+
+		expect(await requestRepository.findById(request.id, {})).toMatchObject({
+			state: 'closed',
+			decision: 'approved',
+		});
+		expect(await publishedVersionOf(workflow.id)).toBeNull();
+	});
+
+	test('a review created in the gap blocks the auto-publish without failing the decision', async () => {
+		const { workflow, versionId } = await createReviewableWorkflow(owner);
+		const request = await createOpenReview(workflow.id, versionId);
+
+		let racingRequestId = '';
+		raceBeforeAutoPublish(async () => {
+			racingRequestId = (await createOpenReview(workflow.id, versionId)).id;
+		});
+
+		const response = await ownerAgent
+			.post(`/workflow-review-requests/${request.id}/decision`)
+			.send({ decision: 'approved' })
+			.expect(200);
+
+		expect(response.body.data).toMatchObject({
+			state: 'closed',
+			decision: 'approved',
+			autoPublish: {
+				status: 'failed',
+				message:
+					"Workflow can't be published while its review is open. Submit this version to the review, or wait for the review to close.",
+			},
+		});
+
+		// The new review is untouched and still guards the workflow.
+		expect(await requestRepository.findOpenRequestForWorkflow(workflow.id, {})).toMatchObject({
+			id: racingRequestId,
+			state: 'open',
+		});
+		expect(await publishedVersionOf(workflow.id)).toBeNull();
+	});
+});

packages/cli/src/modules/workflow-reviews.ee/__tests__/workflow-review-request-decision.service.test.ts

@@ -161,7 +161,7 @@ describe('WorkflowReviewRequestDecisionService.decide', () => {
 		collaborationService.broadcastWorkflowUpdate.mockResolvedValue(undefined);
 	});
 
-	it('throws when the instance policy is disabled, before any lookup or lock', async () => {
+	it('refuses everything once an admin turns reviews off, before any lookup or lock', async () => {
 		workflowReviewPolicyService.get.mockResolvedValue({ enabled: false });
 
 		await expect(service.decide(memberUser(), requestId, approveDto)).rejects.toThrow(
@@ -172,7 +172,7 @@ describe('WorkflowReviewRequestDecisionService.decide', () => {
 		expect(dbLockService.withLockContext).not.toHaveBeenCalled();
 	});
 
-	it('throws NotFoundError when the review request does not exist', async () => {
+	it('refuses a review that does not exist', async () => {
 		requestRepository.findById.mockResolvedValue(null);
 
 		await expect(service.decide(memberUser(), requestId, approveDto)).rejects.toThrow(
@@ -182,7 +182,7 @@ describe('WorkflowReviewRequestDecisionService.decide', () => {
 		expect(dbLockService.withLockContext).not.toHaveBeenCalled();
 	});
 
-	it('throws NotFoundError when the request has no linked workflow row', async () => {
+	it('refuses a review that covers no workflow', async () => {
 		requestRepository.findById.mockResolvedValue(openRequest());
 		workflowRepository.findByRequestId.mockResolvedValue([]);
 
@@ -194,7 +194,7 @@ describe('WorkflowReviewRequestDecisionService.decide', () => {
 		expect(dbLockService.withLockContext).not.toHaveBeenCalled();
 	});
 
-	it('throws NotFoundError when the user cannot view the workflow', async () => {
+	it('hides a review whose workflow the caller cannot view', async () => {
 		mockSuccessfulDecidePath();
 		workflowFinderService.findWorkflowForUser.mockResolvedValue(null);
 
@@ -210,7 +210,7 @@ describe('WorkflowReviewRequestDecisionService.decide', () => {
 		expect(dbLockService.withLockContext).not.toHaveBeenCalled();
 	});
 
-	it('throws NotFoundError when the user cannot view every workflow the request covers', async () => {
+	it('hides a review when the caller cannot view every workflow it covers', async () => {
 		mockSuccessfulDecidePath();
 		workflowRepository.findByRequestId.mockResolvedValue([
 			pinnedRow('ver-1', 'wf-1'),
@@ -227,7 +227,7 @@ describe('WorkflowReviewRequestDecisionService.decide', () => {
 		expect(dbLockService.withLockContext).not.toHaveBeenCalled();
 	});
 
-	it('throws NotFoundError for a non-assigned viewer without an admin override', async () => {
+	it('hides a review from someone who was not asked to review it', async () => {
 		mockSuccessfulDecidePath();
 		reviewerRepository.isReviewer.mockResolvedValue(false);
 
@@ -241,7 +241,7 @@ describe('WorkflowReviewRequestDecisionService.decide', () => {
 	it.each([
 		['closed', openRequest({ state: 'closed' })],
 		['approved', openRequest({ decision: 'approved' })],
-	])('throws ConflictError and never takes the lock when the request is %s', async (_name, req) => {
+	])('refuses a review that is already %s, before taking the lock', async (_name, req) => {
 		mockSuccessfulDecidePath();
 		requestRepository.findById.mockResolvedValue(req);
 
@@ -252,19 +252,8 @@ describe('WorkflowReviewRequestDecisionService.decide', () => {
 		expect(dbLockService.withLockContext).not.toHaveBeenCalled();
 	});
 
-	describe('author eligibility', () => {
-		it('allows an assigned reviewer to decide even when they authored a version', async () => {
-			mockSuccessfulDecidePath();
-			authorRepository.isAuthor.mockResolvedValue(true);
-			reviewerRepository.isReviewer.mockResolvedValue(true);
-			projectRelationRepository.getAccessibleProjectsByRoles.mockResolvedValue([]);
-
-			const result = await service.decide(memberUser(), requestId, approveDto);
-
-			expect(result.decision).toBe('approved');
-		});
-
-		it('throws ForbiddenError for a non-assigned author without an admin override', async () => {
+	describe('who is refused, and how', () => {
+		it('tells an author outright that they cannot decide their own review', async () => {
 			mockSuccessfulDecidePath();
 			authorRepository.isAuthor.mockResolvedValue(true);
 			reviewerRepository.isReviewer.mockResolvedValue(false);
@@ -278,7 +267,7 @@ describe('WorkflowReviewRequestDecisionService.decide', () => {
 		});
 
 		// Check authorization before revealing that the note is invalid.
-		it('tells an author they may not decide even when their note is missing too', async () => {
+		it('refuses an author before complaining about their missing note', async () => {
 			mockSuccessfulDecidePath();
 			authorRepository.isAuthor.mockResolvedValue(true);
 			reviewerRepository.isReviewer.mockResolvedValue(false);
@@ -289,7 +278,7 @@ describe('WorkflowReviewRequestDecisionService.decide', () => {
 			).rejects.toThrow(ForbiddenError);
 		});
 
-		it('still allows an assigned reviewer who became an author while waiting for the lock', async () => {
+		it('still lets an assigned reviewer decide after a re-pin made them an author too', async () => {
 			mockSuccessfulDecidePath();
 			// A version update adds the caller as an author before the decision gets the lock.
 			authorRepository.isAuthor.mockResolvedValueOnce(false).mockResolvedValueOnce(true);
@@ -311,7 +300,7 @@ describe('WorkflowReviewRequestDecisionService.decide', () => {
 			);
 		});
 
-		it('rejects a caller unassigned while waiting for the lock', async () => {
+		it('refuses someone unassigned while they waited for the lock', async () => {
 			mockSuccessfulDecidePath();
 			reviewerRepository.isReviewer.mockResolvedValueOnce(true).mockResolvedValueOnce(false);
 			authorRepository.isAuthor.mockResolvedValue(false);
@@ -323,22 +312,7 @@ describe('WorkflowReviewRequestDecisionService.decide', () => {
 			expect(requestRepository.saveRequest).not.toHaveBeenCalled();
 		});
 
-		it.each([['global:admin'], ['global:owner']])(
-			'allows an author with the %s role without querying project relations',
-			async (slug) => {
-				mockSuccessfulDecidePath();
-				authorRepository.isAuthor.mockResolvedValue(true);
-				reviewerRepository.isReviewer.mockResolvedValue(false);
-				const admin = mock<User>({ id: 'user-1', role: { slug } });
-
-				const result = await service.decide(admin, requestId, approveDto);
-
-				expect(result.decision).toBe('approved');
-				expect(projectRelationRepository.getAccessibleProjectsByRoles).not.toHaveBeenCalled();
-			},
-		);
-
-		it('allows an author who is a project admin of the review project', async () => {
+		it('lets a project admin decide their own review, recorded as an override', async () => {
 			mockSuccessfulDecidePath();
 			authorRepository.isAuthor.mockResolvedValue(true);
 			reviewerRepository.isReviewer.mockResolvedValue(false);
@@ -357,19 +331,6 @@ describe('WorkflowReviewRequestDecisionService.decide', () => {
 			);
 		});
 
-		it('throws ForbiddenError for an author who is only a project admin elsewhere', async () => {
-			mockSuccessfulDecidePath();
-			authorRepository.isAuthor.mockResolvedValue(true);
-			reviewerRepository.isReviewer.mockResolvedValue(false);
-			projectRelationRepository.getAccessibleProjectsByRoles.mockResolvedValue(['other-proj']);
-
-			await expect(service.decide(memberUser(), requestId, approveDto)).rejects.toThrow(
-				ForbiddenError,
-			);
-
-			expect(dbLockService.withLockContext).not.toHaveBeenCalled();
-		});
-
 		it('resolves the admin override once, before taking the lock', async () => {
 			mockSuccessfulDecidePath();
 
@@ -384,7 +345,7 @@ describe('WorkflowReviewRequestDecisionService.decide', () => {
 		});
 	});
 
-	it('approves: closes the request, stamps closedById and approvedAt, and broadcasts', async () => {
+	it('closes the review on approval, recording who approved it and when, and tells open editors', async () => {
 		const request = mockSuccessfulDecidePath();
 
 		const result = await service.decide(memberUser(), requestId, approveDto);
@@ -429,7 +390,7 @@ describe('WorkflowReviewRequestDecisionService.decide', () => {
 	});
 
 	// Use several rows to cover the approval baseline loop.
-	it('approves: freezes a baseline for every workflow the request covers', async () => {
+	it('freezes a comparison baseline for every workflow the review covers', async () => {
 		mockSuccessfulDecidePath();
 		workflowRepository.findByRequestId.mockResolvedValue([
 			pinnedRow('ver-1', 'wf-1'),
@@ -449,7 +410,7 @@ describe('WorkflowReviewRequestDecisionService.decide', () => {
 		);
 	});
 
-	it('requests changes: keeps the request open and leaves closedById/approvedAt untouched', async () => {
+	it('leaves the review open and unstamped when a reviewer asks for changes', async () => {
 		const request = mockSuccessfulDecidePath();
 
 		const result = await service.decide(memberUser(), requestId, requestChangesDto);
@@ -477,7 +438,7 @@ describe('WorkflowReviewRequestDecisionService.decide', () => {
 		});
 	});
 
-	it('allows repeating changes_requested (e.g. a second reviewer)', async () => {
+	it('lets a second reviewer ask for changes again', async () => {
 		mockSuccessfulDecidePath();
 		requestRepository.findById.mockResolvedValue(openRequest({ decision: 'changes_requested' }));
 
@@ -488,7 +449,7 @@ describe('WorkflowReviewRequestDecisionService.decide', () => {
 		expect(savedEntity).toMatchObject({ updatedById: 'user-2' });
 	});
 
-	it('allows approving a changes_requested review', async () => {
+	it('lets a reviewer approve a review that had changes requested', async () => {
 		mockSuccessfulDecidePath();
 		requestRepository.findById.mockResolvedValue(openRequest({ decision: 'changes_requested' }));
 
@@ -498,7 +459,7 @@ describe('WorkflowReviewRequestDecisionService.decide', () => {
 		expect(result.state).toBe('closed');
 	});
 
-	it('throws ConflictError and saves/broadcasts nothing when the request closes between check and lock', async () => {
+	it('writes and announces nothing when the review closes while the decision waits for the lock', async () => {
 		mockSuccessfulDecidePath();
 		requestRepository.findById
 			.mockResolvedValueOnce(openRequest())
@@ -531,7 +492,7 @@ describe('WorkflowReviewRequestDecisionService.decide', () => {
 		expect(workflowService.activateWorkflow).not.toHaveBeenCalled();
 	});
 
-	it('reports and publishes the version re-pinned by a concurrent sync that won the lock', async () => {
+	it('reports and publishes the version a concurrent re-pin left behind', async () => {
 		mockSuccessfulDecidePath();
 		workflowRepository.findByRequestId
 			.mockResolvedValueOnce([pinnedRow('ver-1')])
@@ -579,7 +540,7 @@ describe('WorkflowReviewRequestDecisionService.decide', () => {
 			);
 		});
 
-		it('never publishes on changes_requested and omits the outcome', async () => {
+		it('publishes nothing when a reviewer asks for changes, and reports no outcome', async () => {
 			mockSuccessfulDecidePath();
 
 			const result = await service.decide(memberUser(), requestId, requestChangesDto);
@@ -709,21 +670,4 @@ describe('WorkflowReviewRequestDecisionService.decide', () => {
 			);
 		});
 	});
-
-	it('resolves and logs a warning when the broadcast rejects', async () => {
-		mockSuccessfulDecidePath();
-		collaborationService.broadcastWorkflowReviewStateChanged.mockRejectedValue(
-			new Error('push down'),
-		);
-
-		const result = await service.decide(memberUser(), requestId, approveDto);
-		expect(result.id).toBe(requestId);
-
-		// Wait for the rejected notification to be logged.
-		await new Promise(process.nextTick);
-		expect(logger.warn).toHaveBeenCalledWith(
-			'Failed to broadcast review state change',
-			expect.objectContaining({ workflowId: 'wf-1' }),
-		);
-	});
 });

packages/cli/src/modules/workflow-reviews.ee/__tests__/workflow-review-request-status.service.test.ts

@@ -0,0 +1,217 @@
+import type { ListWorkflowReviewRequestsQueryDto } from '@n8n/api-types';
+import type { LicenseState } from '@n8n/backend-common';
+import { User } from '@n8n/db';
+import type {
+	UserRepository,
+	WorkflowEntity,
+	WorkflowReviewRequestForWorkflowRow,
+	WorkflowReviewRequestRepository,
+} from '@n8n/db';
+import { mock } from 'vitest-mock-extended';
+
+import { ForbiddenError } from '@/errors/response-errors/forbidden.error';
+import { NotFoundError } from '@/errors/response-errors/not-found.error';
+import type { WorkflowReviewPolicyService } from '@/services/workflow-review-policy.service';
+import type { WorkflowFinderService } from '@/workflows/workflow-finder.service';
+
+import type { WorkflowReviewAuthorizationService } from '../workflow-review-authorization.service';
+import { WorkflowReviewFeatureGate } from '../workflow-review-feature-gate.service';
+import { WorkflowReviewRequestStatusService } from '../workflow-review-request-status.service';
+
+const user = mock<User>({ id: 'user-1' });
+
+/** Build a loaded user with the computed pending state. */
+function loadedUser(fields: Partial<User> & { id: string; email: string }): User {
+	const loaded = Object.assign(new User(), { password: 'hashed', authIdentities: [], ...fields });
+	loaded.computeIsPending();
+	return loaded;
+}
+
+describe('WorkflowReviewRequestStatusService.list', () => {
+	const workflowReviewPolicyService = mock<WorkflowReviewPolicyService>();
+	const workflowFinderService = mock<WorkflowFinderService>();
+	const requestRepository = mock<WorkflowReviewRequestRepository>();
+	const userRepository = mock<UserRepository>();
+	const licenseState = mock<LicenseState>();
+	const authorizationService = mock<WorkflowReviewAuthorizationService>();
+
+	const service = new WorkflowReviewRequestStatusService(
+		new WorkflowReviewFeatureGate(licenseState, workflowReviewPolicyService),
+		workflowFinderService,
+		requestRepository,
+		userRepository,
+		authorizationService,
+	);
+
+	const query = mock<ListWorkflowReviewRequestsQueryDto>({
+		workflowId: 'wf-1',
+		skip: 0,
+		take: 1,
+	});
+
+	const reviewRow = (
+		overrides: Partial<WorkflowReviewRequestForWorkflowRow> = {},
+	): WorkflowReviewRequestForWorkflowRow => ({
+		id: 'req-1',
+		projectId: 'proj-1',
+		state: 'open',
+		decision: 'pending',
+		description: null,
+		updatedById: 'user-2',
+		workflowVersionId: 'ver-1',
+		workflowVersionName: null,
+		createdAt: new Date('2024-01-01T00:00:00.000Z'),
+		updatedAt: new Date('2024-01-02T00:00:00.000Z'),
+		...overrides,
+	});
+
+	const mockLatestReview = (overrides: Partial<WorkflowReviewRequestForWorkflowRow> = {}) => {
+		workflowFinderService.findWorkflowForUser.mockResolvedValue(mock<WorkflowEntity>());
+		requestRepository.findRequestsForWorkflow.mockResolvedValue([[reviewRow(overrides)], 1]);
+	};
+
+	const reviewer = loadedUser({
+		id: 'user-2',
+		email: 'reviewer@example.com',
+		firstName: 'Rey',
+		lastName: 'Viewer',
+	});
+
+	beforeEach(() => {
+		vi.resetAllMocks();
+		authorizationService.resolveOpenableRequestIds.mockResolvedValue(new Set());
+		licenseState.isWorkflowReviewsLicensed.mockReturnValue(true);
+		workflowReviewPolicyService.get.mockResolvedValue({ enabled: true });
+	});
+
+	it('refuses to list reviews for a workflow the caller cannot read', async () => {
+		workflowFinderService.findWorkflowForUser.mockResolvedValue(null);
+
+		await expect(service.list(user, query)).rejects.toThrow(NotFoundError);
+
+		expect(workflowFinderService.findWorkflowForUser).toHaveBeenCalledWith('wf-1', user, [
+			'workflow:read',
+		]);
+		expect(requestRepository.findRequestsForWorkflow).not.toHaveBeenCalled();
+	});
+
+	it('refuses to list anything once an admin turns reviews off, before looking the workflow up', async () => {
+		workflowReviewPolicyService.get.mockResolvedValue({ enabled: false });
+
+		await expect(service.list(user, query)).rejects.toThrow(ForbiddenError);
+
+		expect(workflowFinderService.findWorkflowForUser).not.toHaveBeenCalled();
+		expect(requestRepository.findRequestsForWorkflow).not.toHaveBeenCalled();
+	});
+
+	it('names who asked for changes', async () => {
+		mockLatestReview({ decision: 'changes_requested' });
+		userRepository.findManyByIds.mockResolvedValue([reviewer]);
+
+		const { count, data } = await service.list(user, query);
+
+		expect(userRepository.findManyByIds).toHaveBeenCalledWith(['user-2']);
+		expect(count).toBe(1);
+		expect(data).toEqual([
+			{
+				id: 'req-1',
+				state: 'open',
+				decision: 'changes_requested',
+				description: null,
+				workflowVersionId: 'ver-1',
+				workflowVersionName: null,
+				createdAt: '2024-01-01T00:00:00.000Z',
+				updatedAt: '2024-01-02T00:00:00.000Z',
+				decisionBy: {
+					id: 'user-2',
+					email: 'reviewer@example.com',
+					firstName: 'Rey',
+					lastName: 'Viewer',
+				},
+				viewerCanOpen: false,
+			},
+		]);
+	});
+
+	it('carries the name given to the version under review', async () => {
+		mockLatestReview({ workflowVersionName: 'Release candidate' });
+
+		const { data } = await service.list(user, query);
+
+		expect(data[0]).toMatchObject({ workflowVersionName: 'Release candidate' });
+	});
+
+	it('names nobody once the user who asked for changes is deleted', async () => {
+		mockLatestReview({ decision: 'changes_requested' });
+		userRepository.findManyByIds.mockResolvedValue([]);
+
+		const { data } = await service.list(user, query);
+
+		expect(data[0]?.decisionBy).toBeNull();
+	});
+
+	// Only a changes-requested review names anyone: an approval is deliberately
+	// unattributed in the canvas banner, and a pending review has no decider yet.
+	it.each([
+		['the review is still waiting for a decision', {}],
+		['the review records no actor', { decision: 'changes_requested' as const, updatedById: null }],
+		['the review was approved', { state: 'closed' as const, decision: 'approved' as const }],
+	])('names nobody when %s', async (_label, overrides) => {
+		mockLatestReview(overrides);
+
+		const { data } = await service.list(user, query);
+
+		expect(data[0]).toMatchObject({ decisionBy: null });
+		expect(userRepository.findManyByIds).not.toHaveBeenCalled();
+	});
+
+	it('marks the rows the caller may open, resolved in one batched access check', async () => {
+		mockLatestReview();
+		authorizationService.resolveOpenableRequestIds.mockResolvedValue(new Set(['req-1']));
+
+		const { data } = await service.list(user, query);
+
+		expect(authorizationService.resolveOpenableRequestIds).toHaveBeenCalledWith(user, [
+			expect.objectContaining({ id: 'req-1', projectId: 'proj-1' }),
+		]);
+		expect(data[0]?.viewerCanOpen).toBe(true);
+	});
+
+	it('names the deciders of many rows with a single user lookup', async () => {
+		workflowFinderService.findWorkflowForUser.mockResolvedValue(mock<WorkflowEntity>());
+		requestRepository.findRequestsForWorkflow.mockResolvedValue([
+			[
+				reviewRow({ id: 'req-1', decision: 'changes_requested', updatedById: 'user-2' }),
+				reviewRow({ id: 'req-2', decision: 'changes_requested', updatedById: 'user-3' }),
+				reviewRow({
+					id: 'req-3',
+					state: 'closed',
+					decision: 'approved',
+					workflowVersionId: 'ver-3',
+				}),
+				reviewRow({
+					id: 'req-4',
+					state: 'closed',
+					decision: 'approved',
+					workflowVersionId: 'ver-4',
+				}),
+			],
+			4,
+		]);
+		userRepository.findManyByIds.mockResolvedValue([
+			reviewer,
+			loadedUser({ id: 'user-3', email: 'other@example.com' }),
+		]);
+
+		const { data } = await service.list(user, query);
+
+		expect(userRepository.findManyByIds).toHaveBeenCalledTimes(1);
+		expect(userRepository.findManyByIds).toHaveBeenCalledWith(['user-2', 'user-3']);
+		expect(data.map((item) => item.decisionBy?.email ?? null)).toEqual([
+			'reviewer@example.com',
+			'other@example.com',
+			null,
+			null,
+		]);
+	});
+});

packages/cli/src/modules/workflow-reviews.ee/__tests__/workflow-review-request-submission.service.test.ts

@@ -0,0 +1,528 @@
+import type {
+	CreateWorkflowReviewRequestDto,
+	GetWorkflowReviewEligibleReviewersQueryDto,
+} from '@n8n/api-types';
+import type { LicenseState, Logger } from '@n8n/backend-common';
+import { DbLock, User } from '@n8n/db';
+import type {
+	AuthIdentity,
+	DbLockService,
+	OperationContext,
+	Project,
+	SharedWorkflowRepository,
+	Transaction,
+	UserRepository,
+	WorkflowEntity,
+	WorkflowHistoryRepository,
+	WorkflowRepository,
+	WorkflowReviewActivityRepository,
+	WorkflowReviewRequest,
+	WorkflowReviewRequestAuthorRepository,
+	WorkflowReviewRequestRepository,
+	WorkflowReviewRequestReviewerRepository,
+	WorkflowReviewRequestWorkflowRepository,
+} from '@n8n/db';
+import { mock } from 'vitest-mock-extended';
+
+import type { CollaborationService } from '@/collaboration/collaboration.service';
+import { BadRequestError } from '@/errors/response-errors/bad-request.error';
+import { ConflictError } from '@/errors/response-errors/conflict.error';
+import { ForbiddenError } from '@/errors/response-errors/forbidden.error';
+import { NotFoundError } from '@/errors/response-errors/not-found.error';
+import type { EventService } from '@/events/event.service';
+import type { RoleService } from '@/services/role.service';
+import type { WorkflowReviewPolicyService } from '@/services/workflow-review-policy.service';
+import type { WorkflowFinderService } from '@/workflows/workflow-finder.service';
+import type { WorkflowHistoryService } from '@/workflows/workflow-history/workflow-history.service';
+
+import { WorkflowReviewFeatureGate } from '../workflow-review-feature-gate.service';
+import { WorkflowReviewRequestMutationGuard } from '../workflow-review-request-mutation-guard.service';
+import { WorkflowReviewRequestSubmissionService } from '../workflow-review-request-submission.service';
+import { WorkflowReviewStateNotifier } from '../workflow-review-state-notifier.service';
+
+const user = mock<User>({ id: 'user-1' });
+
+/** Build a loaded user with the computed pending state. */
+function loadedUser(fields: Partial<User> & { id: string; email: string }): User {
+	const loaded = Object.assign(new User(), { password: 'hashed', authIdentities: [], ...fields });
+	loaded.computeIsPending();
+	return loaded;
+}
+
+const dto: CreateWorkflowReviewRequestDto = {
+	title: 'Please review',
+	description: 'A description',
+	workflows: [
+		{ workflowId: 'wf-1', workflowVersionId: 'ver-1', workflowVersionName: 'Release candidate' },
+	],
+	reviewerUserIds: ['user-2'],
+};
+
+describe('WorkflowReviewRequestSubmissionService', () => {
+	const workflowReviewPolicyService = mock<WorkflowReviewPolicyService>();
+	const workflowFinderService = mock<WorkflowFinderService>();
+	const workflowHistoryService = mock<WorkflowHistoryService>();
+	const workflowHistoryRepository = mock<WorkflowHistoryRepository>();
+	/** Workflow entity repository; `workflowRepository` stores request links. */
+	const workflowEntityRepository = mock<WorkflowRepository>();
+	const sharedWorkflowRepository = mock<SharedWorkflowRepository>();
+	const requestRepository = mock<WorkflowReviewRequestRepository>();
+	const workflowRepository = mock<WorkflowReviewRequestWorkflowRepository>();
+	const authorRepository = mock<WorkflowReviewRequestAuthorRepository>();
+	const reviewerRepository = mock<WorkflowReviewRequestReviewerRepository>();
+	const activityRepository = mock<WorkflowReviewActivityRepository>();
+	const userRepository = mock<UserRepository>();
+	const roleService = mock<RoleService>();
+	const licenseState = mock<LicenseState>();
+	const dbLockService = mock<DbLockService>();
+	const collaborationService = mock<CollaborationService>();
+	const logger = mock<Logger>();
+	const eventService = mock<EventService>();
+	/** Transaction context used inside the lock. */
+	const ctx: OperationContext = { trx: mock<Transaction>() };
+
+	const service = new WorkflowReviewRequestSubmissionService(
+		new WorkflowReviewFeatureGate(licenseState, workflowReviewPolicyService),
+		workflowFinderService,
+		workflowHistoryService,
+		workflowHistoryRepository,
+		sharedWorkflowRepository,
+		requestRepository,
+		workflowRepository,
+		authorRepository,
+		reviewerRepository,
+		activityRepository,
+		userRepository,
+		roleService,
+		dbLockService,
+		eventService,
+		new WorkflowReviewRequestMutationGuard(workflowEntityRepository, sharedWorkflowRepository),
+		new WorkflowReviewStateNotifier(logger, collaborationService),
+	);
+
+	beforeEach(() => {
+		vi.resetAllMocks();
+		licenseState.isWorkflowReviewsLicensed.mockReturnValue(true);
+		// Enable the feature unless a test overrides it.
+		workflowReviewPolicyService.get.mockResolvedValue({ enabled: true });
+		// Run locked work with the transaction context by default.
+		dbLockService.withLockContext.mockImplementation(async (_id, fn) => await fn(ctx));
+		collaborationService.broadcastWorkflowReviewStateChanged.mockResolvedValue(undefined);
+	});
+
+	describe('opening a review', () => {
+		// Provide the required eligible reviewer by default.
+		beforeEach(() => {
+			roleService.rolesWithScope.mockResolvedValue(['some-role']);
+			userRepository.findEligibleByProjectOrGlobalRoles.mockResolvedValue([
+				loadedUser({ id: 'user-2', email: 'user-2@n8n.io' }),
+			]);
+		});
+
+		/** Everything resolved so `create` runs through to the end. */
+		const mockSuccessfulCreatePath = () => {
+			workflowFinderService.findWorkflowForUser.mockResolvedValue(
+				mock<WorkflowEntity>({ isArchived: false }),
+			);
+			workflowHistoryService.findVersion.mockResolvedValue(mock());
+			// The locked check reads the latest archived state.
+			workflowEntityRepository.findArchivedState.mockResolvedValue({ isArchived: false });
+			sharedWorkflowRepository.getWorkflowOwningProject.mockResolvedValue(
+				mock<Project>({ id: 'project-1' }),
+			);
+			requestRepository.findOpenRequestForWorkflow.mockResolvedValue(null);
+			requestRepository.createRequest.mockResolvedValue(
+				mock<WorkflowReviewRequest>({
+					id: 'req-1',
+					createdAt: new Date('2024-01-01T00:00:00.000Z'),
+					updatedAt: new Date('2024-01-01T00:00:00.000Z'),
+				}),
+			);
+			workflowHistoryRepository.updateVersionMetadata.mockResolvedValue(1);
+		};
+
+		it('refuses to open anything once an admin turns reviews off, before any lookup or lock', async () => {
+			workflowReviewPolicyService.get.mockResolvedValue({ enabled: false });
+
+			await expect(service.create(user, dto)).rejects.toThrow(ForbiddenError);
+
+			expect(workflowFinderService.findWorkflowForUser).not.toHaveBeenCalled();
+			expect(dbLockService.withLockContext).not.toHaveBeenCalled();
+		});
+
+		it('writes the review, its workflow reference, and its author in one transaction', async () => {
+			mockSuccessfulCreatePath();
+
+			const result = await service.create(user, dto);
+
+			expect(result.id).toBe('req-1');
+			expect(dbLockService.withLockContext).toHaveBeenCalledWith(
+				DbLock.WORKFLOW_REVIEW_MUTATION,
+				expect.any(Function),
+			);
+			expect(requestRepository.createRequest).toHaveBeenCalledWith(
+				{
+					projectId: 'project-1',
+					title: 'Please review',
+					description: 'A description',
+					createdById: 'user-1',
+				},
+				ctx,
+			);
+			expect(workflowRepository.createWorkflowRow).toHaveBeenCalledWith(
+				{ workflowReviewRequestId: 'req-1', workflowId: 'wf-1', workflowVersionId: 'ver-1' },
+				ctx,
+			);
+			expect(authorRepository.addAuthor).toHaveBeenCalledWith(
+				{ workflowReviewRequestId: 'req-1', userId: 'user-1' },
+				ctx,
+			);
+		});
+
+		it('refuses a workflow the caller cannot publish, before taking the lock', async () => {
+			workflowFinderService.findWorkflowForUser.mockResolvedValue(null);
+
+			await expect(service.create(user, dto)).rejects.toThrow(NotFoundError);
+
+			expect(workflowFinderService.findWorkflowForUser).toHaveBeenCalledWith('wf-1', user, [
+				'workflow:publish',
+			]);
+			expect(dbLockService.withLockContext).not.toHaveBeenCalled();
+		});
+
+		it('refuses an archived workflow, before taking the lock', async () => {
+			workflowFinderService.findWorkflowForUser.mockResolvedValue(
+				mock<WorkflowEntity>({ isArchived: true }),
+			);
+
+			await expect(service.create(user, dto)).rejects.toThrow(BadRequestError);
+			expect(dbLockService.withLockContext).not.toHaveBeenCalled();
+		});
+
+		it('refuses a version the workflow does not have, before taking the lock', async () => {
+			workflowFinderService.findWorkflowForUser.mockResolvedValue(
+				mock<WorkflowEntity>({ isArchived: false }),
+			);
+			workflowHistoryService.findVersion.mockResolvedValue(null);
+
+			await expect(service.create(user, dto)).rejects.toThrow(BadRequestError);
+			expect(dbLockService.withLockContext).not.toHaveBeenCalled();
+		});
+
+		it('refuses a workflow that belongs to no project', async () => {
+			workflowFinderService.findWorkflowForUser.mockResolvedValue(
+				mock<WorkflowEntity>({ isArchived: false }),
+			);
+			workflowHistoryService.findVersion.mockResolvedValue(mock());
+			sharedWorkflowRepository.getWorkflowOwningProject.mockResolvedValue(undefined);
+
+			await expect(service.create(user, dto)).rejects.toThrow(NotFoundError);
+			expect(dbLockService.withLockContext).not.toHaveBeenCalled();
+		});
+
+		it('points at the review already open on the workflow, and writes nothing', async () => {
+			mockSuccessfulCreatePath();
+			requestRepository.findOpenRequestForWorkflow.mockResolvedValue(
+				mock<WorkflowReviewRequest>({ id: 'existing-1' }),
+			);
+
+			const error = await service.create(user, dto).catch((e: unknown) => e);
+			expect(error).toBeInstanceOf(ConflictError);
+			expect((error as ConflictError).meta).toEqual({ workflowReviewRequestId: 'existing-1' });
+
+			expect(requestRepository.createRequest).not.toHaveBeenCalled();
+			expect(workflowRepository.createWorkflowRow).not.toHaveBeenCalled();
+			expect(authorRepository.addAuthor).not.toHaveBeenCalled();
+			expect(collaborationService.broadcastWorkflowReviewStateChanged).not.toHaveBeenCalled();
+			expect(eventService.emit).not.toHaveBeenCalled();
+		});
+
+		describe('a workflow that changes while the submission waits for the lock', () => {
+			it('refuses one that was archived in the meantime', async () => {
+				mockSuccessfulCreatePath();
+				workflowEntityRepository.findArchivedState.mockResolvedValue({ isArchived: true });
+
+				const creation = service.create(user, dto);
+				await expect(creation).rejects.toThrow(BadRequestError);
+				await expect(creation).rejects.toThrow(
+					"The workflow 'wf-1' is archived and cannot be submitted for review",
+				);
+
+				expect(dbLockService.withLockContext).toHaveBeenCalled();
+				expect(requestRepository.createRequest).not.toHaveBeenCalled();
+				expect(workflowRepository.createWorkflowRow).not.toHaveBeenCalled();
+			});
+
+			it('refuses one that was deleted in the meantime', async () => {
+				mockSuccessfulCreatePath();
+				workflowEntityRepository.findArchivedState.mockResolvedValue(null);
+
+				await expect(service.create(user, dto)).rejects.toThrow(NotFoundError);
+
+				expect(requestRepository.createRequest).not.toHaveBeenCalled();
+				expect(workflowRepository.createWorkflowRow).not.toHaveBeenCalled();
+			});
+
+			it('refuses one whose owning project disappeared in the meantime', async () => {
+				mockSuccessfulCreatePath();
+				sharedWorkflowRepository.getWorkflowOwningProject
+					.mockResolvedValueOnce(mock<Project>({ id: 'project-1' }))
+					.mockResolvedValueOnce(undefined);
+
+				const creation = service.create(user, dto);
+				await expect(creation).rejects.toThrow(NotFoundError);
+				await expect(creation).rejects.toThrow('Could not find workflow');
+
+				expect(dbLockService.withLockContext).toHaveBeenCalled();
+				expect(requestRepository.createRequest).not.toHaveBeenCalled();
+			});
+
+			it('refuses one that moved to another project in the meantime', async () => {
+				mockSuccessfulCreatePath();
+				sharedWorkflowRepository.getWorkflowOwningProject
+					.mockResolvedValueOnce(mock<Project>({ id: 'project-1' }))
+					.mockResolvedValueOnce(mock<Project>({ id: 'project-2' }));
+
+				await expect(service.create(user, dto)).rejects.toThrow(ConflictError);
+
+				expect(dbLockService.withLockContext).toHaveBeenCalled();
+				expect(requestRepository.createRequest).not.toHaveBeenCalled();
+			});
+
+			it('re-reads both facts on the lock transaction', async () => {
+				mockSuccessfulCreatePath();
+
+				await service.create(user, dto);
+
+				expect(workflowEntityRepository.findArchivedState).toHaveBeenCalledWith('wf-1', ctx);
+				expect(sharedWorkflowRepository.getWorkflowOwningProject).toHaveBeenLastCalledWith(
+					'wf-1',
+					ctx,
+				);
+			});
+		});
+
+		describe('assigning reviewers', () => {
+			const mockEligibleReviewers = (...ids: string[]) => {
+				roleService.rolesWithScope.mockResolvedValue(['some-role']);
+				userRepository.findEligibleByProjectOrGlobalRoles.mockResolvedValue(
+					ids.map((id) => loadedUser({ id, email: `${id}@n8n.io` })),
+				);
+			};
+
+			it('writes deduplicated reviewers in the same transaction as the review', async () => {
+				mockSuccessfulCreatePath();
+				mockEligibleReviewers('user-2', 'user-3');
+
+				await service.create(user, {
+					...dto,
+					reviewerUserIds: ['user-2', 'user-2', 'user-3'],
+				});
+
+				expect(reviewerRepository.addReviewers).toHaveBeenCalledWith(
+					{ workflowReviewRequestId: 'req-1', userIds: ['user-2', 'user-3'] },
+					ctx,
+				);
+			});
+
+			it('refuses a requester who assigns themselves, before checking eligibility or locking', async () => {
+				mockSuccessfulCreatePath();
+
+				await expect(service.create(user, { ...dto, reviewerUserIds: ['user-1'] })).rejects.toThrow(
+					BadRequestError,
+				);
+
+				expect(userRepository.findEligibleByProjectOrGlobalRoles).not.toHaveBeenCalled();
+				expect(dbLockService.withLockContext).not.toHaveBeenCalled();
+			});
+
+			it('names the reviewers who are not eligible, before taking the lock', async () => {
+				mockSuccessfulCreatePath();
+				mockEligibleReviewers('user-2');
+
+				await expect(
+					service.create(user, { ...dto, reviewerUserIds: ['user-2', 'user-99'] }),
+				).rejects.toThrow('These users are not eligible to review this workflow: user-99');
+
+				expect(dbLockService.withLockContext).not.toHaveBeenCalled();
+			});
+
+			it('refuses someone who has not accepted their invitation yet, whatever their role', async () => {
+				mockSuccessfulCreatePath();
+				roleService.rolesWithScope.mockResolvedValue(['some-role']);
+				userRepository.findEligibleByProjectOrGlobalRoles.mockResolvedValue([
+					loadedUser({ id: 'user-2', email: 'user-2@n8n.io', password: null }),
+				]);
+
+				await expect(service.create(user, { ...dto, reviewerUserIds: ['user-2'] })).rejects.toThrow(
+					BadRequestError,
+				);
+			});
+
+			// The service still validates this if DTO validation is bypassed.
+			it.each<[string, string[] | undefined]>([
+				['omitted', undefined],
+				['empty', []],
+			])('refuses a review whose reviewer list is %s', async (_name, reviewerUserIds) => {
+				mockSuccessfulCreatePath();
+
+				await expect(
+					service.create(user, {
+						...dto,
+						reviewerUserIds,
+					} as CreateWorkflowReviewRequestDto),
+				).rejects.toThrow(BadRequestError);
+
+				expect(dbLockService.withLockContext).not.toHaveBeenCalled();
+				expect(reviewerRepository.addReviewers).not.toHaveBeenCalled();
+			});
+		});
+
+		describe('naming the version under review', () => {
+			const namedDto = (workflowVersionName: string): CreateWorkflowReviewRequestDto => ({
+				...dto,
+				workflows: [{ ...dto.workflows[0], workflowVersionName }],
+			});
+
+			it('names the version in the same transaction as the review, trimmed', async () => {
+				mockSuccessfulCreatePath();
+
+				await service.create(user, namedDto('  Release candidate  '));
+
+				expect(workflowHistoryRepository.updateVersionMetadata).toHaveBeenCalledWith(
+					{
+						workflowId: 'wf-1',
+						versionId: 'ver-1',
+						name: 'Release candidate',
+						description: undefined,
+					},
+					ctx,
+				);
+			});
+
+			it('writes a trimmed version description alongside the name', async () => {
+				mockSuccessfulCreatePath();
+
+				await service.create(user, {
+					...dto,
+					workflows: [{ ...dto.workflows[0], workflowVersionDescription: '  What changed  ' }],
+				});
+
+				expect(workflowHistoryRepository.updateVersionMetadata).toHaveBeenCalledWith(
+					expect.objectContaining({ description: 'What changed' }),
+					ctx,
+				);
+			});
+
+			it('clears the version description when a blank one is sent', async () => {
+				mockSuccessfulCreatePath();
+
+				await service.create(user, {
+					...dto,
+					workflows: [{ ...dto.workflows[0], workflowVersionDescription: '   ' }],
+				});
+
+				expect(workflowHistoryRepository.updateVersionMetadata).toHaveBeenCalledWith(
+					expect.objectContaining({ description: null }),
+					ctx,
+				);
+			});
+
+			it('leaves the version unnamed when an open review already conflicts', async () => {
+				mockSuccessfulCreatePath();
+				requestRepository.findOpenRequestForWorkflow.mockResolvedValue(
+					mock<WorkflowReviewRequest>({ id: 'existing-1' }),
+				);
+
+				await expect(service.create(user, namedDto('Release candidate'))).rejects.toThrow(
+					ConflictError,
+				);
+
+				expect(workflowHistoryRepository.updateVersionMetadata).not.toHaveBeenCalled();
+			});
+
+			it('refuses the review when the version was pruned before the naming write', async () => {
+				mockSuccessfulCreatePath();
+				workflowHistoryRepository.updateVersionMetadata.mockResolvedValue(0);
+
+				await expect(service.create(user, namedDto('Release candidate'))).rejects.toThrow(
+					BadRequestError,
+				);
+			});
+		});
+
+		it('tells open editors and reports the request exactly once, after the lock resolves', async () => {
+			mockSuccessfulCreatePath();
+			let lockResolved = false;
+			dbLockService.withLockContext.mockImplementation(async (_id, fn) => {
+				const result = await fn(ctx);
+				lockResolved = true;
+				return result;
+			});
+			collaborationService.broadcastWorkflowReviewStateChanged.mockImplementation(async () => {
+				expect(lockResolved).toBe(true);
+			});
+
+			await service.create(user, dto);
+
+			expect(collaborationService.broadcastWorkflowReviewStateChanged).toHaveBeenCalledTimes(1);
+			expect(collaborationService.broadcastWorkflowReviewStateChanged).toHaveBeenCalledWith('wf-1');
+			expect(eventService.emit).toHaveBeenCalledExactlyOnceWith('workflow-review-requested', {
+				user: expect.objectContaining({ id: 'user-1' }),
+				workflowReviewRequestId: 'req-1',
+				projectId: 'project-1',
+				workflowId: 'wf-1',
+				workflowVersionId: 'ver-1',
+				reviewerCount: 1,
+			});
+		});
+	});
+
+	describe('listing who may review a workflow', () => {
+		const query = { workflowId: 'wf-1' } as GetWorkflowReviewEligibleReviewersQueryDto;
+
+		it('refuses a workflow the caller cannot publish', async () => {
+			workflowFinderService.findWorkflowForUser.mockResolvedValue(null);
+
+			await expect(service.getEligibleReviewers(user, query)).rejects.toThrow(NotFoundError);
+
+			expect(workflowFinderService.findWorkflowForUser).toHaveBeenCalledWith('wf-1', user, [
+				'workflow:publish',
+			]);
+			expect(userRepository.findEligibleByProjectOrGlobalRoles).not.toHaveBeenCalled();
+		});
+
+		it('refuses a workflow that belongs to no project', async () => {
+			workflowFinderService.findWorkflowForUser.mockResolvedValue(mock<WorkflowEntity>());
+			sharedWorkflowRepository.getWorkflowOwningProject.mockResolvedValue(undefined);
+
+			await expect(service.getEligibleReviewers(user, query)).rejects.toThrow(NotFoundError);
+		});
+
+		it('offers an SSO user who has no password, rather than treating them as un-invited', async () => {
+			workflowFinderService.findWorkflowForUser.mockResolvedValue(mock<WorkflowEntity>());
+			sharedWorkflowRepository.getWorkflowOwningProject.mockResolvedValue(
+				mock<Project>({ id: 'project-1' }),
+			);
+			roleService.rolesWithScope.mockImplementation(async (namespace) =>
+				namespace === 'project'
+					? ['project:admin', 'project:editor', 'custom:reviewer']
+					: ['global:owner', 'global:admin'],
+			);
+			userRepository.findEligibleByProjectOrGlobalRoles.mockResolvedValue([
+				loadedUser({
+					id: 'user-sso',
+					email: 'sso@n8n.io',
+					password: null,
+					authIdentities: [mock<AuthIdentity>({ providerType: 'ldap' })],
+				}),
+			]);
+
+			const result = await service.getEligibleReviewers(user, query);
+
+			expect(result.data).toEqual([
+				{ id: 'user-sso', email: 'sso@n8n.io', firstName: null, lastName: null },
+			]);
+		});
+	});
+});

packages/cli/src/modules/workflow-reviews.ee/__tests__/workflow-review-request-submission.version-update.service.test.ts

@@ -131,7 +131,7 @@ describe('WorkflowReviewRequestSubmissionService.updateVersion', () => {
 		collaborationService.broadcastWorkflowReviewStateChanged.mockResolvedValue(undefined);
 	});
 
-	it('throws when the instance policy is disabled, before any lookup or lock', async () => {
+	it('refuses everything once an admin turns reviews off, before any lookup or lock', async () => {
 		workflowReviewPolicyService.get.mockResolvedValue({ enabled: false });
 
 		await expect(service.updateVersion(user, requestId, dto)).rejects.toThrow(ForbiddenError);
@@ -141,7 +141,7 @@ describe('WorkflowReviewRequestSubmissionService.updateVersion', () => {
 		expect(dbLockService.withLockContext).not.toHaveBeenCalled();
 	});
 
-	it('throws NotFoundError when the review request does not exist', async () => {
+	it('refuses a review that does not exist', async () => {
 		requestRepository.findById.mockResolvedValue(null);
 
 		await expect(service.updateVersion(user, requestId, dto)).rejects.toThrow(NotFoundError);
@@ -149,7 +149,7 @@ describe('WorkflowReviewRequestSubmissionService.updateVersion', () => {
 		expect(dbLockService.withLockContext).not.toHaveBeenCalled();
 	});
 
-	it('throws NotFoundError when the request does not cover the given workflow', async () => {
+	it('refuses a workflow the review does not cover', async () => {
 		requestRepository.findById.mockResolvedValue(openRequest());
 		workflowRepository.findByRequestId.mockResolvedValue([
 			mock<WorkflowReviewRequestWorkflow>({ workflowId: 'other-wf' }),
@@ -161,7 +161,7 @@ describe('WorkflowReviewRequestSubmissionService.updateVersion', () => {
 		expect(dbLockService.withLockContext).not.toHaveBeenCalled();
 	});
 
-	it('throws NotFoundError when the user lacks publish access to the workflow', async () => {
+	it('refuses a workflow the caller cannot publish', async () => {
 		mockSuccessfulUpdatePath();
 		workflowFinderService.findWorkflowForUser.mockResolvedValue(null);
 
@@ -173,7 +173,7 @@ describe('WorkflowReviewRequestSubmissionService.updateVersion', () => {
 		expect(dbLockService.withLockContext).not.toHaveBeenCalled();
 	});
 
-	it('throws BadRequestError and never takes the lock for an archived workflow', async () => {
+	it('refuses an archived workflow, before taking the lock', async () => {
 		mockSuccessfulUpdatePath();
 		workflowFinderService.findWorkflowForUser.mockResolvedValue(
 			mock<WorkflowEntity>({ isArchived: true }),
@@ -187,7 +187,7 @@ describe('WorkflowReviewRequestSubmissionService.updateVersion', () => {
 	it.each([
 		['closed', openRequest({ state: 'closed' })],
 		['approved', openRequest({ decision: 'approved' })],
-	])('throws ConflictError and never takes the lock when the request is %s', async (_name, req) => {
+	])('refuses a review that is already %s, before taking the lock', async (_name, req) => {
 		mockSuccessfulUpdatePath();
 		requestRepository.findById.mockResolvedValue(req);
 
@@ -196,7 +196,7 @@ describe('WorkflowReviewRequestSubmissionService.updateVersion', () => {
 		expect(dbLockService.withLockContext).not.toHaveBeenCalled();
 	});
 
-	it('throws BadRequestError and never takes the lock when the version does not exist', async () => {
+	it('refuses a version the workflow does not have, before taking the lock', async () => {
 		mockSuccessfulUpdatePath();
 		workflowHistoryService.findVersion.mockResolvedValue(null);
 
@@ -272,7 +272,7 @@ describe('WorkflowReviewRequestSubmissionService.updateVersion', () => {
 		});
 	});
 
-	it('throws ConflictError and writes nothing when the request closes between check and lock', async () => {
+	it('refuses to re-pin a review that closed while the update waited for the lock, and writes nothing', async () => {
 		mockSuccessfulUpdatePath();
 		requestRepository.findById
 			.mockResolvedValueOnce(openRequest())
@@ -283,6 +283,7 @@ describe('WorkflowReviewRequestSubmissionService.updateVersion', () => {
 		expect(workflowRepository.updateWorkflowVersion).not.toHaveBeenCalled();
 		expect(requestRepository.saveRequest).not.toHaveBeenCalled();
 		expect(authorRepository.addAuthorIfMissing).not.toHaveBeenCalled();
+		expect(collaborationService.broadcastWorkflowReviewStateChanged).not.toHaveBeenCalled();
 	});
 
 	it('writes and broadcasts nothing when a concurrent identical sync wins the lock first', async () => {
@@ -332,7 +333,7 @@ describe('WorkflowReviewRequestSubmissionService.updateVersion', () => {
 		expect(requestRepository.saveRequest).not.toHaveBeenCalled();
 	});
 
-	it('throws NotFoundError when the request disappears between check and lock', async () => {
+	it('refuses a review deleted while the update waited for the lock', async () => {
 		mockSuccessfulUpdatePath();
 		requestRepository.findById.mockResolvedValueOnce(openRequest()).mockResolvedValueOnce(null);
 
@@ -402,7 +403,7 @@ describe('WorkflowReviewRequestSubmissionService.updateVersion', () => {
 			);
 		});
 
-		it('throws BadRequestError when the version was pruned before the naming write', async () => {
+		it('refuses the update when the version was pruned before the naming write', async () => {
 			mockSuccessfulUpdatePath();
 			workflowHistoryRepository.updateVersionMetadata.mockResolvedValue(0);
 
@@ -580,51 +581,21 @@ describe('WorkflowReviewRequestSubmissionService.updateVersion', () => {
 		});
 	});
 
-	describe('review state broadcast', () => {
-		it('broadcasts exactly once after the lock resolves', async () => {
-			mockSuccessfulUpdatePath();
-			let lockResolved = false;
-			dbLockService.withLockContext.mockImplementation(async (_id, fn) => {
-				const result = await fn(ctx);
-				lockResolved = true;
-				return result;
-			});
-			collaborationService.broadcastWorkflowReviewStateChanged.mockImplementation(async () => {
-				expect(lockResolved).toBe(true);
-			});
-
-			await service.updateVersion(user, requestId, dto);
-
-			expect(collaborationService.broadcastWorkflowReviewStateChanged).toHaveBeenCalledTimes(1);
-			expect(collaborationService.broadcastWorkflowReviewStateChanged).toHaveBeenCalledWith('wf-1');
+	it('tells open editors exactly once, after the lock resolves', async () => {
+		mockSuccessfulUpdatePath();
+		let lockResolved = false;
+		dbLockService.withLockContext.mockImplementation(async (_id, fn) => {
+			const result = await fn(ctx);
+			lockResolved = true;
+			return result;
+		});
+		collaborationService.broadcastWorkflowReviewStateChanged.mockImplementation(async () => {
+			expect(lockResolved).toBe(true);
 		});
 
-		it('does not broadcast on an in-transaction conflict', async () => {
-			mockSuccessfulUpdatePath();
-			requestRepository.findById
-				.mockResolvedValueOnce(openRequest())
-				.mockResolvedValueOnce(openRequest({ state: 'closed' }));
+		await service.updateVersion(user, requestId, dto);
 
-			await expect(service.updateVersion(user, requestId, dto)).rejects.toThrow(ConflictError);
-
-			expect(collaborationService.broadcastWorkflowReviewStateChanged).not.toHaveBeenCalled();
-		});
-
-		it('resolves and logs a warning when the broadcast rejects', async () => {
-			mockSuccessfulUpdatePath();
-			collaborationService.broadcastWorkflowReviewStateChanged.mockRejectedValue(
-				new Error('push down'),
-			);
-
-			const result = await service.updateVersion(user, requestId, dto);
-			expect(result.id).toBe(requestId);
-
-			// Wait for the rejected notification to be logged.
-			await new Promise(process.nextTick);
-			expect(logger.warn).toHaveBeenCalledWith(
-				'Failed to broadcast review state change',
-				expect.objectContaining({ workflowId: 'wf-1' }),
-			);
-		});
+		expect(collaborationService.broadcastWorkflowReviewStateChanged).toHaveBeenCalledTimes(1);
+		expect(collaborationService.broadcastWorkflowReviewStateChanged).toHaveBeenCalledWith('wf-1');
 	});
 });

packages/cli/src/modules/workflow-reviews.ee/__tests__/workflow-review-request.service.test.ts

@@ -1,785 +0,0 @@
-import type {
-	CreateWorkflowReviewRequestDto,
-	GetWorkflowReviewEligibleReviewersQueryDto,
-	ListWorkflowReviewRequestsQueryDto,
-} from '@n8n/api-types';
-import type { LicenseState, Logger } from '@n8n/backend-common';
-import { DbLock, User } from '@n8n/db';
-import type {
-	AuthIdentity,
-	DbLockService,
-	Project,
-	SharedWorkflowRepository,
-	UserRepository,
-	WorkflowEntity,
-	WorkflowHistoryRepository,
-	WorkflowReviewRequest,
-	WorkflowReviewRequestAuthorRepository,
-	WorkflowReviewRequestRepository,
-	WorkflowReviewRequestForWorkflowRow,
-	WorkflowReviewActivityRepository,
-	WorkflowReviewRequestReviewerRepository,
-	WorkflowReviewRequestWorkflowRepository,
-	WorkflowRepository,
-	Transaction,
-	OperationContext,
-} from '@n8n/db';
-import { mock } from 'vitest-mock-extended';
-
-import type { CollaborationService } from '@/collaboration/collaboration.service';
-import { BadRequestError } from '@/errors/response-errors/bad-request.error';
-import { ConflictError } from '@/errors/response-errors/conflict.error';
-import { ForbiddenError } from '@/errors/response-errors/forbidden.error';
-import { NotFoundError } from '@/errors/response-errors/not-found.error';
-import type { EventService } from '@/events/event.service';
-import type { RoleService } from '@/services/role.service';
-import type { WorkflowReviewPolicyService } from '@/services/workflow-review-policy.service';
-import type { WorkflowFinderService } from '@/workflows/workflow-finder.service';
-import type { WorkflowHistoryService } from '@/workflows/workflow-history/workflow-history.service';
-import type { WorkflowReviewAuthorizationService } from '../workflow-review-authorization.service';
-
-import { WorkflowReviewFeatureGate } from '../workflow-review-feature-gate.service';
-import { WorkflowReviewRequestMutationGuard } from '../workflow-review-request-mutation-guard.service';
-import { WorkflowReviewRequestStatusService } from '../workflow-review-request-status.service';
-import { WorkflowReviewRequestSubmissionService } from '../workflow-review-request-submission.service';
-import { WorkflowReviewStateNotifier } from '../workflow-review-state-notifier.service';
-const user = mock<User>({ id: 'user-1' });
-
-/** Build a loaded user with the computed pending state. */
-function loadedUser(fields: Partial<User> & { id: string; email: string }): User {
-	const loaded = Object.assign(new User(), { password: 'hashed', authIdentities: [], ...fields });
-	loaded.computeIsPending();
-	return loaded;
-}
-
-const dto: CreateWorkflowReviewRequestDto = {
-	title: 'Please review',
-	description: 'A description',
-	workflows: [
-		{ workflowId: 'wf-1', workflowVersionId: 'ver-1', workflowVersionName: 'Release candidate' },
-	],
-	reviewerUserIds: ['user-2'],
-};
-
-describe('workflow review request services', () => {
-	const workflowReviewPolicyService = mock<WorkflowReviewPolicyService>();
-	const workflowFinderService = mock<WorkflowFinderService>();
-	const workflowHistoryService = mock<WorkflowHistoryService>();
-	const workflowHistoryRepository = mock<WorkflowHistoryRepository>();
-	/** Workflow entity repository; `workflowRepository` stores request links. */
-	const workflowEntityRepository = mock<WorkflowRepository>();
-	const sharedWorkflowRepository = mock<SharedWorkflowRepository>();
-	const requestRepository = mock<WorkflowReviewRequestRepository>();
-	const workflowRepository = mock<WorkflowReviewRequestWorkflowRepository>();
-	const authorRepository = mock<WorkflowReviewRequestAuthorRepository>();
-	const reviewerRepository = mock<WorkflowReviewRequestReviewerRepository>();
-	const activityRepository = mock<WorkflowReviewActivityRepository>();
-	const userRepository = mock<UserRepository>();
-	const roleService = mock<RoleService>();
-	const licenseState = mock<LicenseState>();
-	const dbLockService = mock<DbLockService>();
-	const collaborationService = mock<CollaborationService>();
-	const authorizationService = mock<WorkflowReviewAuthorizationService>();
-	const logger = mock<Logger>();
-	const eventService = mock<EventService>();
-	/** Transaction context used inside the lock. */
-	const ctx: OperationContext = { trx: mock<Transaction>() };
-
-	const featureGate = new WorkflowReviewFeatureGate(licenseState, workflowReviewPolicyService);
-	const mutationGuard = new WorkflowReviewRequestMutationGuard(
-		workflowEntityRepository,
-		sharedWorkflowRepository,
-	);
-	const submissionService = new WorkflowReviewRequestSubmissionService(
-		featureGate,
-		workflowFinderService,
-		workflowHistoryService,
-		workflowHistoryRepository,
-		sharedWorkflowRepository,
-		requestRepository,
-		workflowRepository,
-		authorRepository,
-		reviewerRepository,
-		activityRepository,
-		userRepository,
-		roleService,
-		dbLockService,
-		eventService,
-		mutationGuard,
-		new WorkflowReviewStateNotifier(logger, collaborationService),
-	);
-	const statusService = new WorkflowReviewRequestStatusService(
-		featureGate,
-		workflowFinderService,
-		requestRepository,
-		userRepository,
-		authorizationService,
-	);
-
-	beforeEach(() => {
-		vi.resetAllMocks();
-		authorizationService.resolveOpenableRequestIds.mockResolvedValue(new Set());
-		licenseState.isWorkflowReviewsLicensed.mockReturnValue(true);
-		// Enable the feature unless a test overrides it.
-		workflowReviewPolicyService.get.mockResolvedValue({ enabled: true });
-		// Run locked work with the transaction context by default.
-		dbLockService.withLockContext.mockImplementation(async (_id, fn) => await fn(ctx));
-		collaborationService.broadcastWorkflowReviewStateChanged.mockResolvedValue(undefined);
-	});
-
-	describe('create', () => {
-		// Provide the required eligible reviewer by default.
-		beforeEach(() => {
-			roleService.rolesWithScope.mockResolvedValue(['some-role']);
-			userRepository.findEligibleByProjectOrGlobalRoles.mockResolvedValue([
-				loadedUser({ id: 'user-2', email: 'user-2@n8n.io' }),
-			]);
-		});
-
-		const mockSuccessfulCreatePath = () => {
-			workflowFinderService.findWorkflowForUser.mockResolvedValue(
-				mock<WorkflowEntity>({ isArchived: false }),
-			);
-			workflowHistoryService.findVersion.mockResolvedValue(mock());
-			// The locked check reads the latest archived state.
-			workflowEntityRepository.findArchivedState.mockResolvedValue({ isArchived: false });
-			sharedWorkflowRepository.getWorkflowOwningProject.mockResolvedValue(
-				mock<Project>({ id: 'project-1' }),
-			);
-			requestRepository.findOpenRequestForWorkflow.mockResolvedValue(null);
-			requestRepository.createRequest.mockResolvedValue(
-				mock<WorkflowReviewRequest>({
-					id: 'req-1',
-					createdAt: new Date('2024-01-01T00:00:00.000Z'),
-					updatedAt: new Date('2024-01-01T00:00:00.000Z'),
-				}),
-			);
-			workflowHistoryRepository.updateVersionMetadata.mockResolvedValue(1);
-		};
-
-		it('throws when the instance policy is disabled, before any lookup or lock', async () => {
-			workflowReviewPolicyService.get.mockResolvedValue({ enabled: false });
-
-			await expect(submissionService.create(user, dto)).rejects.toThrow(ForbiddenError);
-
-			expect(workflowFinderService.findWorkflowForUser).not.toHaveBeenCalled();
-			expect(dbLockService.withLockContext).not.toHaveBeenCalled();
-		});
-
-		it('creates the review request, workflow reference, and author in one transaction', async () => {
-			workflowFinderService.findWorkflowForUser.mockResolvedValue(
-				mock<WorkflowEntity>({ isArchived: false }),
-			);
-			workflowHistoryService.findVersion.mockResolvedValue(mock());
-			workflowEntityRepository.findArchivedState.mockResolvedValue({ isArchived: false });
-			sharedWorkflowRepository.getWorkflowOwningProject.mockResolvedValue(
-				mock<Project>({ id: 'project-1' }),
-			);
-			requestRepository.findOpenRequestForWorkflow.mockResolvedValue(null);
-			requestRepository.createRequest.mockResolvedValue(
-				mock<WorkflowReviewRequest>({
-					id: 'req-1',
-					createdAt: new Date('2024-01-01T00:00:00.000Z'),
-					updatedAt: new Date('2024-01-01T00:00:00.000Z'),
-				}),
-			);
-
-			const result = await submissionService.create(user, dto);
-
-			expect(result.id).toBe('req-1');
-			expect(dbLockService.withLockContext).toHaveBeenCalledWith(
-				DbLock.WORKFLOW_REVIEW_MUTATION,
-				expect.any(Function),
-			);
-			expect(requestRepository.createRequest).toHaveBeenCalledWith(
-				{
-					projectId: 'project-1',
-					title: 'Please review',
-					description: 'A description',
-					createdById: 'user-1',
-				},
-				ctx,
-			);
-			expect(workflowRepository.createWorkflowRow).toHaveBeenCalledWith(
-				{ workflowReviewRequestId: 'req-1', workflowId: 'wf-1', workflowVersionId: 'ver-1' },
-				ctx,
-			);
-			expect(authorRepository.addAuthor).toHaveBeenCalledWith(
-				{ workflowReviewRequestId: 'req-1', userId: 'user-1' },
-				ctx,
-			);
-		});
-
-		it('throws NotFoundError without acquiring a lock when the workflow cannot be found', async () => {
-			workflowFinderService.findWorkflowForUser.mockResolvedValue(null);
-
-			await expect(submissionService.create(user, dto)).rejects.toThrow(NotFoundError);
-
-			expect(workflowFinderService.findWorkflowForUser).toHaveBeenCalledWith('wf-1', user, [
-				'workflow:publish',
-			]);
-			expect(dbLockService.withLockContext).not.toHaveBeenCalled();
-		});
-
-		it('throws BadRequestError and never takes the lock for an archived workflow', async () => {
-			workflowFinderService.findWorkflowForUser.mockResolvedValue(
-				mock<WorkflowEntity>({ isArchived: true }),
-			);
-
-			await expect(submissionService.create(user, dto)).rejects.toThrow(BadRequestError);
-			expect(dbLockService.withLockContext).not.toHaveBeenCalled();
-		});
-
-		it('throws BadRequestError and never takes the lock when the version does not exist', async () => {
-			workflowFinderService.findWorkflowForUser.mockResolvedValue(
-				mock<WorkflowEntity>({ isArchived: false }),
-			);
-			workflowHistoryService.findVersion.mockResolvedValue(null);
-
-			await expect(submissionService.create(user, dto)).rejects.toThrow(BadRequestError);
-			expect(dbLockService.withLockContext).not.toHaveBeenCalled();
-		});
-
-		it('throws NotFoundError when the workflow has no owning project', async () => {
-			workflowFinderService.findWorkflowForUser.mockResolvedValue(
-				mock<WorkflowEntity>({ isArchived: false }),
-			);
-			workflowHistoryService.findVersion.mockResolvedValue(mock());
-			sharedWorkflowRepository.getWorkflowOwningProject.mockResolvedValue(undefined);
-
-			await expect(submissionService.create(user, dto)).rejects.toThrow(NotFoundError);
-			expect(dbLockService.withLockContext).not.toHaveBeenCalled();
-		});
-
-		it('throws ConflictError carrying the existing id and writes nothing when an open review exists', async () => {
-			workflowFinderService.findWorkflowForUser.mockResolvedValue(
-				mock<WorkflowEntity>({ isArchived: false }),
-			);
-			workflowHistoryService.findVersion.mockResolvedValue(mock());
-			workflowEntityRepository.findArchivedState.mockResolvedValue({ isArchived: false });
-			sharedWorkflowRepository.getWorkflowOwningProject.mockResolvedValue(
-				mock<Project>({ id: 'project-1' }),
-			);
-			requestRepository.findOpenRequestForWorkflow.mockResolvedValue(
-				mock<WorkflowReviewRequest>({ id: 'existing-1' }),
-			);
-
-			const error = await submissionService.create(user, dto).catch((e: unknown) => e);
-			expect(error).toBeInstanceOf(ConflictError);
-			expect((error as ConflictError).meta).toEqual({ workflowReviewRequestId: 'existing-1' });
-
-			expect(requestRepository.createRequest).not.toHaveBeenCalled();
-			expect(workflowRepository.createWorkflowRow).not.toHaveBeenCalled();
-			expect(authorRepository.addAuthor).not.toHaveBeenCalled();
-		});
-
-		it('rejects a workflow archived between the pre-lock check and the lock', async () => {
-			mockSuccessfulCreatePath();
-			workflowEntityRepository.findArchivedState.mockResolvedValue({ isArchived: true });
-
-			const creation = submissionService.create(user, dto);
-			await expect(creation).rejects.toThrow(BadRequestError);
-			await expect(creation).rejects.toThrow(
-				"The workflow 'wf-1' is archived and cannot be submitted for review",
-			);
-
-			expect(dbLockService.withLockContext).toHaveBeenCalled();
-			expect(requestRepository.createRequest).not.toHaveBeenCalled();
-			expect(workflowRepository.createWorkflowRow).not.toHaveBeenCalled();
-		});
-
-		it('rejects a workflow deleted between the pre-lock check and the lock', async () => {
-			mockSuccessfulCreatePath();
-			workflowEntityRepository.findArchivedState.mockResolvedValue(null);
-
-			await expect(submissionService.create(user, dto)).rejects.toThrow(NotFoundError);
-
-			expect(requestRepository.createRequest).not.toHaveBeenCalled();
-			expect(workflowRepository.createWorkflowRow).not.toHaveBeenCalled();
-		});
-
-		it('rejects a workflow whose owner disappears between the pre-lock check and the lock', async () => {
-			mockSuccessfulCreatePath();
-			sharedWorkflowRepository.getWorkflowOwningProject
-				.mockResolvedValueOnce(mock<Project>({ id: 'project-1' }))
-				.mockResolvedValueOnce(undefined);
-
-			const creation = submissionService.create(user, dto);
-			await expect(creation).rejects.toThrow(NotFoundError);
-			await expect(creation).rejects.toThrow('Could not find workflow');
-
-			expect(dbLockService.withLockContext).toHaveBeenCalled();
-			expect(requestRepository.createRequest).not.toHaveBeenCalled();
-		});
-
-		// Both reads must use the lock transaction.
-		it('runs both in-lock re-check reads on the lock transaction', async () => {
-			mockSuccessfulCreatePath();
-
-			await submissionService.create(user, dto);
-
-			expect(workflowEntityRepository.findArchivedState).toHaveBeenCalledWith('wf-1', ctx);
-			expect(sharedWorkflowRepository.getWorkflowOwningProject).toHaveBeenLastCalledWith(
-				'wf-1',
-				ctx,
-			);
-		});
-
-		it('rejects a workflow moved to another project between the pre-lock check and the lock', async () => {
-			mockSuccessfulCreatePath();
-			sharedWorkflowRepository.getWorkflowOwningProject
-				.mockResolvedValueOnce(mock<Project>({ id: 'project-1' }))
-				.mockResolvedValueOnce(mock<Project>({ id: 'project-2' }));
-
-			await expect(submissionService.create(user, dto)).rejects.toThrow(ConflictError);
-
-			expect(dbLockService.withLockContext).toHaveBeenCalled();
-			expect(requestRepository.createRequest).not.toHaveBeenCalled();
-		});
-
-		describe('reviewer assignment', () => {
-			const mockEligibleReviewers = (...ids: string[]) => {
-				roleService.rolesWithScope.mockResolvedValue(['some-role']);
-				userRepository.findEligibleByProjectOrGlobalRoles.mockResolvedValue(
-					ids.map((id) => loadedUser({ id, email: `${id}@n8n.io` })),
-				);
-			};
-
-			it('writes deduplicated reviewers in the same transaction as the request', async () => {
-				mockSuccessfulCreatePath();
-				mockEligibleReviewers('user-2', 'user-3');
-
-				await submissionService.create(user, {
-					...dto,
-					reviewerUserIds: ['user-2', 'user-2', 'user-3'],
-				});
-
-				expect(reviewerRepository.addReviewers).toHaveBeenCalledWith(
-					{ workflowReviewRequestId: 'req-1', userIds: ['user-2', 'user-3'] },
-					ctx,
-				);
-			});
-
-			it('rejects self-assignment before checking eligibility or taking the lock', async () => {
-				mockSuccessfulCreatePath();
-
-				await expect(
-					submissionService.create(user, { ...dto, reviewerUserIds: ['user-1'] }),
-				).rejects.toThrow(BadRequestError);
-
-				expect(userRepository.findEligibleByProjectOrGlobalRoles).not.toHaveBeenCalled();
-				expect(dbLockService.withLockContext).not.toHaveBeenCalled();
-			});
-
-			it('rejects reviewers outside the eligible set before taking the lock, naming them', async () => {
-				mockSuccessfulCreatePath();
-				mockEligibleReviewers('user-2');
-
-				await expect(
-					submissionService.create(user, { ...dto, reviewerUserIds: ['user-2', 'user-99'] }),
-				).rejects.toThrow('These users are not eligible to review this workflow: user-99');
-
-				expect(dbLockService.withLockContext).not.toHaveBeenCalled();
-			});
-
-			it('rejects a pending user as reviewer even when their role qualifies', async () => {
-				mockSuccessfulCreatePath();
-				roleService.rolesWithScope.mockResolvedValue(['some-role']);
-				userRepository.findEligibleByProjectOrGlobalRoles.mockResolvedValue([
-					loadedUser({ id: 'user-2', email: 'user-2@n8n.io', password: null }),
-				]);
-
-				await expect(
-					submissionService.create(user, { ...dto, reviewerUserIds: ['user-2'] }),
-				).rejects.toThrow(BadRequestError);
-			});
-
-			// The service still validates this if DTO validation is bypassed.
-			it.each<[string, string[] | undefined]>([
-				['omitted', undefined],
-				['empty', []],
-			])('rejects a create with %s reviewers', async (_name, reviewerUserIds) => {
-				mockSuccessfulCreatePath();
-
-				await expect(
-					submissionService.create(user, {
-						...dto,
-						reviewerUserIds,
-					} as CreateWorkflowReviewRequestDto),
-				).rejects.toThrow(BadRequestError);
-
-				expect(dbLockService.withLockContext).not.toHaveBeenCalled();
-				expect(reviewerRepository.addReviewers).not.toHaveBeenCalled();
-			});
-		});
-
-		describe('pinned version naming', () => {
-			const namedDto = (workflowVersionName: string): CreateWorkflowReviewRequestDto => ({
-				...dto,
-				workflows: [{ ...dto.workflows[0], workflowVersionName }],
-			});
-
-			it('names the pinned version in the same transaction as the request', async () => {
-				mockSuccessfulCreatePath();
-
-				await submissionService.create(user, namedDto('  Release candidate  '));
-
-				expect(workflowHistoryRepository.updateVersionMetadata).toHaveBeenCalledWith(
-					{
-						workflowId: 'wf-1',
-						versionId: 'ver-1',
-						name: 'Release candidate',
-						description: undefined,
-					},
-					ctx,
-				);
-			});
-
-			it('writes a trimmed version description alongside the name', async () => {
-				mockSuccessfulCreatePath();
-
-				await submissionService.create(user, {
-					...dto,
-					workflows: [{ ...dto.workflows[0], workflowVersionDescription: '  What changed  ' }],
-				});
-
-				expect(workflowHistoryRepository.updateVersionMetadata).toHaveBeenCalledWith(
-					expect.objectContaining({ description: 'What changed' }),
-					ctx,
-				);
-			});
-
-			it('clears the version description when an empty string is sent', async () => {
-				mockSuccessfulCreatePath();
-
-				await submissionService.create(user, {
-					...dto,
-					workflows: [{ ...dto.workflows[0], workflowVersionDescription: '   ' }],
-				});
-
-				expect(workflowHistoryRepository.updateVersionMetadata).toHaveBeenCalledWith(
-					expect.objectContaining({ description: null }),
-					ctx,
-				);
-			});
-
-			it('does not name the version when an open review already conflicts', async () => {
-				mockSuccessfulCreatePath();
-				requestRepository.findOpenRequestForWorkflow.mockResolvedValue(
-					mock<WorkflowReviewRequest>({ id: 'existing-1' }),
-				);
-
-				await expect(submissionService.create(user, namedDto('Release candidate'))).rejects.toThrow(
-					ConflictError,
-				);
-
-				expect(workflowHistoryRepository.updateVersionMetadata).not.toHaveBeenCalled();
-			});
-
-			it('throws BadRequestError when the version was pruned before the naming write', async () => {
-				mockSuccessfulCreatePath();
-				workflowHistoryRepository.updateVersionMetadata.mockResolvedValue(0);
-
-				await expect(submissionService.create(user, namedDto('Release candidate'))).rejects.toThrow(
-					BadRequestError,
-				);
-			});
-		});
-
-		describe('review state broadcast', () => {
-			it('broadcasts exactly once after the lock resolves', async () => {
-				mockSuccessfulCreatePath();
-				let lockResolved = false;
-				dbLockService.withLockContext.mockImplementation(async (_id, fn) => {
-					const result = await fn(ctx);
-					lockResolved = true;
-					return result;
-				});
-				collaborationService.broadcastWorkflowReviewStateChanged.mockImplementation(async () => {
-					expect(lockResolved).toBe(true);
-				});
-
-				await submissionService.create(user, dto);
-
-				expect(collaborationService.broadcastWorkflowReviewStateChanged).toHaveBeenCalledTimes(1);
-				expect(collaborationService.broadcastWorkflowReviewStateChanged).toHaveBeenCalledWith(
-					'wf-1',
-				);
-				expect(eventService.emit).toHaveBeenCalledExactlyOnceWith('workflow-review-requested', {
-					user: expect.objectContaining({ id: 'user-1' }),
-					workflowReviewRequestId: 'req-1',
-					projectId: 'project-1',
-					workflowId: 'wf-1',
-					workflowVersionId: 'ver-1',
-					reviewerCount: 1,
-				});
-			});
-
-			it('does not broadcast on conflict', async () => {
-				mockSuccessfulCreatePath();
-				requestRepository.findOpenRequestForWorkflow.mockResolvedValue(
-					mock<WorkflowReviewRequest>({ id: 'existing-1' }),
-				);
-
-				await expect(submissionService.create(user, dto)).rejects.toThrow(ConflictError);
-
-				expect(collaborationService.broadcastWorkflowReviewStateChanged).not.toHaveBeenCalled();
-				expect(eventService.emit).not.toHaveBeenCalled();
-			});
-
-			it('resolves and logs a warning when the broadcast rejects', async () => {
-				mockSuccessfulCreatePath();
-				collaborationService.broadcastWorkflowReviewStateChanged.mockRejectedValue(
-					new Error('push down'),
-				);
-
-				const result = await submissionService.create(user, dto);
-				expect(result.id).toBe('req-1');
-
-				// Wait for the rejected notification to be logged.
-				await new Promise(process.nextTick);
-				expect(logger.warn).toHaveBeenCalledWith(
-					'Failed to broadcast review state change',
-					expect.objectContaining({ workflowId: 'wf-1' }),
-				);
-			});
-		});
-	});
-
-	describe('getEligibleReviewers', () => {
-		const query = { workflowId: 'wf-1' } as GetWorkflowReviewEligibleReviewersQueryDto;
-
-		const mockEligibleLookupPath = () => {
-			workflowFinderService.findWorkflowForUser.mockResolvedValue(mock<WorkflowEntity>());
-			sharedWorkflowRepository.getWorkflowOwningProject.mockResolvedValue(
-				mock<Project>({ id: 'project-1' }),
-			);
-			roleService.rolesWithScope.mockImplementation(async (namespace) =>
-				namespace === 'project'
-					? ['project:admin', 'project:editor', 'custom:reviewer']
-					: ['global:owner', 'global:admin'],
-			);
-		};
-
-		it('throws NotFoundError when the user lacks publish access to the workflow', async () => {
-			workflowFinderService.findWorkflowForUser.mockResolvedValue(null);
-
-			await expect(submissionService.getEligibleReviewers(user, query)).rejects.toThrow(
-				NotFoundError,
-			);
-
-			expect(workflowFinderService.findWorkflowForUser).toHaveBeenCalledWith('wf-1', user, [
-				'workflow:publish',
-			]);
-			expect(userRepository.findEligibleByProjectOrGlobalRoles).not.toHaveBeenCalled();
-		});
-
-		it('throws NotFoundError when the workflow has no owning project', async () => {
-			workflowFinderService.findWorkflowForUser.mockResolvedValue(mock<WorkflowEntity>());
-			sharedWorkflowRepository.getWorkflowOwningProject.mockResolvedValue(undefined);
-
-			await expect(submissionService.getEligibleReviewers(user, query)).rejects.toThrow(
-				NotFoundError,
-			);
-		});
-
-		it('does not misclassify an SSO user without a password as pending', async () => {
-			mockEligibleLookupPath();
-			userRepository.findEligibleByProjectOrGlobalRoles.mockResolvedValue([
-				loadedUser({
-					id: 'user-sso',
-					email: 'sso@n8n.io',
-					password: null,
-					authIdentities: [mock<AuthIdentity>({ providerType: 'ldap' })],
-				}),
-			]);
-
-			const result = await submissionService.getEligibleReviewers(user, query);
-
-			expect(result.data).toEqual([
-				{ id: 'user-sso', email: 'sso@n8n.io', firstName: null, lastName: null },
-			]);
-		});
-	});
-
-	describe('list', () => {
-		const query = mock<ListWorkflowReviewRequestsQueryDto>({
-			workflowId: 'wf-1',
-			skip: 0,
-			take: 1,
-		});
-
-		const latestReviewRow = (
-			overrides: Partial<WorkflowReviewRequestForWorkflowRow> = {},
-		): WorkflowReviewRequestForWorkflowRow => ({
-			id: 'req-1',
-			projectId: 'proj-1',
-			state: 'open',
-			decision: 'pending',
-			description: null,
-			updatedById: 'user-2',
-			workflowVersionId: 'ver-1',
-			workflowVersionName: null,
-			createdAt: new Date('2024-01-01T00:00:00.000Z'),
-			updatedAt: new Date('2024-01-02T00:00:00.000Z'),
-			...overrides,
-		});
-
-		const mockLatestReview = (overrides: Partial<WorkflowReviewRequestForWorkflowRow> = {}) => {
-			workflowFinderService.findWorkflowForUser.mockResolvedValue(mock<WorkflowEntity>());
-			requestRepository.findRequestsForWorkflow.mockResolvedValue([
-				[latestReviewRow(overrides)],
-				1,
-			]);
-		};
-
-		const reviewer = loadedUser({
-			id: 'user-2',
-			email: 'reviewer@example.com',
-			firstName: 'Rey',
-			lastName: 'Viewer',
-		});
-
-		it('throws NotFoundError when the user has no read access to the workflow', async () => {
-			workflowFinderService.findWorkflowForUser.mockResolvedValue(null);
-
-			await expect(statusService.list(user, query)).rejects.toThrow(NotFoundError);
-
-			expect(workflowFinderService.findWorkflowForUser).toHaveBeenCalledWith('wf-1', user, [
-				'workflow:read',
-			]);
-			expect(requestRepository.findRequestsForWorkflow).not.toHaveBeenCalled();
-		});
-
-		it('throws when the instance policy is disabled, before any lookup', async () => {
-			workflowReviewPolicyService.get.mockResolvedValue({ enabled: false });
-
-			await expect(statusService.list(user, query)).rejects.toThrow(ForbiddenError);
-
-			expect(workflowFinderService.findWorkflowForUser).not.toHaveBeenCalled();
-			expect(requestRepository.findRequestsForWorkflow).not.toHaveBeenCalled();
-		});
-
-		it('resolves the decision actor of a changes-requested review', async () => {
-			mockLatestReview({ decision: 'changes_requested' });
-			userRepository.findManyByIds.mockResolvedValue([reviewer]);
-
-			const { count, data } = await statusService.list(user, query);
-
-			expect(userRepository.findManyByIds).toHaveBeenCalledWith(['user-2']);
-			expect(count).toBe(1);
-			expect(data).toEqual([
-				{
-					id: 'req-1',
-					state: 'open',
-					decision: 'changes_requested',
-					description: null,
-					workflowVersionId: 'ver-1',
-					workflowVersionName: null,
-					createdAt: '2024-01-01T00:00:00.000Z',
-					updatedAt: '2024-01-02T00:00:00.000Z',
-					decisionBy: {
-						id: 'user-2',
-						email: 'reviewer@example.com',
-						firstName: 'Rey',
-						lastName: 'Viewer',
-					},
-					viewerCanOpen: false,
-				},
-			]);
-		});
-
-		it('carries the pinned version name through to the response', async () => {
-			mockLatestReview({ workflowVersionName: 'Release candidate' });
-
-			const { data } = await statusService.list(user, query);
-
-			expect(data[0]).toMatchObject({ workflowVersionName: 'Release candidate' });
-		});
-
-		it('falls back to no actor when the deciding user was deleted', async () => {
-			mockLatestReview({ decision: 'changes_requested' });
-			userRepository.findManyByIds.mockResolvedValue([]);
-
-			const { data } = await statusService.list(user, query);
-
-			expect(data[0]?.decisionBy).toBeNull();
-		});
-
-		it('resolves no actor when the decision records none', async () => {
-			mockLatestReview({ decision: 'changes_requested', updatedById: null });
-
-			const { data } = await statusService.list(user, query);
-
-			expect(userRepository.findManyByIds).not.toHaveBeenCalled();
-			expect(data[0]?.decisionBy).toBeNull();
-		});
-
-		it('names no actor for an approved review', async () => {
-			mockLatestReview({ state: 'closed', decision: 'approved' });
-
-			const { data } = await statusService.list(user, query);
-
-			expect(data[0]).toMatchObject({ decisionBy: null });
-			expect(userRepository.findManyByIds).not.toHaveBeenCalled();
-		});
-
-		it('marks rows the caller may open, resolved in one batched access check', async () => {
-			mockLatestReview();
-			authorizationService.resolveOpenableRequestIds.mockResolvedValue(new Set(['req-1']));
-
-			const { data } = await statusService.list(user, query);
-
-			expect(authorizationService.resolveOpenableRequestIds).toHaveBeenCalledWith(user, [
-				expect.objectContaining({ id: 'req-1', projectId: 'proj-1' }),
-			]);
-			expect(data[0]?.viewerCanOpen).toBe(true);
-		});
-
-		it('derives no decision actor for a pending review', async () => {
-			mockLatestReview();
-
-			const { data } = await statusService.list(user, query);
-
-			expect(userRepository.findManyByIds).not.toHaveBeenCalled();
-			expect(data[0]).toMatchObject({ decisionBy: null });
-		});
-
-		it('enriches many rows with one decision-actor lookup', async () => {
-			workflowFinderService.findWorkflowForUser.mockResolvedValue(mock<WorkflowEntity>());
-			requestRepository.findRequestsForWorkflow.mockResolvedValue([
-				[
-					latestReviewRow({ id: 'req-1', decision: 'changes_requested', updatedById: 'user-2' }),
-					latestReviewRow({ id: 'req-2', decision: 'changes_requested', updatedById: 'user-3' }),
-					latestReviewRow({
-						id: 'req-3',
-						state: 'closed',
-						decision: 'approved',
-						workflowVersionId: 'ver-3',
-					}),
-					latestReviewRow({
-						id: 'req-4',
-						state: 'closed',
-						decision: 'approved',
-						workflowVersionId: 'ver-4',
-					}),
-				],
-				4,
-			]);
-			userRepository.findManyByIds.mockResolvedValue([
-				reviewer,
-				loadedUser({ id: 'user-3', email: 'other@example.com' }),
-			]);
-			const { data } = await statusService.list(user, query);
-
-			expect(userRepository.findManyByIds).toHaveBeenCalledTimes(1);
-			expect(userRepository.findManyByIds).toHaveBeenCalledWith(['user-2', 'user-3']);
-			expect(data.map((item) => item.decisionBy?.email ?? null)).toEqual([
-				'reviewer@example.com',
-				'other@example.com',
-				null,
-				null,
-			]);
-		});
-	});
-});

packages/cli/src/modules/workflow-reviews.ee/__tests__/workflow-review-state-notifier.service.test.ts

@@ -0,0 +1,61 @@
+import type { Logger } from '@n8n/backend-common';
+import { mock } from 'vitest-mock-extended';
+
+import type { CollaborationService } from '@/collaboration/collaboration.service';
+
+import { WorkflowReviewStateNotifier } from '../workflow-review-state-notifier.service';
+
+/**
+ * Every review mutation ends by telling open editors their review state moved.
+ * Delivery is deliberately fire-and-forget, so the guarantee under test is that a
+ * failed broadcast never reaches the caller — the mutation has already committed.
+ */
+describe('WorkflowReviewStateNotifier', () => {
+	const logger = mock<Logger>();
+	const collaborationService = mock<CollaborationService>();
+	const notifier = new WorkflowReviewStateNotifier(logger, collaborationService);
+
+	beforeEach(() => {
+		vi.resetAllMocks();
+		collaborationService.broadcastWorkflowReviewStateChanged.mockResolvedValue(undefined);
+	});
+
+	it('tells open editors of one workflow', () => {
+		notifier.notify('wf-1');
+
+		expect(
+			collaborationService.broadcastWorkflowReviewStateChanged,
+		).toHaveBeenCalledExactlyOnceWith('wf-1');
+	});
+
+	it('tells open editors of every workflow a batch touched', () => {
+		notifier.notifyMany(['wf-1', 'wf-2']);
+
+		expect(collaborationService.broadcastWorkflowReviewStateChanged).toHaveBeenCalledTimes(2);
+		expect(collaborationService.broadcastWorkflowReviewStateChanged).toHaveBeenCalledWith('wf-1');
+		expect(collaborationService.broadcastWorkflowReviewStateChanged).toHaveBeenCalledWith('wf-2');
+	});
+
+	it('sends nothing for an empty batch', () => {
+		notifier.notifyMany([]);
+
+		expect(collaborationService.broadcastWorkflowReviewStateChanged).not.toHaveBeenCalled();
+	});
+
+	// The mutation is already committed when this runs, so a delivery failure can
+	// only be logged. Every caller relies on this instead of catching it themselves.
+	it('only warns when delivery fails, and never throws at the caller', async () => {
+		collaborationService.broadcastWorkflowReviewStateChanged.mockRejectedValue(
+			new Error('push down'),
+		);
+
+		expect(() => notifier.notify('wf-1')).not.toThrow();
+
+		// Wait for the rejected notification to be logged.
+		await new Promise(process.nextTick);
+		expect(logger.warn).toHaveBeenCalledWith(
+			'Failed to broadcast review state change',
+			expect.objectContaining({ workflowId: 'wf-1' }),
+		);
+	});
+});

packages/cli/src/modules/workflow-reviews.ee/__tests__/workflow-review-status.controller.integration.test.ts

@@ -0,0 +1,376 @@
+import {
+	createTeamProject,
+	createWorkflow,
+	linkUserToProject,
+	mockInstance,
+	testDb,
+} from '@n8n/backend-test-utils';
+import type { Project, User } from '@n8n/db';
+import { UserRepository, WorkflowReviewRequestRepository } from '@n8n/db';
+import { Container } from '@n8n/di';
+import { v4 as uuid } from 'uuid';
+
+import { ActiveWorkflowManager } from '@/active-workflow-manager';
+import { WorkflowReviewPolicyService } from '@/services/workflow-review-policy.service';
+import { WorkflowValidationService } from '@/workflows/workflow-validation.service';
+import { createMember } from '@test-integration/db/users';
+import { createWorkflowHistoryItem } from '@test-integration/db/workflow-history';
+import type { SuperAgentTest } from '@test-integration/types';
+import * as utils from '@test-integration/utils';
+
+import {
+	createReviewableWorkflow,
+	REVIEW_TABLES,
+	seedReview,
+	seedReviewActors,
+	stubWorkflowValidation,
+} from './support/workflow-review-test-data';
+
+mockInstance(ActiveWorkflowManager);
+const workflowValidationService = mockInstance(WorkflowValidationService);
+
+const testServer = utils.setupTestServer({
+	endpointGroups: ['workflow-reviews', 'workflows'],
+	enabledFeatures: ['feat:workflowReviews'],
+	modules: ['workflow-reviews'],
+});
+
+let owner: User;
+let member: User;
+let ownerProject: Project;
+let teamProject: Project;
+let ownerAgent: SuperAgentTest;
+let memberAgent: SuperAgentTest;
+let viewerAgent: SuperAgentTest;
+
+let requestRepository: WorkflowReviewRequestRepository;
+let userRepository: UserRepository;
+let policyService: WorkflowReviewPolicyService;
+
+beforeAll(async () => {
+	await utils.initNodeTypes();
+	requestRepository = Container.get(WorkflowReviewRequestRepository);
+	userRepository = Container.get(UserRepository);
+	policyService = Container.get(WorkflowReviewPolicyService);
+});
+
+beforeEach(async () => {
+	testServer.license.enable('feat:workflowReviews');
+	await testDb.truncate([...REVIEW_TABLES]);
+	await policyService.set(true);
+	stubWorkflowValidation(workflowValidationService);
+
+	({ owner, member, ownerProject, teamProject, ownerAgent, memberAgent, viewerAgent } =
+		await seedReviewActors(testServer.authAgentFor));
+});
+
+const listRequests = (agent: SuperAgentTest, query: Record<string, unknown>) =>
+	agent.get('/workflow-review-requests').query(query);
+
+describe('GET /workflow-review-requests', () => {
+	test('returns an empty list when the workflow has no reviews', async () => {
+		const { workflow } = await createReviewableWorkflow(owner);
+
+		const response = await listRequests(ownerAgent, {
+			workflowId: workflow.id,
+			state: 'open',
+			take: 1,
+		}).expect(200);
+
+		expect(response.body.data).toEqual({ count: 0, data: [] });
+	});
+
+	test('returns the open review as a minimal summary', async () => {
+		const { workflow, versionId } = await createReviewableWorkflow(owner);
+		const request = await seedReview({
+			projectId: ownerProject.id,
+			workflowId: workflow.id,
+			versionId,
+			author: owner,
+			title: 'Confidential title',
+			description: 'Confidential description',
+		});
+
+		const response = await listRequests(ownerAgent, {
+			workflowId: workflow.id,
+			state: 'open',
+			take: 1,
+		}).expect(200);
+
+		expect(response.body.data.count).toBe(1);
+		expect(response.body.data.data).toHaveLength(1);
+
+		expect(response.body.data.data[0]).toEqual({
+			id: request.id,
+			state: 'open',
+			decision: 'pending',
+			workflowVersionId: versionId,
+			workflowVersionName: null,
+			// The owner can act on the review, so the description rides along; the
+			// title stays off the workflow-scoped list entirely.
+			description: 'Confidential description',
+			createdAt: expect.any(String),
+			updatedAt: expect.any(String),
+			// Does not apply to a pending review
+			decisionBy: null,
+			viewerCanOpen: true,
+		});
+	});
+
+	describe('pinned version name', () => {
+		async function listPinnedVersionName(workflowId: string) {
+			const response = await listRequests(ownerAgent, { workflowId, take: 1 }).expect(200);
+
+			expect(response.body.data.data).toHaveLength(1);
+			return response.body.data.data[0].workflowVersionName;
+		}
+
+		test('returns the name the pinned version was given', async () => {
+			const workflow = await createWorkflow({}, owner);
+			const versionId = uuid();
+			await createWorkflowHistoryItem(workflow.id, { versionId, name: 'Release candidate' });
+			await seedReview({
+				projectId: ownerProject.id,
+				workflowId: workflow.id,
+				versionId,
+				author: owner,
+			});
+
+			expect(await listPinnedVersionName(workflow.id)).toBe('Release candidate');
+		});
+
+		test('returns no name for an unnamed version under review', async () => {
+			const { workflow, versionId } = await createReviewableWorkflow(owner);
+			await seedReview({
+				projectId: ownerProject.id,
+				workflowId: workflow.id,
+				versionId,
+				author: owner,
+			});
+
+			expect(await listPinnedVersionName(workflow.id)).toBeNull();
+		});
+	});
+
+	test('withholds the description from a requester who cannot act on the review', async () => {
+		const { workflow, versionId } = await createReviewableWorkflow(teamProject);
+		await seedReview({
+			projectId: teamProject.id,
+			workflowId: workflow.id,
+			versionId,
+			author: owner,
+			title: 'Confidential title',
+			description: 'Confidential description',
+		});
+
+		const viewerResponse = await listRequests(viewerAgent, {
+			workflowId: workflow.id,
+			take: 1,
+		}).expect(200);
+		expect(viewerResponse.body.data.data[0].description).toBeNull();
+
+		const editorResponse = await listRequests(memberAgent, {
+			workflowId: workflow.id,
+			take: 1,
+		}).expect(200);
+		expect(editorResponse.body.data.data[0].description).toBe('Confidential description');
+	});
+
+	test('returns the newest review, closed ones included, when no state is asked for', async () => {
+		const { workflow, versionId } = await createReviewableWorkflow(owner);
+		const older = await seedReview({
+			projectId: ownerProject.id,
+			workflowId: workflow.id,
+			versionId,
+			author: owner,
+			state: 'closed',
+			title: 'Older',
+		});
+		const newest = await seedReview({
+			projectId: ownerProject.id,
+			workflowId: workflow.id,
+			versionId,
+			author: owner,
+			state: 'open',
+			title: 'Newest',
+		});
+		// Both rows are created within the same millisecond, so state the age
+		// explicitly instead of asserting against a timestamp tie.
+		await requestRepository.update(older.id, { createdAt: new Date('2026-01-01T00:00:00.000Z') });
+		await requestRepository.update(newest.id, { createdAt: new Date('2026-01-02T00:00:00.000Z') });
+
+		const response = await listRequests(ownerAgent, { workflowId: workflow.id, take: 1 }).expect(
+			200,
+		);
+
+		expect(response.body.data.data).toHaveLength(1);
+		expect(response.body.data.data[0]).toMatchObject({ id: newest.id, state: 'open' });
+	});
+
+	test('names who asked for changes', async () => {
+		const { workflow, versionId } = await createReviewableWorkflow(owner);
+		await seedReview({
+			projectId: ownerProject.id,
+			workflowId: workflow.id,
+			versionId,
+			author: owner,
+			decision: 'changes_requested',
+			title: 'Needs work',
+			updatedById: member.id,
+		});
+
+		const response = await listRequests(ownerAgent, { workflowId: workflow.id, take: 1 }).expect(
+			200,
+		);
+
+		expect(response.body.data.data[0]).toMatchObject({
+			decision: 'changes_requested',
+			decisionBy: {
+				id: member.id,
+				email: member.email,
+				firstName: member.firstName,
+				lastName: member.lastName,
+			},
+		});
+	});
+
+	test('names nobody once the deciding user is deleted', async () => {
+		const { workflow, versionId } = await createReviewableWorkflow(owner);
+		const reviewer = await createMember();
+		await seedReview({
+			projectId: ownerProject.id,
+			workflowId: workflow.id,
+			versionId,
+			author: owner,
+			decision: 'changes_requested',
+			title: 'Needs work',
+			updatedById: reviewer.id,
+		});
+		await userRepository.delete(reviewer.id);
+
+		const response = await listRequests(ownerAgent, { workflowId: workflow.id, take: 1 }).expect(
+			200,
+		);
+
+		expect(response.body.data.data[0]).toMatchObject({
+			decision: 'changes_requested',
+			decisionBy: null,
+		});
+	});
+
+	test('names nobody for an approved review', async () => {
+		const { workflow, versionId } = await createReviewableWorkflow(owner);
+		await seedReview({
+			projectId: ownerProject.id,
+			workflowId: workflow.id,
+			versionId,
+			author: owner,
+			state: 'closed',
+			decision: 'approved',
+			title: 'Approved',
+			updatedById: member.id,
+		});
+
+		const response = await listRequests(ownerAgent, { workflowId: workflow.id, take: 1 }).expect(
+			200,
+		);
+
+		expect(response.body.data.data[0]).toMatchObject({
+			state: 'closed',
+			decision: 'approved',
+			// Approval is never attributed in the canvas banner
+			decisionBy: null,
+		});
+	});
+
+	test('leaves closed reviews out of the open list, and in when no state is asked for', async () => {
+		const { workflow, versionId } = await createReviewableWorkflow(owner);
+		const closed = await seedReview({
+			projectId: ownerProject.id,
+			workflowId: workflow.id,
+			versionId,
+			author: owner,
+			state: 'closed',
+			title: 'Closed',
+		});
+
+		const openResponse = await listRequests(ownerAgent, {
+			workflowId: workflow.id,
+			state: 'open',
+			take: 1,
+		}).expect(200);
+		expect(openResponse.body.data).toEqual({ count: 0, data: [] });
+
+		const allResponse = await listRequests(ownerAgent, { workflowId: workflow.id }).expect(200);
+		expect(allResponse.body.data.count).toBe(1);
+		expect(allResponse.body.data.data[0]).toMatchObject({ id: closed.id, state: 'closed' });
+	});
+
+	test('does not include requests of other workflows', async () => {
+		const { workflow } = await createReviewableWorkflow(owner);
+		const other = await createReviewableWorkflow(owner, { versionId: 'version-other' });
+		await seedReview({
+			projectId: ownerProject.id,
+			workflowId: other.workflow.id,
+			versionId: 'version-other',
+			author: owner,
+			title: 'For the other workflow',
+		});
+
+		const response = await listRequests(ownerAgent, { workflowId: workflow.id }).expect(200);
+
+		expect(response.body.data).toEqual({ count: 0, data: [] });
+	});
+
+	test('hides a workflow the caller cannot access', async () => {
+		const { workflow } = await createReviewableWorkflow(owner);
+
+		await listRequests(memberAgent, {
+			workflowId: workflow.id,
+			state: 'open',
+			take: 1,
+		}).expect(404);
+	});
+
+	test('lets someone who can only view the workflow see its reviews', async () => {
+		const project = await createTeamProject('team', owner);
+		await linkUserToProject(member, project, 'project:viewer');
+		const { workflow, versionId } = await createReviewableWorkflow(project, {
+			versionId: 'version-1',
+		});
+		const request = await seedReview({
+			projectId: project.id,
+			workflowId: workflow.id,
+			versionId,
+			author: owner,
+			title: 'Open review',
+		});
+
+		const response = await listRequests(memberAgent, {
+			workflowId: workflow.id,
+			state: 'open',
+			take: 1,
+		}).expect(200);
+
+		expect(response.body.data.count).toBe(1);
+		expect(response.body.data.data[0].id).toBe(request.id);
+	});
+
+	test('refuses everything once an admin turns reviews off', async () => {
+		const { workflow } = await createReviewableWorkflow(owner);
+		await policyService.set(false);
+
+		await listRequests(ownerAgent, { workflowId: workflow.id }).expect(403);
+	});
+
+	// The only end-to-end proof that the `@Licensed` decorator is enforced by the
+	// middleware. Its presence on every handler is asserted exhaustively against
+	// the real route metadata in `workflow-review-requests.controller.test.ts`.
+	test('refuses everything on an instance without a workflow reviews licence', async () => {
+		testServer.license.disable('feat:workflowReviews');
+
+		await listRequests(ownerAgent, { workflowId: 'wf-1' }).expect(403);
+
+		testServer.license.enable('feat:workflowReviews');
+	});
+});

packages/cli/src/modules/workflow-reviews.ee/__tests__/workflow-review-version-update.controller.integration.test.ts

@@ -0,0 +1,417 @@
+import type { WorkflowReviewInboxItem } from '@n8n/api-types';
+import { mockInstance, testDb } from '@n8n/backend-test-utils';
+import type { Project, User } from '@n8n/db';
+import {
+	WorkflowHistoryRepository,
+	WorkflowReviewRequestAuthorRepository,
+	WorkflowReviewRequestRepository,
+	WorkflowReviewRequestWorkflowRepository,
+} from '@n8n/db';
+import { Container } from '@n8n/di';
+
+import { ActiveWorkflowManager } from '@/active-workflow-manager';
+import { WorkflowReviewPolicyService } from '@/services/workflow-review-policy.service';
+import { WorkflowValidationService } from '@/workflows/workflow-validation.service';
+import { createWorkflowHistoryItem } from '@test-integration/db/workflow-history';
+import type { SuperAgentTest } from '@test-integration/types';
+import * as utils from '@test-integration/utils';
+
+import {
+	createReviewableWorkflow,
+	findVersionName,
+	REVIEW_TABLES,
+	seedReview,
+	seedReviewActors,
+	stubWorkflowValidation,
+	versionUpdatePayload,
+} from './support/workflow-review-test-data';
+
+mockInstance(ActiveWorkflowManager);
+const workflowValidationService = mockInstance(WorkflowValidationService);
+
+const testServer = utils.setupTestServer({
+	endpointGroups: ['workflow-reviews', 'workflows'],
+	enabledFeatures: ['feat:workflowReviews'],
+	modules: ['workflow-reviews'],
+});
+
+let owner: User;
+let member: User;
+let ownerProject: Project;
+let teamProject: Project;
+let ownerAgent: SuperAgentTest;
+let memberAgent: SuperAgentTest;
+
+let requestRepository: WorkflowReviewRequestRepository;
+let workflowRepository: WorkflowReviewRequestWorkflowRepository;
+let authorRepository: WorkflowReviewRequestAuthorRepository;
+let workflowHistoryRepository: WorkflowHistoryRepository;
+let policyService: WorkflowReviewPolicyService;
+
+beforeAll(async () => {
+	await utils.initNodeTypes();
+	requestRepository = Container.get(WorkflowReviewRequestRepository);
+	workflowRepository = Container.get(WorkflowReviewRequestWorkflowRepository);
+	authorRepository = Container.get(WorkflowReviewRequestAuthorRepository);
+	workflowHistoryRepository = Container.get(WorkflowHistoryRepository);
+	policyService = Container.get(WorkflowReviewPolicyService);
+});
+
+beforeEach(async () => {
+	testServer.license.enable('feat:workflowReviews');
+	await testDb.truncate([...REVIEW_TABLES]);
+	await policyService.set(true);
+	stubWorkflowValidation(workflowValidationService);
+
+	({ owner, member, ownerProject, teamProject, ownerAgent, memberAgent } = await seedReviewActors(
+		testServer.authAgentFor,
+	));
+});
+
+/** Seed an open review pinned to `versionId`, authored by `author`. */
+async function seedOpenRequest(
+	workflowId: string,
+	versionId: string,
+	author: User,
+	projectId = ownerProject.id,
+	overrides: {
+		state?: 'open' | 'closed';
+		decision?: 'pending' | 'changes_requested';
+		description?: string | null;
+	} = {},
+) {
+	return await seedReview({
+		projectId,
+		workflowId,
+		versionId,
+		author,
+		title: 'Existing review',
+		...overrides,
+	});
+}
+
+const updateVersion = (agent: SuperAgentTest, requestId: string, body: object) =>
+	agent.post(`/workflow-review-requests/${requestId}/update-version`).send(body);
+
+/** A workflow with two history versions, so it can be re-pinned. */
+async function createRepinnableWorkflow(ownerOrProject: User | Project = owner) {
+	const { workflow } = await createReviewableWorkflow(ownerOrProject, { versionId: 'version-1' });
+	await createWorkflowHistoryItem(workflow.id, { versionId: 'version-2' });
+	return workflow;
+}
+
+describe('POST /workflow-review-requests/:workflowReviewRequestId/update-version', () => {
+	test('re-pins the version, resets the decision, and keeps the author list deduplicated', async () => {
+		const workflow = await createRepinnableWorkflow();
+		const request = await seedOpenRequest(workflow.id, 'version-1', owner, ownerProject.id, {
+			decision: 'changes_requested',
+		});
+
+		const response = await updateVersion(
+			ownerAgent,
+			request.id,
+			versionUpdatePayload({ workflowId: workflow.id, versionId: 'version-2' }),
+		).expect(200);
+
+		expect(response.body.data).toEqual({
+			id: request.id,
+			state: 'open',
+			decision: 'pending',
+			workflowVersionId: 'version-2',
+			createdAt: expect.any(String),
+			updatedAt: expect.any(String),
+		});
+		expect(JSON.stringify(response.body)).not.toMatch(/sync/i);
+
+		const childRows = await workflowRepository.find();
+		expect(childRows).toHaveLength(1);
+		expect(childRows[0]).toMatchObject({ workflowVersionId: 'version-2' });
+
+		const updated = await requestRepository.findById(request.id, {});
+		expect(updated).toMatchObject({ decision: 'pending', updatedById: owner.id });
+
+		const authorRows = await authorRepository.find();
+		expect(authorRows).toHaveLength(1);
+		expect(authorRows[0]).toMatchObject({ userId: owner.id });
+	});
+
+	test('appends a second publish-capable user to the authors', async () => {
+		const workflow = await createRepinnableWorkflow(teamProject);
+		const request = await seedOpenRequest(workflow.id, 'version-1', owner, teamProject.id);
+
+		await updateVersion(
+			memberAgent,
+			request.id,
+			versionUpdatePayload({ workflowId: workflow.id, versionId: 'version-2' }),
+		).expect(200);
+
+		const authorRows = await authorRepository.find();
+		expect(authorRows.map((row) => row.userId).sort()).toEqual([member.id, owner.id].sort());
+
+		const updated = await requestRepository.findById(request.id, {});
+		expect(updated).toMatchObject({ updatedById: member.id });
+
+		// Both read surfaces expose every author while keeping the original requester
+		// canonical. Author order is the frontend's concern, so only membership is asserted.
+		const inbox = await ownerAgent.get('/workflow-review-requests/inbox').expect(200);
+		const inboxItem = (inbox.body.data.data as WorkflowReviewInboxItem[]).find(
+			(item) => item.id === request.id,
+		)!;
+		expect(inboxItem.requester).toMatchObject({ id: owner.id });
+		expect(inboxItem.authors.map((author) => author.id).sort()).toEqual(
+			[member.id, owner.id].sort(),
+		);
+
+		const detail = await ownerAgent.get(`/workflow-review-requests/${request.id}`).expect(200);
+		expect(detail.body.data.requester).toMatchObject({ id: owner.id });
+		expect(
+			(detail.body.data as WorkflowReviewInboxItem).authors.map((author) => author.id).sort(),
+		).toEqual([member.id, owner.id].sort());
+	});
+
+	test('writes nothing when the version under review is unchanged', async () => {
+		const { workflow, versionId } = await createReviewableWorkflow(owner);
+		const request = await seedOpenRequest(workflow.id, versionId, owner, ownerProject.id, {
+			decision: 'changes_requested',
+		});
+
+		const response = await updateVersion(
+			ownerAgent,
+			request.id,
+			versionUpdatePayload({ workflowId: workflow.id, versionId }),
+		).expect(200);
+
+		// No-op: nothing new to review, so the decision is deliberately NOT reset.
+		expect(response.body.data).toMatchObject({
+			id: request.id,
+			decision: 'changes_requested',
+			workflowVersionId: versionId,
+		});
+
+		const unchanged = await requestRepository.findById(request.id, {});
+		expect(unchanged?.updatedAt).toEqual(request.updatedAt);
+		expect(unchanged?.decision).toBe('changes_requested');
+	});
+
+	test('updates the review description when re-pinning the version', async () => {
+		const workflow = await createRepinnableWorkflow();
+		const request = await seedOpenRequest(workflow.id, 'version-1', owner, ownerProject.id, {
+			description: 'Original review description',
+		});
+
+		await updateVersion(
+			ownerAgent,
+			request.id,
+			versionUpdatePayload({
+				workflowId: workflow.id,
+				versionId: 'version-2',
+				description: '  Updated review description  ',
+			}),
+		).expect(200);
+
+		const updated = await requestRepository.findById(request.id, {});
+		expect(updated?.description).toBe('Updated review description');
+	});
+
+	test('updates the review description when the version is already pinned', async () => {
+		const { workflow, versionId } = await createReviewableWorkflow(owner);
+		const request = await seedOpenRequest(workflow.id, versionId, owner, ownerProject.id, {
+			decision: 'changes_requested',
+			description: 'Original review description',
+		});
+
+		await updateVersion(
+			ownerAgent,
+			request.id,
+			versionUpdatePayload({
+				workflowId: workflow.id,
+				versionId,
+				description: 'Updated review description',
+			}),
+		).expect(200);
+
+		const updated = await requestRepository.findById(request.id, {});
+		expect(updated).toMatchObject({
+			description: 'Updated review description',
+			decision: 'changes_requested',
+		});
+	});
+
+	test.each([
+		{ name: 'an empty string', description: '' },
+		{ name: 'a whitespace-only string', description: '   ' },
+	])('clears the review description when $name is sent', async ({ description }) => {
+		const { workflow, versionId } = await createReviewableWorkflow(owner);
+		const request = await seedOpenRequest(workflow.id, versionId, owner, ownerProject.id, {
+			description: 'Original review description',
+		});
+
+		await updateVersion(
+			ownerAgent,
+			request.id,
+			versionUpdatePayload({ workflowId: workflow.id, versionId, description }),
+		).expect(200);
+
+		expect((await requestRepository.findById(request.id, {}))?.description).toBeNull();
+	});
+
+	test('hides a review that does not exist', async () => {
+		const { workflow, versionId } = await createReviewableWorkflow(owner);
+
+		await updateVersion(
+			ownerAgent,
+			'unknown-request',
+			versionUpdatePayload({ workflowId: workflow.id, versionId }),
+		).expect(404);
+	});
+
+	test('hides a review whose workflow the caller cannot access', async () => {
+		const { workflow, versionId } = await createReviewableWorkflow(owner);
+		const request = await seedOpenRequest(workflow.id, versionId, owner);
+
+		await updateVersion(
+			memberAgent,
+			request.id,
+			versionUpdatePayload({ workflowId: workflow.id, versionId }),
+		).expect(404);
+	});
+
+	test('hides a workflow the review does not cover', async () => {
+		const { workflow, versionId } = await createReviewableWorkflow(owner);
+		const other = await createReviewableWorkflow(owner, { versionId: 'version-other' });
+		const request = await seedOpenRequest(workflow.id, versionId, owner);
+
+		await updateVersion(
+			ownerAgent,
+			request.id,
+			versionUpdatePayload({ workflowId: other.workflow.id, versionId: 'version-other' }),
+		).expect(404);
+	});
+
+	test('refuses to re-pin a closed review', async () => {
+		const { workflow, versionId } = await createReviewableWorkflow(owner);
+		const request = await seedOpenRequest(workflow.id, versionId, owner, ownerProject.id, {
+			state: 'closed',
+		});
+
+		const response = await updateVersion(
+			ownerAgent,
+			request.id,
+			versionUpdatePayload({ workflowId: workflow.id, versionId }),
+		).expect(409);
+
+		expect(JSON.stringify(response.body)).not.toMatch(/sync/i);
+	});
+
+	test('refuses an archived workflow', async () => {
+		const { workflow, versionId } = await createReviewableWorkflow(owner, { isArchived: true });
+		const request = await seedOpenRequest(workflow.id, versionId, owner);
+
+		await updateVersion(
+			ownerAgent,
+			request.id,
+			versionUpdatePayload({ workflowId: workflow.id, versionId }),
+		).expect(400);
+	});
+
+	test('refuses a version the workflow does not have', async () => {
+		const { workflow, versionId } = await createReviewableWorkflow(owner);
+		const request = await seedOpenRequest(workflow.id, versionId, owner);
+
+		await updateVersion(
+			ownerAgent,
+			request.id,
+			versionUpdatePayload({ workflowId: workflow.id, versionId: 'unknown-version' }),
+		).expect(400);
+	});
+
+	test('refuses everything once an admin turns reviews off', async () => {
+		const { workflow, versionId } = await createReviewableWorkflow(owner);
+		const request = await seedOpenRequest(workflow.id, versionId, owner);
+		await policyService.set(false);
+
+		await updateVersion(
+			ownerAgent,
+			request.id,
+			versionUpdatePayload({ workflowId: workflow.id, versionId }),
+		).expect(403);
+	});
+
+	describe('pinned version naming', () => {
+		test('names the newly pinned version', async () => {
+			const workflow = await createRepinnableWorkflow();
+			const request = await seedOpenRequest(workflow.id, 'version-1', owner);
+
+			await updateVersion(
+				ownerAgent,
+				request.id,
+				versionUpdatePayload({ workflowId: workflow.id, versionId: 'version-2' }),
+			).expect(200);
+
+			expect(await findVersionName(workflow.id, 'version-2')).toBe('Release candidate');
+			// The previously pinned version keeps whatever name it had.
+			expect(await findVersionName(workflow.id, 'version-1')).toBeNull();
+		});
+
+		test('renames the version on a re-pin to the version already pinned', async () => {
+			const { workflow, versionId } = await createReviewableWorkflow(owner);
+			const request = await seedOpenRequest(workflow.id, versionId, owner);
+
+			await updateVersion(
+				ownerAgent,
+				request.id,
+				versionUpdatePayload({ workflowId: workflow.id, versionId, versionName: 'Renamed' }),
+			).expect(200);
+
+			expect(await findVersionName(workflow.id, versionId)).toBe('Renamed');
+			// Still a no-op for the review itself.
+			const unchanged = await requestRepository.findById(request.id, {});
+			expect(unchanged?.updatedAt).toEqual(request.updatedAt);
+		});
+
+		test('persists the version description on a re-pin', async () => {
+			const workflow = await createRepinnableWorkflow();
+			const request = await seedOpenRequest(workflow.id, 'version-1', owner);
+
+			await updateVersion(
+				ownerAgent,
+				request.id,
+				versionUpdatePayload({
+					workflowId: workflow.id,
+					versionId: 'version-2',
+					workflowVersionDescription: 'What changed in this version',
+				}),
+			).expect(200);
+
+			const version = await workflowHistoryRepository.findOneBy({
+				workflowId: workflow.id,
+				versionId: 'version-2',
+			});
+			expect(version?.description).toBe('What changed in this version');
+		});
+
+		test('updates the description of the version already pinned', async () => {
+			const { workflow, versionId } = await createReviewableWorkflow(owner);
+			const request = await seedOpenRequest(workflow.id, versionId, owner);
+
+			await updateVersion(
+				ownerAgent,
+				request.id,
+				versionUpdatePayload({
+					workflowId: workflow.id,
+					versionId,
+					workflowVersionDescription: 'Description added later',
+				}),
+			).expect(200);
+
+			const version = await workflowHistoryRepository.findOneBy({
+				workflowId: workflow.id,
+				versionId,
+			});
+			expect(version?.description).toBe('Description added later');
+			// Still a no-op for the review itself.
+			const unchanged = await requestRepository.findById(request.id, {});
+			expect(unchanged?.updatedAt).toEqual(request.updatedAt);
+		});
+	});
+});